Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1tUier-0028rh-OY for pgsql-hackers@arkaria.postgresql.org; Mon, 06 Jan 2025 08:40:22 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1tUier-006byq-3c for pgsql-hackers@arkaria.postgresql.org; Mon, 06 Jan 2025 08:40:20 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1tUieq-006byi-Mb for pgsql-hackers@lists.postgresql.org; Mon, 06 Jan 2025 08:40:20 +0000 Received: from mail-yb1-xb29.google.com ([2607:f8b0:4864:20::b29]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.96) (envelope-from ) id 1tUieo-0007Xp-0A for pgsql-hackers@lists.postgresql.org; Mon, 06 Jan 2025 08:40:19 +0000 Received: by mail-yb1-xb29.google.com with SMTP id 3f1490d57ef6-e479e529ebcso18206616276.3 for ; Mon, 06 Jan 2025 00:40:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1736152817; x=1736757617; darn=lists.postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=lQxMV6eyKeKyZjSLWJV7s4i458C7/2n9WLqGQ2U3RAE=; b=DekOqgPH8Ufqkl7lrkjZv4YwUhldiaJDdRbtKYGD1bgrFqCRntHikxYFzkY96D7Jvr 5CS3LxIYJkGyY+EyIBWGd+rzfSBO3EEQbt1VMufAz4NYWpRVSY0BwKcV591y+9twxEyc yUT6JvbSfvJC164K40iNAqOOXFYhFVOTla+gnk/h7Q3uoGiLk+V//CkHmGEEcmnVeKaY BvDQA/UQl1naI5mCAcfUWYYxHWtcsZPZHkxBm6uuGcXN4vjJjZ4uoo6tMcnXYnibKDzi w0hH+BtONinR9Bi686lBuo8bIsciVzmJek2ZwoD9erBKvF2VijpwCJdP2pK0AN5bk1Fn PkCA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736152817; x=1736757617; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=lQxMV6eyKeKyZjSLWJV7s4i458C7/2n9WLqGQ2U3RAE=; b=bWZrbSBHW+RkB5HhFXrQHK5rMKqAcvq2ewDK9hOOFlJ2nEWZp9o0mD58u7dpkc6xoy HVKSuFuMMKZa4D7Dbif+3pfAcEHcwhJJndV7xovP/98pGqY+gdbhgR1FkL8YpWQShO1y e2F4ydbRyzuYoBtIq0vN4avBwXh/tPdz1+L8TLBMze7CvfAIs7Exc1lvOvPu2hQrnTOB 97USKhHvaqZ70ef/JZ+9cZYEt/gCO3UUvdg2E+v5JF2+1SXkSVkvYzHgUD1a+nTEmkMV RjObcnedpya15xFvXL3q46JKruO1z12ngv9dmGAfvCjrojVdluYxfeq7sHF7DblthexG vJOw== X-Forwarded-Encrypted: i=1; AJvYcCXxUFFZn3tf4iZaeXf1grSHe+BoV9SmfmPd6lxEstXIzXS3ktgBXpOiB1i6zWYomuzZkbieedvgGgCbJXfT@lists.postgresql.org X-Gm-Message-State: AOJu0Yy2ndwtKDqb6cs4YlaeQQxlmVcU/Ud+bWiMog4BR2rbkb+Y1NDR XY9a7F5Y4ITT12MIzc3suuUZXfxNjOw2jvNoiJWv3Abj5sfBc0rAYklj47W82p0Up049OMcD/dp y75gZa8rSdtXBobIEK1Y0SLs1dns= X-Gm-Gg: ASbGncuubgQZ3dN0dPE96znjLRdqekHN24VDdD0T8hYRqSlZO675TchWWcoQu6364MQ NsTB9HvMBS/Xloj8pJUtLtXKz3vPk81WJ18tVQLUaizyC+gDX5rSyGNmEE0zKIzoKuwCHZIA= X-Google-Smtp-Source: AGHT+IFzT5YGv+sMdVkhxulRdDGhA2e6qOcwDLvWPvxbuyD7Qj29IKtffjZFS4X8z4ZWGqw4xtDLZpftAMMHnhG9Stk= X-Received: by 2002:a05:690c:f84:b0:6ef:993a:29b4 with SMTP id 00721157ae682-6f3f7f2b8bamr416501647b3.0.1736152817645; Mon, 06 Jan 2025 00:40:17 -0800 (PST) MIME-Version: 1.0 References: <3chredgnjcmccym2kczawfih226b4ac6co7p6z4jeofevrcosi@mrsxkx2x2c65> <20241120201313.t4wbhld4ktgielaf@erthalion.local> In-Reply-To: From: Pavel Stehule Date: Mon, 6 Jan 2025 09:39:39 +0100 Message-ID: Subject: Re: Re: proposal: schema variables To: jian he Cc: Dmitry Dolgov <9erthalion6@gmail.com>, Laurenz Albe , Erik Rijkers , Michael Paquier , Amit Kapila , DUVAL REMI , PostgreSQL Hackers Content-Type: multipart/alternative; boundary="000000000000f8c05b062b059344" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --000000000000f8c05b062b059344 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi ne 5. 1. 2025 v 5:52 odes=C3=ADlatel jian he 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 =3D 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 i= t > */ > 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 =3D v1; > select count(*) from tenk1 where unique1 =3D 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 =3D v1+1; > let v1 =3D 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 --000000000000f8c05b062b059344 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi

