Re: Re: proposal: schema variables

Hi

ne 5. 1. 2025 v 5:52 odesílatel jian he <jian(dot)universality(at)gmail(dot)com>
napsal:

> diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
> index 9c2957eb546..624858db301 100644
> --- a/src/include/nodes/primnodes.h
> +++ b/src/include/nodes/primnodes.h
> @@ -361,6 +361,9 @@ typedef struct Const
> * of the `paramid' field contain the SubLink's subLinkId, and
> * the low-order 16 bits contain the column number. (This type
> * of Param is also converted to PARAM_EXEC during planning.)
> + *
> + * PARAM_VARIABLE: The parameter is a reference to a session variable
> + * (paramid holds the variable's OID).
> */
> typedef enum ParamKind
> {
> @@ -368,6 +371,7 @@ typedef enum ParamKind
> PARAM_EXEC,
> PARAM_SUBLINK,
> PARAM_MULTIEXPR,
> + PARAM_VARIABLE,
> } ParamKind;
>
> typedef struct Param
> @@ -380,6 +384,10 @@ typedef struct Param
> int32 paramtypmod pg_node_attr(query_jumble_ignore);
> /* OID of collation, or InvalidOid if none */
> Oid paramcollid pg_node_attr(query_jumble_ignore);
> + /* OID of session variable if it is used */
> + Oid paramvarid;
> + /* true when param is used like base node of assignment indirection */
> + bool parambasenode;
> /* token location, or -1 if unknown */
> ParseLoc location;
> } Param;
>
> comment should be "(paramvarid holds the variable's OID)"
> also
> + /* true when param is used like base node of assignment indirection */
> is kind of hard to understand.
> param->parambasenode = true;
> only happens in transformLetStmt so we can change it to
> + /* true when param is used in part of the LET statement.*/
>

I don't think the proposed change of comment is better, but the text
can be extended.

<-->/*
<--> * true if param is used as base node of assignment indirection
<--> * (when target of LET statement is an array field or an record field).
<--> * For this param we do not check SELECT access right, because this
<--> * param is used just for execution of UPDATE operation.
<--> */

>
>
> --- a/src/include/executor/execdesc.h
> +++ b/src/include/executor/execdesc.h
> @@ -51,6 +51,10 @@ typedef struct QueryDesc
> /* This field is set by ExecutePlan */
> bool already_executed; /* true if previously executed */
>
> + /* reference to session variables buffer */
> + int num_session_variables;
> + SessionVariableValue *session_variables;
> +
> /* This is always set NULL by the core system, but plugins can change it
> */
> struct Instrumentation *totaltime; /* total time spent in ExecutorRun */
> } QueryDesc;
> QueryDesc->num_session_variables and
> QueryDesc->session_variables are never used in 0001 and 0002.
> it may be used in later patches,
> but at least 0001 and 0002, we don't need to change
> struct QueryDesc?
>
>
moved to patch 17

> contrib/postgres_fdw/deparse.c
> There are some cases of "case T_Param:"
> do we need to do something on it?
>

I think so session variables cannot be executed remotely, and then do
nothing there it is correct.

> also on src/backend/nodes/queryjumblefuncs.c, one
> appearance of "case T_Param:". (but it seems no need change here)
>

CREATE VARIABLE v1 AS int;
> CREATE VARIABLE v2 AS int;
> SELECT pg_stat_statements_reset() IS NOT NULL AS t;
> select count(*) from tenk1 where unique1 = v1;
> select count(*) from tenk1 where unique1 = v2;
>
> should the last two queries be normalized as one query in
> pg_stat_statements?
> if so, then maybe in typedef struct Param
> we need change to:
> Oid paramvarid pg_node_attr(query_jumble_ignore);
>

changed

In this case for this moment we can try to be consistent with plpgsql
variables. So I did it

but then
> let v1 = v1+1;
> let v1 = v2+1;
> will be normalized as one query.
>

yes, but this case is not too important, because at the end (I hope), this
statement inside plpgsql will be executed as
simple expression, and then it will not be processed by pg_stat_statements