Re: Re: proposal: schema variables

Hi

st 8. 1. 2025 v 10:31 odesílatel jian he <jian(dot)universality(at)gmail(dot)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