st 8. 1. 2025 v 10:31 odesílatel jian he <jian.universality@gmail.com> napsal:

hi.

you forgot change
<function>acldefault</function>
should add
'V' for <literal>SESSION VARIABLE</literal>

done

in doc/src/sgml/ddl.sgml
<sect1 id="ddl-session-variables">
maybe some examples (<programlisting>) of session variables being
shadowed would be great.

because doc/src/sgml/ref/create_variable.sgml said
<note>
<para>
Session variables can be <quote>shadowed</quote> by other identifiers.
For details, see <xref linkend="ddl-session-variables"/>.
</para>
</note>
If I click the link, then in ddl.sgml, there is also a bunch of text
saying that variable will be shadowed
in some situations, but there are no simple examples to demonstrate that.

done

"Create an date session variable <literal>var1</literal>:"
maybe it should be rephrased as
"Create a session variable <literal>var1</literal> as date data type?"
(i am not native english speaker)

changed to

Create a session variable <literal>var1</literal> of data type date

-----------------------------------------------------------------------
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -379,7 +379,8 @@ typedef struct Param
Expr xpr;
ParamKind paramkind; /* kind of parameter.
See above */
int paramid; /* numeric ID
for parameter */
- Oid paramtype; /* pg_type OID
of parameter's datatype */
+ /* pg_type OID of parameter's datatype */
+ Oid paramtype
pg_node_attr(query_jumble_ignore);

I think we need the above to make
select v2;
select v1;
normalized as one query.

Why do you think so? paramtype is PARAM_VARIABLE every time for all session variables.

If we will ignore paramtype, then we cannot to decide SELECT v1 and SELECT $1

-----------------------------------------------------------------------
when we create a new table, we do something like this:
DefineRelation
heap_create_with_catalog
GetNewRelFileNumber
GetNewOidWithIndex

relation Oid uniqueness and variable uniqueness is the same thing.
If variable oid uniqueness problem ever reached,
at that moment, we should care more about relation oid uniqueness problem?

and in GetNewRelFileNumber, we have comments:
""
* As with GetNewOidWithIndex(), there is some theoretical risk of a race
* condition, but it doesn't seem worth worrying about.
"""
also comments in GetNewOidWithIndex
"""
* Note that we are effectively assuming that the table has a relatively small
* number of entries (much less than 2^32) and there aren't very long runs of
* consecutive existing OIDs. This is a mostly reasonable assumption for
* system catalogs.
"""
that means pg_catalog.pg_variable.varcreate_lsn is not really necessary?

I don't think so. This is a different problem, and different use case (I think)

oids will always be unique. Any system table has a unique index on the oid column. It is true for session variables too.

The values of session variables are always in memory. The values of other database objects are primary in files. Postgres routines never hold database objects in memory longer than one command or one transaction. And in this time, the consistency of memory is protected by locks. When the system drops some database objects, then it drops related files too. Nothing similar is for session variables. It is a database object that is forever only in memory, I cannot verify the content against some file. varcreate_lsn is used instead.

You can imagine scenario:

session A:

CREATE VARIABLE v1 AS int;

-- variable v1 with oid 1 is created

session B:

LET v1 = 100;

PREPARE p AS SELECT v1;

session A:

repeat bin n times:

DROP VARIABLE v1;

CREATE VARIABLE v1 AS numeric;

session b:

there is possibility so there will be variable with varid 1

EXECUTE p --> CRASH

The plan of p will be replaced (due dependency), but without possibility to check against varcreate_lsn we will use wrong value - possibly with wrong type, surely with wrong content because related variables were dropped a long time ago. With varcreate_lsn I can detect that the stored value is obsolete (although it has the same id).

-----------------------------------------------------------------------
I think the latest patch for LET self assign ACL SELECT is not correct,
previously I also tried the same idea.

test demo.
CREATE TYPE vartest_t1 AS (a int, b int);
CREATE VARIABLE var1 AS vartest_t1;
CREATE ROLE regress_var_test_role;
GRANT UPDATE ON VARIABLE var1 TO regress_var_test_role;
GRANT select ON table tenk1 TO regress_var_test_role;
SET ROLE TO regress_var_test_role;

--both should fail
LET var1.a = var1.a + 10;
LET var1.a = (select * from (select count(*) from tenk1 where unique1
= var1.a + 10));

fixed + regress test

--------------------------------------------------------

In today's version I changed GetSessionVariable API little bit. Now it returns only value and *isnull. GetSessionVariableWithTypeCheck is completely removed. Instead I add

GetSessionVariableWithTypeid, but it is introduced in patch allow-parallel-execution-queries-with-session-variab.patch. It is not necessary before. I removed the field varid from structure SessionVariableValue (execnodes.h) because it was not used everywhere.

Regards

Pavel