Re: proposal: schema variables

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