MIME-Version: 1.0
References: <afb4e296d7ce57986f23bcfdee39b259b8f85f56.camel@cybertec.at>
 <CAFj8pRAjU-X6rEE9=1++PdtXOPc2uo=yu-tcFXByi-kN3B_7Vw@mail.gmail.com>
 <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>
 <CAFj8pRDy4jhPE8K+Y_0xHqCjWnG65byF+CQFg6Tk+9MwgHYsAw@mail.gmail.com>
 <CAFj8pRA_JetqEqFcZMxhOTBVCHWbvftNipv25OZjhhbe8k4_sw@mail.gmail.com>
 <CAFj8pRCJ1_XZZAWGuFdPjFGf2WgPSbH8LLoFDOTnO4_9Z9o_Ww@mail.gmail.com>
 <CAFj8pRA00BYE25sE9DwrLcsDn6OY5E=FXNSRMCOF34Y=AuP6cw@mail.gmail.com>
 <CAFj8pRAGAQf0+0cVVRmUJpwyATtBP1YCRU0SxtVQNg+yT=Ongw@mail.gmail.com>
 <CACJufxHx3FyO4jkv24W9Q_e5weSSy2Nv1Ue437nWU2MQsgu3RA@mail.gmail.com>
 <CAFj8pRByN-He41pSh0nRj4swQEPiRZ1aQ6TcFEqyCV_FZXZukg@mail.gmail.com>
In-Reply-To: 
 <CAFj8pRByN-He41pSh0nRj4swQEPiRZ1aQ6TcFEqyCV_FZXZukg@mail.gmail.com>
From: jian he <jian.universality@gmail.com>
Date: Mon, 9 Dec 2024 14:16:04 +0800
Message-ID: 
 <CACJufxHCZ1pidPKAqB_-wpA+Dqa2_n-LJQMwoCsHRQE-xZ_v2A@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"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CACJufxHCZ1pidPKAqB_-wpA%2BDqa2_n-LJQMwoCsHRQE-xZ_v2A%40mail.gmail.com>
Precedence: bulk

On Mon, Dec 9, 2024 at 2:33=E2=80=AFAM Pavel Stehule <pavel.stehule@gmail.c=
om> wrote:
>
> Hi
>
again. only applied
v20241208-0001-Enhancing-catalog-for-support-session-variables-and-.patch.

/* we want SessionVariableCreatePostprocess to see the catalog changes. */
0001 doesn't have SessionVariableCreatePostprocess,
so this comment is wrong in the context of 0001.

typo:
As above, but if the variable isn't found and is_mussing is not NULL
is_mussing should be is_missing.

----------------------------------------------
issue with  grant.sgml and revoke.sgml.

* there are no regress tests for WITH GRANT OPTION but it seems to work;
there are no REVOKE CASCADE tests. the following tests show
REVOKE CASCADE works.

create variable v1 as int;
GRANT select on VARIABLE v1 to alice with grant option;
set session authorization alice;
GRANT select on VARIABLE v1 to bob with grant option;
reset session authorization;

select varacl from pg_variable where varname  =3D 'v1';
--output
{jian=3Drw/jian,alice=3Dr*/jian,bob=3Dr*/alice}
revoke all privileges on variable v1 from alice cascade;
select varacl from pg_variable where varname  =3D 'v1';
--output
 {jian=3Drw/jian}

revoke GRANT OPTION FOR all privileges on variable v1 from alice cascade;
also works.

* in revoke.sgml and grant.sgml.
if you look closely, " | ALL VARIABLES IN SCHEMA schema_name [, ...] }" is =
wrong
because there is no left-curly-bracket, but there is a right-curly-bracket.

* in revoke.sgml.
<phrase>where <replaceable
class=3D"parameter">role_specification</replaceable> can be:</phrase>
    [ GROUP ] <replaceable class=3D"parameter">role_name</replaceable>
  | PUBLIC
  | CURRENT_ROLE
  | CURRENT_USER
  | SESSION_USER
should be at the end of the synopsis section?
otherwise it looks weird, maybe we can put the REVOKE VARIABLE code upper.
grant.sgml changes position looks fine to me.


*  <para>
   The <command>GRANT</command> command has two basic variants: one
   that grants privileges on a database object (table, column, view,
   foreign table, sequence, database, foreign-data wrapper, foreign server,
   function, procedure, procedural language, large object, configuration
   parameter, schema, tablespace, or type), and one that grants
   membership in a role.  These variants are similar in many ways, but
   they are different enough to be described separately.
  </para>
this <para> in grant.sgml needs to also mention "variable"?

*   <para>
    Privileges on databases, tablespaces, schemas, languages, and
    configuration parameters are
    <productname>PostgreSQL</productname> extensions.
   </para>
this <para> in grant.sgml needs to also mention "variable"?

----------------------------------------------
*
+   <para>
+    Inside a query or an expression, a session variable can be
+    <quote>shadowed</quote> by a column with the same name.  Similarly, th=
e
+    name of a function or procedure argument or a PL/pgSQL variable (see
PL/pgSQL should decorated as <application>PL/pgSQL</application>

* we already support \dV and \dV+ in
v20241208-0001-Enhancing-catalog-for-support-session-variables-and-.patch
so we should update doc/src/sgml/ref/psql-ref.sgml also.
I briefly searched \dV in
v20241208-0002-Storage-for-session-variables-and-SQL-interface.patch,
no entry.


* in doc/src/sgml/ddl.sgml
<table id=3D"privilege-abbrevs-table"> and <table
id=3D"privileges-summary-table"> need to change for variable?
<varlistentry id=3D"ddl-priv-select">, <varlistentry
id=3D"ddl-priv-update"> need to change for variable?

it's kind of tricky. if we only apply
v20241208-0001-Enhancing-catalog-for-support-session-variables-and-.patch
we can not  SELECT or  UPDATE variable.
so how are we going to mention these privileges for variable?


 * we can add some tests for EVENT TRIGGER test,
since event trigger support CREATE|DROP variable. atually, I think
there is a bug.

create variable v1 as int;
CREATE OR REPLACE FUNCTION event_trigger_report_dropped()
RETURNS event_trigger
LANGUAGE plpgsql
AS $$
DECLARE r record;
BEGIN
FOR r IN SELECT * from pg_event_trigger_dropped_objects()
LOOP
IF NOT r.normal AND NOT r.original THEN
    CONTINUE;
END IF;
RAISE NOTICE 'NORMAL: orig=3D% normal=3D% istemp=3D% type=3D% identity=3D% =
name=3D% args=3D%',
    r.original, r.normal, r.is_temporary, r.object_type,
    r.object_identity, r.address_names, r.address_args;
END LOOP;
END; $$;

CREATE EVENT TRIGGER regress_event_trigger_report_dropped ON sql_drop
WHEN TAG IN ('DROP VARIABLE')
EXECUTE PROCEDURE event_trigger_report_dropped();

--output:
src9=3D# drop variable v1;
NOTICE:  test_event_trigger: ddl_command_start DROP VARIABLE
NOTICE:  NORMAL: orig=3Dt normal=3Df istemp=3Df type=3Dsession variable
identity=3Dpublic.v1 name=3D{public,$} args=3D{}
DROP VARIABLE

should i expect
NOTICE:  NORMAL: orig=3Dt normal=3Df istemp=3Df type=3Dsession variable
identity=3Dpublic.v1 name=3D{public,$} args=3D{}
to be
NOTICE:  NORMAL: orig=3Dt normal=3Df istemp=3Df type=3Dsession variable
identity=3Dpublic.v1 name=3D{public,v1} args=3D{}
?