Feedback-ID: i0fe9450f:Fastmail
Date: Fri, 3 Mar 2023 09:37:27 +0900
From: Michael Paquier <michael@paquier.xyz>
To: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Normalization of utility queries in pg_stat_statements
Message-ID: <ZAFBR1cwIq39ywX+@paquier.xyz>
References: <Y+MRdEq9W9XVa2AB@paquier.xyz>
 <Y+16K+q9L0yjV3zA@paquier.xyz>
 <0e067034-e92c-302b-24ed-492bca228e26@gmail.com>
 <Y+7n+uz6XkzJpB7z@paquier.xyz>
 <14965f63-6d78-45ff-d029-daafe8eddb40@gmail.com>
 <Y/Lbt12YUHSU2fYl@paquier.xyz>
 <Y/LcU/u8CbD+nxYp@paquier.xyz>
 <Y/7Y9U/y/keAW3qH@paquier.xyz>
 <1f84685f-1ffd-4937-de63-aaa1513a0b7e@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="/srFhCKZBALC75ci"
Content-Disposition: inline
In-Reply-To: <1f84685f-1ffd-4937-de63-aaa1513a0b7e@gmail.com>
Archived-At: <https://www.postgresql.org/message-id/ZAFBR1cwIq39ywX%2B%40paquier.xyz>
Precedence: bulk


--/srFhCKZBALC75ci
Content-Type: multipart/mixed; boundary="V9fnfvRasGV5oO8+"
Content-Disposition: inline


--V9fnfvRasGV5oO8+
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Mar 02, 2023 at 08:12:24AM +0100, Drouvot, Bertrand wrote:
> Applying 0001 produces:
>=20
> Applying: Split more regression tests of pg_stat_statements
> .git/rebase-apply/patch:1735: new blank line at EOF.
> +
> .git/rebase-apply/patch:2264: new blank line at EOF.
> +
> warning: 2 lines add whitespace errors.

Indeed, removed.

> What about removing those comments?

Removing these two as well.

> Except from the Nits above, 0001 LGTM.

Thanks for double-checking, applied 0001 to finish this part of the
work.  I am attaching the remaining bits as of the attached, combined
into a single patch.  I am going to look at it again at the beginning
of next week and potentially apply it so as the normalization reflects
to the reports of pg_stat_statements.
--
Michael

--V9fnfvRasGV5oO8+
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v6-0003-Remove-normalization-of-A_Const-nodes.patch"
Content-Transfer-Encoding: quoted-printable

=46rom f96f6a65d5318b059527230d074660ffec099129 Mon Sep 17 00:00:00 2001
=46rom: Michael Paquier <michael@paquier.xyz>
Date: Fri, 3 Mar 2023 09:28:13 +0900
Subject: [PATCH v6] Remove normalization of A_Const nodes

Doing so leads to weird cases with commands that can define a
transaction isolation (SET TRANSACTION and BEGIN), as the normalization
is not able to copy with the full field, yet.

Applying normalization of Const nodes to DDLs changes the states of the
following commands:
- DECLARE
- EXPLAIN
- CREATE MATERIALIZED VIEW
- CTAS

At the end, this should be merged with the previous patch, but keeping
it separate shows the difference of behavior between the two approaches
in the regression tests of pg_stat_statements.
---
 src/include/nodes/parsenodes.h                |  8 +++-
 src/include/nodes/primnodes.h                 |  9 +++--
 .../pg_stat_statements/expected/cursors.out   | 14 +++----
 .../pg_stat_statements/expected/utility.out   | 38 +++++++++----------
 .../pg_stat_statements/pg_stat_statements.c   |  4 +-
 5 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index f7d7f10f7d..259e814253 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3221,14 +3221,18 @@ typedef struct InlineCodeBlock
  * list contains copies of the expressions for all output arguments, in the
  * order of the procedure's declared arguments.  (outargs is never evaluat=
ed,
  * but is useful to the caller as a reference for what to assign to.)
