Re: Re: proposal: schema variables

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, Erik Rijkers <er(at)xs4all(dot)nl>, Michael Paquier <michael(at)paquier(dot)xyz>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, DUVAL REMI <REMI(dot)DUVAL(at)cheops(dot)fr>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Re: proposal: schema variables
Date: 2025-02-09 20:56:54
Message-ID: CAFj8pRDqK4XMpsmKMx+ERPBMnsCfQ9x+Q=NAeiaGVXVRSx1rcQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-performance

Hi

> create domain dnn as int not null;
> CREATE VARIABLE var3 AS dnn;
> alter domain dnn drop not null;
> alter domain dnn set not null;
> ERROR: cannot alter domain "dnn" because session variable "public.var3"
> uses it
>

> This is not great.
> we allow a constraint, then we drop it, now we cannot re add it again.
>

I think this is correct. And I am afraid so we need to choose from two bad
things. There is not possible any other way

The data of session variables are not shared, so we cannot recheck if data
are valid against new constraints or not.

Then there was two possibilities (and not any other)
a) disallow add new constraints
b) ignore possible inconsistency

I implemented @a (can be easily removed), for example temp tables uses @b

you can try - in one session do modification of domain
and in second session do just

create temp table t (a dnn);
-- after drop not null
insert into t values(null);
-- you can successfully set not null to dnn
insert into t values(null); -- now fails
but select * from t -- and the t has null in a

So unfortunately there is no good solution - you can prefer consistency
(and stop altering) or you can allow altering (and stop consistency).

Usage of variables disallow type altering already - so disallowing altering
domain looks more correct - but I am open to any discussion. I can change
the behaviour the same way like temp tables.
In this case I don't see any reason be extra dogmatic - after any update or
reset, the value will be corrected for new constraints, and what is
important - although the value will not be fully consistent for domain
type, it is still fully consistent for base type, so there is not risk of
crashes - and after disconnect of all sessions, the all inconsistent values
will be lost equally like data of temp tables. Now, a more restrictive
variant is implemented - so it is safe, and future change should not have
compatibility issues.

> 0001 and 0002 are simple, but the size is still large.
> maybe we can not support the domain in the first 2 patches.
>

Removing domain support decrease the size about few kilobytes

> also
> CREATE VARIABLE var3 AS dnn;
> let var3 = 11;
> create view v1 as select var3;
> select * from v1;
> you can see reconnect to session then
> ERROR: domain dnn does not allow null values
> this is not ok?
>

this is ok

the content of session variables is not shared. The variable in the second
session is initialized to null, and the null is disallowed.

The variable is initialized when it is used for the first time in the
session. And it is initialized to default value - without default value,
it is initialized to NULL (possibility to set default does different
patch). So when you use a view for the first time, then the variable is
initialized
and it fails.

It is consistent with plpgsql

(2025-02-09 20:23:14) postgres=#
do $$
declare x dnn;
begin
end
$$;
ERROR: domain dnn does not allow null values
CONTEXT: PL/pgSQL function inline_code_block line 2 during statement block
local variable initialization

There is no raised syntax error or some compilation error. The raised error
is just runtime initialization error like session variables

>
>
> also
> create table t(var3 int);
> CREATE POLICY p1r ON t AS RESTRICTIVE TO alice USING (var3 <> var3);
> create table t1(a int);
> CREATE POLICY p2 ON t1 AS RESTRICTIVE TO alice USING (a <> var3);
> p1r is so confusing. there is no way to understand the intention.
> p2 should also not be allowed, since var3 value is volatile,
> session reconnection will change the value.
>

One of the main targets of session variables is using RLS.

Postgres allows to

`CREATE POLICY p2 ON t1 AS RESTRICTIVE TO pavel USING (a <>
current_user::regrole::int)`

and I don't need reconnect - just I can use `set role to ...` or I can use
security definer function

Inside the queries, the session variables are stable, because the values
are passed as copy when the query is started.

This can be changed by wrapping a volatile function, but on the top level,
the variables are stable (we talked about implementation of really volatile
variables), but it is not implemented.

create variable v int;

CREATE OR REPLACE FUNCTION public.fx()
RETURNS integer
LANGUAGE plpgsql
AS $function$
begin
let v = v + 1;
return v;
end;
$function$

(2025-02-09 20:41:34) postgres=# select v, fx() from generate_series(1,10);
┌───┬────┐
│ v │ fx │
╞═══╪════╡
│ 5 │ 6 │
│ 5 │ 7 │
│ 5 │ 8 │
│ 5 │ 9 │
│ 5 │ 10 │
│ 5 │ 11 │
│ 5 │ 12 │
│ 5 │ 13 │
│ 5 │ 14 │
│ 5 │ 15 │
└───┴────┘
(10 rows)

