st 18. 12. 2024 v 4:00 odesílatel jian he <jian.universality@gmail.com> napsal:
hi.

/*
 * has_session_variable_privilege variants
 *        These are all named "has_session_variable_privilege" at the SQL level.
 *        They take various combinations of variable name, variable OID,
 *        user name, user OID, or implicit user = current_user.
 *
 *        The result is a boolean value: true if user has the indicated
 *        privilege, false if not.  The variants that take a relation OID
 *        return NULL if the OID doesn't exist.
 */
/*
 * has_session_variable_privilege_name_name
 *        Check user privileges on a session variable given
 *        name username, text sessin variable name, and text priv name.
 */
"The variants that take a relation OID return NULL if the OID doesn't exist."
should it be
"The variants that take an OID type return NULL if the OID doesn't exist."
?

yes, this comment was wrong, and I fixed it

 *<><-->The result is a boolean value: true if user has the indicated
 *<><-->privilege, false if not, or NULL if session variable doesn't
 *<><-->exists.
 

typo, "sessin" should be "session".

fixed
 
----------------<<<>>>>-------------------
  <sect1 id="ddl-session-variables">
   <title>Session Variables</title>
only mentioned that "Session variables themselves are persistent, but their
values are neither persistent nor shared (like the content of temporary tables).
"
I feel like this sentence is not that explicit. we actually want to say
"Once a session exits, the variable value is reset to NULL, one
session cannot see another session variable value."

This is not fully true. I wrote new paragraph there

   <para>
    The session variable holds a value in session memory.  This value is private
    to each session and is released when the session ends.
   </para>
 

+    <para>
+     A persistent database object that holds a value in session memory.  This
+     value is private to each session and is released when the session ends.
+     Read or write access to session variables is controlled by privileges,
+     similar to other database objects.
+    </para>
i do like this description in glossary.sgml.
maybe we can copy it and put it to ddl.sgml "<sect1 id="ddl-session-variables">

ok - I did it

----------------<<<>>>>-------------------
REVOKE [ GRANT OPTION FOR ]
    { { SELECT | UPDATE } [, ...] | ALL [ PRIVILEGES ] }
    ON { VARIABLE <replaceable>variable_name</replaceable> [, ...]
       | ALL VARIABLES IN SCHEMA <replaceable
class="parameter">schema_name</replaceable> [, ...] }
    FROM { [ GROUP ] <replaceable
class="parameter">role_specification</replaceable> | PUBLIC } [, ...]
    [ GRANTED BY <replaceable
class="parameter">role_specification</replaceable> ]
    [ CASCADE | RESTRICT ]
revoke, seems still not right.
since with this, we can say:
REVOKE ALL PRIVILEGES ON VARIABLE v1 FROM group group alice CASCADE;

i think the correct one should be:
REVOKE [ GRANT OPTION FOR ]
    { { SELECT | UPDATE } [, ...] | ALL [ PRIVILEGES ] }
    ON { VARIABLE <replaceable>variable_name</replaceable> [, ...]
       | ALL VARIABLES IN SCHEMA <replaceable
class="parameter">schema_name</replaceable> [, ...] }
    FROM <replaceable class="parameter">role_specification</replaceable> [, ...]
    [ GRANTED BY <replaceable
class="parameter">role_specification</replaceable> ]
    [ CASCADE | RESTRICT ]

fixed
 

----------------<<<>>>>-------------------
<programlisting>
CREATE VARIABLE public.current_user_id AS integer;
GRANT READ ON VARIABLE public.current_user_id TO PUBLIC;
LET current_user_id = (SELECT id FROM users WHERE usename = session_user);
SELECT current_user_id;
</programlisting>
"GRANT READ" should be "GRANT SELECT".

fixed - note it is from second patch
 
----------------<<<>>>>-------------------
doc/src/sgml/ref/alter_default_privileges.sgml
GRANT { SELECT | UPDATE | ALL [ PRIVILEGES ] }
    ON VARIABLES
    TO { [ GROUP ] <replaceable
class="parameter">role_name</replaceable> | PUBLIC } [, ...] [ WITH
GRANT OPTION ]
the above part is wrong?
should be:
GRANT { { SELECT | UPDATE } [,...]
    | ALL [ PRIVILEGES ] }
    ON VARIABLES
    TO { [ GROUP ] <replaceable
class="parameter">role_name</replaceable> | PUBLIC } [, ...] [ WITH
GRANT OPTION ]

since we can:
ALTER DEFAULT PRIVILEGES
FOR ROLE alice
IN SCHEMA svartest
GRANT SELECT, UPDATE ON VARIABLES TO bob;

fixed
 
----------------<<<>>>>-----------------------------
CREATE VARIABLE IF NOT EXISTS v2 AS comp;
grant update on variable v2 to alice;
set role alice;
LET v2.a  = 12; --acl permission error
LET v2.b = 12; --acl permission error
LET v2 = (11,12); --ok.

 

not sure this is the desired behavior, for composite type variables, you are
allowed to change all the values, but you are not allowed to update the field
value of the composite.  The following are normal table test update cases.

create type comp as (a int, b  int);
create table t2(a comp);
insert into t2 select '(11,12)';
grant update (a ) on t2 to alice;
set role alice;
update t2 set a.a = 13; --ok
update t2 set a.b = 13; --ok
update t2 set a = '(11,13)'; --ok

I think this is a bug, but I need more time for investigation. For field update you need to read the content
the variable, but you are missing SELECT right on the variable, and then the LET fails. Unfortunately
this is done inside the executor, so it is harder to fix it.

 
----------------<<<>>>>-----------------------------
domain seems to have an issue.

CREATE domain d1 AS int;
CREATE VARIABLE var1 AS d1;
let var1 = 3;
--this should fail?.
alter domain d1 add check (value <> 3);
select var1;
ERROR:  value for domain d1 violates check constraint "d1_check"

I fixed it

CREATE DOMAIN testvar_domain AS int;
CREATE VARIABLE var1 AS testvar_domain;

(2024-12-18 21:21:15) postgres=# ALTER DOMAIN testvar_domain ADD CHECK(value <> 100);
ERROR:  cannot alter domain "testvar_domain" because session variable "public.var1" uses it

Unfortunately I cannot force constraint check validation in other sessions, so the most safe solution for now is
restriction of this ALTER when domain is used by some variable.  I wrote regress tests for this.
Note: looks so validation of domain check constraints doesn't work for temporary tables (what is expected,
not sure if it is documented).

----------------<<<>>>>-----------------------------
doc/src/sgml/ref/alter_variable.sgml
<title>Parameters</title> section, the order should
be: name, new_owner, new_name, new_schema?

changed
 

I am beginning to look around 0002.

Thank you very much

Regards

Pavel