+ * The transformed call state is not relevant in the query jumbling, only =
the
+ * function call is.
  * ----------------------
  */
 typedef struct CallStmt
 {
 	NodeTag		type;
 	FuncCall   *funccall;		/* from the parser */
-	FuncExpr   *funcexpr;		/* transformed call, with only input args */
-	List	   *outargs;		/* transformed output-argument expressions */
+	/* transformed call, with only input args */
+	FuncExpr   *funcexpr pg_node_attr(query_jumble_ignore);
+	/* transformed output-argument expressions */
+	List	   *outargs pg_node_attr(query_jumble_ignore);
 } CallStmt;
=20
 typedef struct CallContext
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index b4292253cc..4220c63ab7 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -128,8 +128,10 @@ typedef struct TableFunc
  * CREATE MATERIALIZED VIEW
  *
  * For CREATE MATERIALIZED VIEW, viewQuery is the parsed-but-not-rewritten
- * SELECT Query for the view; otherwise it's NULL.  (Although it's actually
- * Query*, we declare it as Node* to avoid a forward reference.)
+ * SELECT Query for the view; otherwise it's NULL.  This is irrelevant in
+ * the query jumbling as CreateTableAsStmt already includes a reference to
+ * its own Query, so ignore it.  (Although it's actually Query*, we declare
+ * it as Node* to avoid a forward reference.)
  */
 typedef struct IntoClause
 {
@@ -141,7 +143,8 @@ typedef struct IntoClause
 	List	   *options;		/* options from WITH clause */
 	OnCommitAction onCommit;	/* what do we do at COMMIT? */
 	char	   *tableSpaceName; /* table space to use, or NULL */
-	Node	   *viewQuery;		/* materialized view's SELECT query */
+	/* materialized view's SELECT query */
+	Node	   *viewQuery pg_node_attr(query_jumble_ignore);
 	bool		skipData;		/* true for WITH NO DATA */
 } IntoClause;
=20
diff --git a/contrib/pg_stat_statements/expected/cursors.out b/contrib/pg_s=
tat_statements/expected/cursors.out
index 5d0dc196f9..46375ea905 100644
--- a/contrib/pg_stat_statements/expected/cursors.out
+++ b/contrib/pg_stat_statements/expected/cursors.out
@@ -16,10 +16,10 @@ CLOSE cursor_stats_1;
 DECLARE cursor_stats_1 CURSOR WITH HOLD FOR SELECT 2;
 CLOSE cursor_stats_1;
 SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "=
C";
- calls | rows |                        query                        =20
--------+------+------------------------------------------------------
+ calls | rows |                         query                        =20
+-------+------+-------------------------------------------------------
      2 |    0 | CLOSE cursor_stats_1
-     2 |    0 | DECLARE cursor_stats_1 CURSOR WITH HOLD FOR SELECT 1
+     2 |    0 | DECLARE cursor_stats_1 CURSOR WITH HOLD FOR SELECT $1
      1 |    1 | SELECT pg_stat_statements_reset()
 (3 rows)
=20
@@ -49,14 +49,14 @@ CLOSE cursor_stats_1;
 CLOSE cursor_stats_2;
 COMMIT;
 SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "=
C";
- calls | rows |                        query                        =20
--------+------+------------------------------------------------------
+ calls | rows |                         query                        =20
+-------+------+-------------------------------------------------------
      1 |    0 | BEGIN
      1 |    0 | CLOSE cursor_stats_1
      1 |    0 | CLOSE cursor_stats_2
      1 |    0 | COMMIT
-     1 |    0 | DECLARE cursor_stats_1 CURSOR WITH HOLD FOR SELECT 2
-     1 |    0 | DECLARE cursor_stats_2 CURSOR WITH HOLD FOR SELECT 3
+     1 |    0 | DECLARE cursor_stats_1 CURSOR WITH HOLD FOR SELECT $1
+     1 |    0 | DECLARE cursor_stats_2 CURSOR WITH HOLD FOR SELECT $1
      1 |    1 | FETCH 1 IN cursor_stats_1
      1 |    1 | FETCH 1 IN cursor_stats_2
      1 |    1 | SELECT pg_stat_statements_reset()