ne 5. 1. 2025 v=C2=A05:52 odes= =C3=ADlatel jian he <jian.universality@gmail.com> napsal:
diff --git a/src/include/nodes/pr= imnodes.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
=C2=A0 * of the `paramid' field contain the SubLink's subLinkId, an= d
=C2=A0 * the low-order 16 bits contain the column number.=C2=A0 (This type<= br> =C2=A0 * of Param is also converted to PARAM_EXEC during planning.)
+ *
+ * PARAM_VARIABLE:=C2=A0 The parameter is a reference to a session variabl= e
+ * (paramid holds the variable's OID).
=C2=A0 */
=C2=A0typedef enum ParamKind
=C2=A0{
@@ -368,6 +371,7 @@ typedef enum ParamKind
=C2=A0 PARAM_EXEC,
=C2=A0 PARAM_SUBLINK,
=C2=A0 PARAM_MULTIEXPR,
+ PARAM_VARIABLE,
=C2=A0} ParamKind;

=C2=A0typedef struct Param
@@ -380,6 +384,10 @@ typedef struct Param
=C2=A0 int32 paramtypmod pg_node_attr(query_jumble_ignore);
=C2=A0 /* OID of collation, or InvalidOid if none */
=C2=A0 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;
=C2=A0 /* token location, or -1 if unknown */
=C2=A0 ParseLoc location;
=C2=A0} 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 =3D 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 b= etter, 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 executio= n of UPDATE operation.
<--> */

=C2=A0
=


--- a/src/include/executor/execdesc.h
+++ b/src/include/executor/execdesc.h
@@ -51,6 +51,10 @@ typedef struct QueryDesc
=C2=A0 /* This field is set by ExecutePlan */
=C2=A0 bool already_executed; /* true if previously executed */

+ /* reference to session variables buffer */
+ int num_session_variables;
+ SessionVariableValue *session_variables;
+
=C2=A0 /* This is always set NULL by the core system, but plugins can chang= e it */
=C2=A0 struct Instrumentation *totaltime; /* total time spent in ExecutorRu= n */
=C2=A0} 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 thi= nk so session variables cannot be executed remotely, and then do nothing th= ere it is correct.
=C2=A0
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 =3D v1;
select count(*) from tenk1 where unique1 =3D v2;

should the last two queries be normalized as one query in pg_stat_statement= s?
if so, then maybe in typedef struct Param
we need change to:
Oid=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 paramvarid pg_node_attr(query_= jumble_ignore);

changed

=
In this case for this moment we can try to be consistent with pl= pgsql variables.=C2=A0=C2=A0 So I did it

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

yes, b= ut this case is not too important, because at the end (I hope), this statem= ent inside plpgsql will be executed as
simple expression, an= d then it will not be processed by pg_stat_statements

<= div>
=C2=A0
--000000000000f8c05b062b059344--