The current behave is similar to usage plpgsql variables in queries - the
values are copied and are stable for the query

(2025-02-09 20:49:15) postgres=# do $$
declare
r record;
v int default 0;
begin
for r in select v, i from generate_series(1,3) g(i)
loop
v := v + 1;
raise notice '%', r;
end loop;
end$$;
NOTICE: (0,1)
NOTICE: (0,2)
NOTICE: (0,3)
DO

>
>
> src/bin/pg_dump/pg_dump.h
> /*
> * The VariableInfo struct is used to represent session variables
> */
> typedef struct _VariableInfo
> {
> DumpableObject dobj;
> DumpableAcl dacl;
> Oid vartype;
> char *vartypname;
> char *varacl;
> char *rvaracl;
> char *initvaracl;
> char *initrvaracl;
> Oid varcollation;
> const char *rolname; /* name of owner, or empty string */
> } VariableInfo;
> these fields (varacl, rvaracl, initvaracl, initrvaracl) were never being
> used.
> we can remove them.
>

removed

>
> CollInfo *coll;
> coll = findCollationByOid(varcollation);
> if (coll)
> appendPQExpBuffer(query, " COLLATE %s",
> fmtQualifiedDumpable(coll));
> here, it should be
> ```CollInfo *coll = NULL;```?
>

No, why? The coll value is set by findCollationByOid every time, so
initialization is useless.

>
> I don't understand the changes made in getAdditionalACLs.
> I thought pg_init_privs had nothing to do with the session variable.
>

yes, it is useless

removed

>
>
> minor issue in getVariables.
> query = createPQExpBuffer();
> resetPQExpBuffer(query);
> no need to use resetPQExpBuffer here.
>

fixed

>
>
> create type ab as (a int, b text);
> create type abc as (a ab, c text);
> create type abcd as (a abc, d text);
> CREATE VARIABLE var2 AS abcd;
>
> select var2.a.c;
> ERROR: cross-database references are not implemented: var2.a.c
>
> Is this error what we expected? I am not 100% sure.
>

Unfortunately, it is expected. Postgres parser doesn't support direct
access to nested composite types. It can be designed differently, but it is
implemented
to be consistent with current postgres behavior.

create type ab as (a int, b text);
create type abc as (a ab, c text);
create type abcd as (a abc, d text);
create table foo(a abc);

(2025-02-09 06:53:36) postgres=# select foo.a.a from foo;
ERROR: cross-database references are not implemented: foo.a.a

you should to use parenthesis

(2025-02-09 06:53:42) postgres=# select (foo.a).a from foo;
┌───┐
│ a │
╞═══╡
└───┘
(0 rows)

--------------------another contrived corner case.-----------------------
> create type pg_variable as (
> oid oid, vartype oid, varcreate_lsn pg_lsn,
> varname name, varnamespace oid, varowner oid,
> vartypmod int, varcollation oid, varacl aclitem[]);
> create variable pg_variable as pg_variable;
> let pg_variable = row (18041, 10116, '0/25137B0','pg_variable', 2200,
> 10,-1,0, NULL)::pg_variable;
>
> select pg_variable.oid from pg_variable where pg_variable.oid =
> pg_variable.oid;
> this query, the WHERE clause, it's really hard to distinguish session
> variable or column reference.
> I am not sure if this is fine or not.
>

Yes, the basic rule is - don't name the variable exactly like the table or
like the column. Better use a dedicated schema that should not be on the
search path.

There are some prepared tools already

create variable pg_variable as pg_variable;

* patch 07

set session_variables_ambiguity_warning to on;

(2025-02-09 21:09:22) postgres=# select pg_variable.oid from pg_variable
where pg_variable.oid = pg_variable.oid;
WARNING: session variable "pg_variable.oid" is shadowed
LINE 1: select pg_variable.oid from pg_variable where pg_variable.oi...
^
DETAIL: Session variables can be shadowed by columns, routine's variables
and routine's arguments with the same name.
WARNING: session variable "pg_variable.oid" is shadowed
LINE 1: select pg_variable.oid from pg_variable where pg_variable.oi...
^
DETAIL: Session variables can be shadowed by columns, routine's variables
and routine's arguments with the same name.
WARNING: session variable "pg_variable.oid" is shadowed
LINE 1: ...able.oid from pg_variable where pg_variable.oid = pg_variabl...
^
DETAIL: Session variables can be shadowed by columns, routine's variables
and routine's arguments with the same name.
┌───────┐
│ oid │
╞═══════╡
│ 16389 │
└───────┘
(1 row)