diff --git a/contrib/pg_stat_statements/expected/utility.out b/contrib/pg_s=
tat_statements/expected/utility.out
index dbb8f661c0..826ada4eaa 100644
--- a/contrib/pg_stat_statements/expected/utility.out
+++ b/contrib/pg_stat_statements/expected/utility.out
@@ -226,10 +226,10 @@ EXPLAIN (costs off) SELECT a FROM generate_series(1,1=
0) AS tab(a) WHERE a =3D 7;
 (2 rows)
=20
 SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "=
C";
- calls | rows |                                     query                 =
                   =20
--------+------+-----------------------------------------------------------=
--------------------
-     2 |    0 | EXPLAIN (costs off) SELECT 1
-     2 |    0 | EXPLAIN (costs off) SELECT a FROM generate_series(1,10) AS=
 tab(a) WHERE a =3D 3
+ calls | rows |                                      query                =
                     =20
+-------+------+-----------------------------------------------------------=
----------------------
+     2 |    0 | EXPLAIN (costs off) SELECT $1
+     2 |    0 | EXPLAIN (costs off) SELECT a FROM generate_series($1,$2) A=
S tab(a) WHERE a =3D $3
      1 |    1 | SELECT pg_stat_statements_reset()
 (3 rows)
=20
@@ -326,12 +326,12 @@ CREATE TABLE ctas_stats_2 AS
     FROM generate_series(1, 5) AS tab(a) WHERE a < 4 AND a > 1;
 DROP TABLE ctas_stats_2;
 SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "=
C";
- calls | rows |                              query                        =
     =20
--------+------+-----------------------------------------------------------=
------
-     2 |    2 | CREATE TABLE ctas_stats_1 AS SELECT 1 AS a
-     2 |    4 | CREATE TABLE ctas_stats_2 AS                              =
     +
-       |      |   SELECT a AS col1, 2::int AS col2                        =
     +
-       |      |     FROM generate_series(1, 10) AS tab(a) WHERE a < 5 AND =
a > 2
+ calls | rows |                               query                       =
        =20
+-------+------+-----------------------------------------------------------=
---------
+     2 |    2 | CREATE TABLE ctas_stats_1 AS SELECT $1 AS a
+     2 |    4 | CREATE TABLE ctas_stats_2 AS                              =
        +
+       |      |   SELECT a AS col1, $1::int AS col2                       =
        +
+       |      |     FROM generate_series($2, $3) AS tab(a) WHERE a < $4 AN=
D a > $5
      2 |    0 | DROP TABLE ctas_stats_1
      2 |    0 | DROP TABLE ctas_stats_2
      1 |    1 | SELECT pg_stat_statements_reset()
@@ -354,11 +354,11 @@ CREATE MATERIALIZED VIEW matview_stats_1 AS
     FROM generate_series(1, 5) AS tab(a) WHERE a < 4 AND a > 3;
 DROP MATERIALIZED VIEW matview_stats_1;
 SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "=
C";
- calls | rows |                              query                        =
     =20
--------+------+-----------------------------------------------------------=
------
-     2 |    2 | CREATE MATERIALIZED VIEW matview_stats_1 AS               =
     +
-       |      |   SELECT a AS col1, 2::int AS col2                        =
     +
-       |      |     FROM generate_series(1, 10) AS tab(a) WHERE a < 5 AND =
a > 2
+ calls | rows |                               query                       =
        =20
+-------+------+-----------------------------------------------------------=
---------
+     2 |    2 | CREATE MATERIALIZED VIEW matview_stats_1 AS               =
        +
+       |      |   SELECT a AS col1, $1::int AS col2                       =
        +
+       |      |     FROM generate_series($2, $3) AS tab(a) WHERE a < $4 AN=
D a > $5
      2 |    0 | DROP MATERIALIZED VIEW matview_stats_1
      1 |    1 | SELECT pg_stat_statements_reset()
 (3 rows)
