On Mon, Dec 9, 2024 at 2:33 AM Pavel Stehule <[email protected]> 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 = 'v1';
--output
{jian=rw/jian,alice=r*/jian,bob=r*/alice}
revoke all privileges on variable v1 from alice cascade;
select varacl from pg_variable where varname = 'v1';
--output
{jian=rw/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="parameter">role_specification</replaceable> can be:</phrase>
[ GROUP ] <replaceable class="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, the
+ 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="privilege-abbrevs-table"> and <table
id="privileges-summary-table"> need to change for variable?
<varlistentry id="ddl-priv-select">, <varlistentry
id="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=% normal=% istemp=% type=% identity=% name=% args=%',
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=# drop variable v1;
NOTICE: test_event_trigger: ddl_command_start DROP VARIABLE
NOTICE: NORMAL: orig=t normal=f istemp=f type=session variable
identity=public.v1 name={public,$} args={}
DROP VARIABLE
should i expect
NOTICE: NORMAL: orig=t normal=f istemp=f type=session variable
identity=public.v1 name={public,$} args={}
to be
NOTICE: NORMAL: orig=t normal=f istemp=f type=session variable
identity=public.v1 name={public,v1} args={}
?