* patch 08 - variable fencing - it helps with forcing of usage of some
identifier like the variable

(2025-02-09 21:11:58) postgres=# set session_variables_ambiguity_warning to
off;
SET
(2025-02-09 21:12:05) postgres=# let pg_variable.oid = 16389;
LET
(2025-02-09 21:12:07) postgres=# select pg_variable.oid from pg_variable
where pg_variable.oid = variable(pg_variable).oid;
┌───────┐
│ oid │
╞═══════╡
│ 16389 │
└───────┘
(1 row)

(2025-02-09 21:12:10) postgres=# select pg_variable.oid from pg_variable
where pg_variable.oid = variable(pg_variable.oid);
┌───────┐
│ oid │
╞═══════╡
│ 16389 │
└───────┘
(1 row)

* patch 09 - possibility to force usage of variable fencing in some
contexts - it can raise an error when the session variable was used,
but without fencing (in some contexts). Now the contextes are "none, all,
nonspi", but probably better (and more intuitive) will be (and +/- equal)
"none, all, nested".
patch 09 implements some protection against unwanted usage of the session
variable.

I think these tools are enough for safe working with session variables. But
(similarly to PL/pgSQL or PL/SQL) the basic rule is usage of names that are
not in possible collisions.

I don't want to force dedicated schema or dedicated syntax - because it can
be a problem for porting from other databases. But common style for
plpgsql or plsql is using some form of hungarian notation or using prefixes
like _ for local variable and __ for global variable - or using schema name
like package name
schema_name.varname

regards

Pavel

Attachment Content-Type Size
v20250209-0022-pg_restore-A-variable.patch text/x-patch 2.8 KB
v20250209-0021-transactional-variables.patch text/x-patch 37.3 KB
v20250209-0020-this-patch-changes-error-message-column-doesn-t-exis.patch text/x-patch 29.2 KB
v20250209-0019-expression-with-session-variables-can-be-inlined.patch text/x-patch 4.2 KB
v20250209-0018-plpgsql-implementation-for-LET-statement.patch text/x-patch 17.3 KB
v20250209-0017-allow-parallel-execution-queries-with-session-variab.patch text/x-patch 15.8 KB
v20250209-0016-allow-read-an-value-of-session-variable-directly-fro.patch text/x-patch 15.5 KB
v20250209-0015-Implementation-of-NOT-NULL-and-IMMUTABLE-clauses.patch text/x-patch 33.9 KB
v20250209-0014-Implementation-of-DEFAULT-clause-default-expressions.patch text/x-patch 32.1 KB
v20250209-0012-implementation-of-temporary-session-variables.patch text/x-patch 40.6 KB
v20250209-0011-PREPARE-LET-support.patch text/x-patch 7.4 KB
v20250209-0010-EXPLAIN-LET-support.patch text/x-patch 8.2 KB
v20250209-0013-Implementation-ON-TRANSACTION-END-RESET-clause.patch text/x-patch 14.6 KB
v20250209-0009-dynamic-check-of-usage-of-session-variable-fences.patch text/x-patch 16.3 KB
v20250209-0007-GUC-session_variables_ambiguity_warning.patch text/x-patch 15.1 KB
v20250209-0006-plpgsql-tests.patch text/x-patch 16.9 KB
v20250209-0005-memory-cleaning-after-DROP-VARIABLE.patch text/x-patch 20.9 KB
v20250209-0004-DISCARD-VARIABLES.patch text/x-patch 9.6 KB
v20250209-0008-variable-fence-syntax-support-and-variable-fence-usa.patch text/x-patch 19.5 KB
v20250209-0003-function-pg_session_variables-for-cleaning-tests.patch text/x-patch 4.3 KB
v20250209-0002-Storage-for-session-variables-and-SQL-interface.patch text/x-patch 159.7 KB
v20250209-0001-Enhancing-catalog-for-support-session-variables-and-.patch text/x-patch 169.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2025-02-09 22:13:26 Re: describe special values in GUC descriptions more consistently
Previous Message Jacob Brazeal 2025-02-09 20:41:58 Re: MAX_BACKENDS size (comment accuracy)

Browse pgsql-performance by date

  From Date Subject
Next Message Tom Lane 2025-02-09 22:40:33 Re: Poor performance with row wise comparisons
Previous Message kyle Hailey 2025-02-08 19:41:18 Re: lwlock:LockManager wait_events