@@ -508,19 +508,19 @@ FETCH FORWARD ALL pgss_cursor;
=20
 COMMIT;
 SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "=
C";
- calls | rows |                                   query                   =
                =20
--------+------+-----------------------------------------------------------=
-----------------
+ calls | rows |                                  query                    =
             =20
+-------+------+-----------------------------------------------------------=
--------------
      1 |    0 | BEGIN
      1 |    0 | COMMIT
      1 |    3 | COPY pgss_ctas (a, b) FROM STDIN
      1 |   13 | CREATE MATERIALIZED VIEW pgss_matv AS SELECT * FROM pgss_c=
tas
-     1 |   10 | CREATE TABLE pgss_ctas AS SELECT a, 'ctas' b FROM generate=
_series(1, 10) a
+     1 |   10 | CREATE TABLE pgss_ctas AS SELECT a, $1 b FROM generate_ser=
ies($2, $3) a
      1 |    0 | DECLARE pgss_cursor CURSOR FOR SELECT * FROM pgss_matv
      1 |    5 | FETCH FORWARD 5 pgss_cursor
      1 |    7 | FETCH FORWARD ALL pgss_cursor
      1 |    1 | FETCH NEXT pgss_cursor
      1 |   13 | REFRESH MATERIALIZED VIEW pgss_matv
-     1 |   10 | SELECT generate_series(1, 10) c INTO pgss_select_into
+     1 |   10 | SELECT generate_series($1, $2) c INTO pgss_select_into
      1 |    1 | SELECT pg_stat_statements_reset()
 (12 rows)
=20
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_s=
tat_statements/pg_stat_statements.c
index ad1fe44496..5285c3f7fa 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -836,8 +836,10 @@ pgss_post_parse_analyze(ParseState *pstate, Query *que=
ry, JumbleState *jstate)
 	if (query->utilityStmt)
 	{
 		if (pgss_track_utility && !PGSS_HANDLED_UTILITY(query->utilityStmt))
+		{
 			query->queryId =3D UINT64CONST(0);
-		return;
+			return;
+		}
 	}
=20
 	/*
--=20
2.39.2


--V9fnfvRasGV5oO8+--

--/srFhCKZBALC75ci
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmQBQUcACgkQnvQgOdby
QH1C8Q/9FnQKK2gtLim+SXR+TET9cEw7NEt6DF3QCLca/J6/6b/ZSXMyVce6+Eo4
Po4+dkgoAwumGG/8MaW4YBQGr/53oFTLiRVkReqFuKZoZs6DdQ17O1Va2iloxWhp
r9Y+K7ZwvpmreLzzgwym6sNIz/jHbIAp6H45ctzIMu9PPiN81mrpHBUTnL4ofvFu
tkTfVIsP3R2ts14X9x0i/70wRcJ3lV2Y9CJFRnp69rxWawBF0x6uZtkMK7XXoykR
jlA+Xuo/SE0ALcKXXFMqoo85uyF753rvGdiW+oY1v28cfWBpp9J0KIDGcaBjMuZ+
jSLaC0bb9e/IfzEea2M1C/ZT5PYuHLcihHxQRjxDnjnYVLyqlGcH8o8jmkunYbyy
lnqNDEtd2xDPb5cnUoyegeFpLgzQWb0xURSQEzQ4RrwQw0GgBoNxW9hUtj0xBClA
ZW33k0lhSE5waxAWLpLxPNVvle1rdxCgDFG5ob4MoVmolFDGQx+uA1vWxZvuggOI
JAj1u6p+t2UpW1jsTZCMQaASN24+LlnKx8XMr057g79mnaw4lgD8RRDzC8juVD5v
7l3ZP3mHKG+322KQqmPgD13+gYIrQzjpyhjhd/IPgxw8Te0RBBRqLSnhvILEG3Cx
hnUC9bv4gSD0H6OSQkIT4G5cWcXire9WIsvcXvdrWIfVSv9jRmI=
=QyAC
-----END PGP SIGNATURE-----

--/srFhCKZBALC75ci--