MIME-Version: 1.0
References: 
 <CAFj8pRC+hPCc2X88xC=pTJoqmVPApDsageZOMyqaxi5788WxHA@mail.gmail.com>
 <CAFj8pRDJ9cq00VYSHxs6LsoHNWjhYXyWWBtV6UgeWwhs0AHa9A@mail.gmail.com>
 <CAFj8pRBPXTcw_3fpKtgVthV2+9rZGhxitZ40DnAwCrK601TZZg@mail.gmail.com>
 <ndtfl4tsnpkb7m7hwvnmlpsascpgd3a7xvjmjhtxffsbrgygtm@4du6zsmnnwq5>
 <CAFj8pRAu4XvNCGu1751t=2YEqLqTjDA3FavMExm2S0KYQq=DdQ@mail.gmail.com>
 <CAFj8pRAsEoeZv0HEnA8CKgFKDSQ-wYw18Os1vdksWCV7ez2bVw@mail.gmail.com>
 <3chredgnjcmccym2kczawfih226b4ac6co7p6z4jeofevrcosi@mrsxkx2x2c65>
 <CAFj8pRBoWPDTOwn5FmMzc+1qiopw+N04U26nviOdF61fs8A2wQ@mail.gmail.com>
 <stckyvkl4yyzvgjsaawojs3xikke7mmds5bhv7l7qerclywywk@h4v4n43xm6u2>
 <CAFj8pRB_E1GM_YGT-ti4bXka6mhLdAAFeTe+BHgHFYC+qb-76g@mail.gmail.com>
 <20241120201313.t4wbhld4ktgielaf@erthalion.local>
 <CAFj8pRBWqEb8i6WmrF_Xh64=48GtisKijgczMv7HTTpe4GswuA@mail.gmail.com>
 <CAFj8pRAry0esQiHcK=6BwwFKDY0zanug6k07CEQzRPBqZ6iW0Q@mail.gmail.com>
In-Reply-To: 
 <CAFj8pRAry0esQiHcK=6BwwFKDY0zanug6k07CEQzRPBqZ6iW0Q@mail.gmail.com>
From: jian he <jian.universality@gmail.com>
Date: Wed, 18 Dec 2024 11:00:01 +0800
Message-ID: 
 <CACJufxFNjKrmyEi9SLfPCq4c9GUN+5eoOtbZwBPq9eKoO8REUw@mail.gmail.com>
Subject: Re: proposal: schema variables
To: Pavel Stehule <pavel.stehule@gmail.com>
Cc: Dmitry Dolgov <9erthalion6@gmail.com>,
 Laurenz Albe <laurenz.albe@cybertec.at>,
	Erik Rijkers <er@xs4all.nl>, Michael Paquier <michael@paquier.xyz>,
	Amit Kapila <amit.kapila16@gmail.com>, DUVAL REMI <REMI.DUVAL@cheops.fr>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CACJufxFNjKrmyEi9SLfPCq4c9GUN%2B5eoOtbZwBPq9eKoO8REUw%40mail.gmail.com>
Precedence: bulk

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."
?

typo, "sessin" should be "session".
----------------<<<>>>>-------------------
  <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."

+    <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">
----------------<<<>>>>-------------------
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 ]

----------------<<<>>>>-------------------
<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".
----------------<<<>>>>-------------------
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;
----------------<<<>>>>-----------------------------
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
----------------<<<>>>>-----------------------------
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"
----------------<<<>>>>-----------------------------
doc/src/sgml/ref/alter_variable.sgml
<title>Parameters</title> section, the order should
be: name, new_owner, new_name, new_schema?

I am beginning to look around 0002.