Subject: Re: Normalize queries starting with SET for pg_stat_statements

From: Greg Sabino Mullane <[email-masked]>
Date: Tue, 13 Aug 2024 10:54:34 -0400

Now that I've spent some time away from this, I'm reconsidering why we are
going through all the trouble of semi-jumbling SET statements. Maybe we
just keep it simple and everything becomes "SET myvar = $1" or even "SET
myvar" full stop? I'm having a hard time finding a real-world situation in
which we need to distinguish different SET/RESET items within
pg_stat_statements.

Cheers,
Greg

---

From: Michael Paquier <[email-masked]>
Date: Mon, 19 Aug 2024 15:28:52 +0900

On Tue, Aug 13, 2024 at 10:54:34AM -0400, Greg Sabino Mullane wrote:
> myvar" full stop?

Showing a dollar-character to show the fact that we have a value
behind makes the post sense to me.

> I'm having a hard time finding a real-world situation in

I'm -1 on keeping the distinction, and AFAIK it's not really different
with the underlying problems that we need to solve for SET TRANSACTION
and the kind, no? 

FWIW, I'm OK with hiding the value when it comes to a SET clause in a
CREATE FUNCTION.  We already hide the contents of SQL queries inside
the SQL functions when these are queries that can be normalized, so
there is a kind of thin argument for consistency, or something close
to that.
--
Michael

---

From: Michael Paquier <[email-masked]>
Date: Tue, 24 Sep 2024 16:57:28 +0900

> there is a kind of thin argument for consistency, or something close

This thread was one of the things I wanted to look at for this commit
fest because that's an issue I have on my stack for some time.  And
here you go with a revised patch set.

First, the TAP test proposed upthread is not required, so let's remove
it and rely on pg_stat_statements.  It is true that there are a lot of
coverage holes in pg_stat_statements with the various flavors of SET
queries.  The TAP test was able to cover a good part of them, still
missed a few spots.

A second thing is that like you I have settled down to a custom
implementation for VariableSetStmt because we have too many grammar
patterns, some of them with values nested in clauses (SET TIME ZONE is
one example), and we should report the grammar keywords without
hardcoding a location.  Like for the TIME ZONE part, this comes to a
limitation because it is not possible to normalize the case of SET
TIME ZONE 'value' without some refactoring of gram.y.  Perhaps we
could do that in the long-term, but I come down to the fact that I'm
also OK with letting things as they are in the patch, because the
primary case I want to tackle at the SET name = value patterns, and
SET TIME ZONE is just a different flavor of that that can be
translated as well to the most common "name = value" pattern.  A
second case is SET SESSION CHARACTERISTICS AS TRANSACTION with its
list of options.  Contrary to the patch proposed, I don't think that
it is a good idea to hide that arguments may be included in the
jumbling in the custom function, so I have added one field in
VariableSetStmt to do that, and documented why we need the field in
parsenodes.h.  That strikes me as the best balance, and that's going
to be hard to miss each time somebody adds a new grammar for
VariableSetStmt.  That's very unlikely at this stage of the project,
but who knows, people like fancy new features.

Attached are two patches:
- 0001 adds a bunch of tests in pg_stat_statements, to cover the SET
patterns.  (typo in commit message of this patch, will fix later)
- 0002 is the addition of the normalization.  It is possible to see
how the normalization changes things in pg_stat_statements.

0001 is straight-forward and that was I think a mistake to not include
that from the start when I've expanded these tests in the v16 cycle
(well, time..).  0002 also is quite conservative at the end, and this
design can be used to tune easily the jumbling patterns from gram.y
depending on the feedback we'd get.
--
Michael
From 919e03ec0d0673183a9266c1ef16893754005c7e Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Tue, 24 Sep 2024 16:19:08 +0900
Subject: [PATCH v2 1/2] pg_stta_statements: Expand tests with SET grammar

There are many grammar flavors that depend on the parse node
VariableSetStmt.  This closes the gap in pg_stat_statements by providing
test coverage for all of them (well, a large majority of them, at
least).
---
 .../pg_stat_statements/expected/utility.out   | 100 ++++++++++++++++--
 contrib/pg_stat_statements/sql/utility.sql    |  50 ++++++++-
 2 files changed, 140 insertions(+), 10 deletions(-)

diff --git a/contrib/pg_stat_statements/expected/utility.out b/contrib/pg_stat_statements/expected/utility.out
index fa738b5c00..58ba2018c1 100644
--- a/contrib/pg_stat_statements/expected/utility.out
+++ b/contrib/pg_stat_statements/expected/utility.out
@@ -66,7 +66,8 @@ DROP SERVER server_stats;
 DROP FOREIGN DATA WRAPPER wrapper_stats;
 -- Functions
 CREATE FUNCTION func_stats(a text DEFAULT 'a_data', b text DEFAULT lower('b_data'))
-  RETURNS text AS $$ SELECT $1::text || '_' || $2::text; $$ LANGUAGE SQL;
+  RETURNS text AS $$ SELECT $1::text || '_' || $2::text; $$ LANGUAGE SQL
+  SET work_mem = '256kB';
 DROP FUNCTION func_stats;
 -- Rules
 CREATE TABLE tab_rule_stats (a int, b int);
@@ -106,7 +107,8 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
      1 |    0 | CREATE FOREIGN DATA WRAPPER wrapper_stats
      1 |    0 | CREATE FOREIGN TABLE foreign_stats (a int) SERVER server_stats
      1 |    0 | CREATE FUNCTION func_stats(a text DEFAULT 'a_data', b text DEFAULT lower('b_data'))+
-       |      |   RETURNS text AS $$ SELECT $1::text || '_' || $2::text; $$ LANGUAGE SQL
+       |      |   RETURNS text AS $$ SELECT $1::text || '_' || $2::text; $$ LANGUAGE SQL           +
+       |      |   SET work_mem = '256kB'
      1 |    0 | CREATE FUNCTION trigger_func_stats () RETURNS trigger LANGUAGE plpgsql             +
        |      |   AS $$ BEGIN return OLD; end; $$
      1 |    0 | CREATE INDEX pt_stats2_index ON ONLY pt_stats2 (a)
@@ -551,12 +553,26 @@ SELECT pg_stat_statements_reset() IS NOT NULL AS t;
 
 -- SET statements.
 -- These use two different strings, still they count as one entry.
+CREATE ROLE regress_stat_set_1;
+CREATE ROLE regress_stat_set_2;
 SET work_mem = '1MB';
 Set work_mem = '1MB';
 SET work_mem = '2MB';
+SET work_mem = DEFAULT;
+SET work_mem TO DEFAULT;
+SET work_mem FROM CURRENT;
+BEGIN;
+SET LOCAL work_mem = '128kB';
+SET LOCAL work_mem = '256kB';
+SET LOCAL work_mem = DEFAULT;
+SET LOCAL work_mem TO DEFAULT;
+SET LOCAL work_mem FROM CURRENT;
+COMMIT;
 RESET work_mem;
 SET enable_seqscan = off;
 SET enable_seqscan = on;
+SET SESSION work_mem = '300kB';
+SET SESSION work_mem = '400kB';
 RESET enable_seqscan;
 -- SET TRANSACTION ISOLATION
 BEGIN;
@@ -564,33 +580,82 @@ SET TRANSACTION ISOLATION LEVEL READ COMMITTED;
 SET TRANSACTION ISOLATION LEVEL REPEATABLE READ;
 SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
 COMMIT;
--- SET SESSION CHARACTERISTICS
+-- SET SESSION AUTHORIZATION
 SET SESSION SESSION AUTHORIZATION DEFAULT;
+SET SESSION AUTHORIZATION 'regress_stat_set_1';
+SET SESSION AUTHORIZATION 'regress_stat_set_2';
 RESET SESSION AUTHORIZATION;
 BEGIN;
 SET LOCAL SESSION AUTHORIZATION DEFAULT;
+SET LOCAL SESSION AUTHORIZATION 'regress_stat_set_1';
+SET LOCAL SESSION AUTHORIZATION 'regress_stat_set_2';
 RESET SESSION AUTHORIZATION;
 COMMIT;
+-- SET SESSION CHARACTERISTICS
+SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY;
+SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY, READ ONLY;
+SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY, READ WRITE;
+-- SET XML OPTION
+SET XML OPTION DOCUMENT;
+SET XML OPTION CONTENT;
+-- SET TIME ZONE
+SET TIME ZONE 'America/New_York';
+SET TIME ZONE 'Asia/Tokyo';
+SET TIME ZONE DEFAULT;
+SET TIME ZONE LOCAL;
+SET TIME ZONE 'CST7CDT,M4.1.0,M10.5.0';
+RESET TIME ZONE;
+-- SET NAMES -- client_encoding
+SET NAMES 'latin1';
+SET NAMES 'latin2';
 SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
- calls | rows |                       query                        
--------+------+----------------------------------------------------
-     2 |    0 | BEGIN
-     2 |    0 | COMMIT
+ calls | rows |                              query                               
+-------+------+------------------------------------------------------------------
+     3 |    0 | BEGIN
+     3 |    0 | COMMIT
+     1 |    0 | CREATE ROLE regress_stat_set_1
+     1 |    0 | CREATE ROLE regress_stat_set_2
      2 |    0 | RESET SESSION AUTHORIZATION
+     1 |    0 | RESET TIME ZONE
      1 |    0 | RESET enable_seqscan
      1 |    0 | RESET work_mem
      1 |    1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
+     1 |    0 | SET LOCAL SESSION AUTHORIZATION 'regress_stat_set_1'
+     1 |    0 | SET LOCAL SESSION AUTHORIZATION 'regress_stat_set_2'
      1 |    0 | SET LOCAL SESSION AUTHORIZATION DEFAULT
+     1 |    0 | SET LOCAL work_mem = '128kB'
+     1 |    0 | SET LOCAL work_mem = '256kB'
+     2 |    0 | SET LOCAL work_mem = DEFAULT
+     1 |    0 | SET LOCAL work_mem FROM CURRENT
+     1 |    0 | SET NAMES 'latin1'
+     1 |    0 | SET NAMES 'latin2'
+     1 |    0 | SET SESSION AUTHORIZATION 'regress_stat_set_1'
+     1 |    0 | SET SESSION AUTHORIZATION 'regress_stat_set_2'
+     1 |    0 | SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY
+     1 |    0 | SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY, READ ONLY
+     1 |    0 | SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY, READ WRITE
      1 |    0 | SET SESSION SESSION AUTHORIZATION DEFAULT
+     1 |    0 | SET SESSION work_mem = '300kB'
+     1 |    0 | SET SESSION work_mem = '400kB'
+     1 |    0 | SET TIME ZONE 'America/New_York'
+     1 |    0 | SET TIME ZONE 'Asia/Tokyo'
+     1 |    0 | SET TIME ZONE 'CST7CDT,M4.1.0,M10.5.0'
+     2 |    0 | SET TIME ZONE DEFAULT
      1 |    0 | SET TRANSACTION ISOLATION LEVEL READ COMMITTED
      1 |    0 | SET TRANSACTION ISOLATION LEVEL REPEATABLE READ
      1 |    0 | SET TRANSACTION ISOLATION LEVEL SERIALIZABLE
+     1 |    0 | SET XML OPTION CONTENT
+     1 |    0 | SET XML OPTION DOCUMENT
      1 |    0 | SET enable_seqscan = off
      1 |    0 | SET enable_seqscan = on
      2 |    0 | SET work_mem = '1MB'
      1 |    0 | SET work_mem = '2MB'
-(15 rows)
+     2 |    0 | SET work_mem = DEFAULT
+     1 |    0 | SET work_mem FROM CURRENT
+(41 rows)
 
+DROP ROLE regress_stat_set_1;
+DROP ROLE regress_stat_set_2;
 SELECT pg_stat_statements_reset() IS NOT NULL AS t;
  t 
 ---
@@ -664,3 +729,22 @@ SELECT pg_stat_statements_reset() IS NOT NULL AS t;
  t
 (1 row)
 
+-- Special cases.  Keep these ones at the end to avoid issues in this script.
+SET SCHEMA 'foo';
+SET SCHEMA 'public';
+RESET ALL;
+SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
+ calls | rows |                       query                        
+-------+------+----------------------------------------------------
+     1 |    0 | RESET ALL
+     1 |    1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
+     1 |    0 | SET SCHEMA 'foo'
+     1 |    0 | SET SCHEMA 'public'
+(4 rows)
+
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
+(1 row)
+
diff --git a/contrib/pg_stat_statements/sql/utility.sql b/contrib/pg_stat_statements/sql/utility.sql
index 4f7afece1d..20893c6b34 100644
--- a/contrib/pg_stat_statements/sql/utility.sql
+++ b/contrib/pg_stat_statements/sql/utility.sql
@@ -47,7 +47,8 @@ DROP FOREIGN DATA WRAPPER wrapper_stats;
 
 -- Functions
 CREATE FUNCTION func_stats(a text DEFAULT 'a_data', b text DEFAULT lower('b_data'))
-  RETURNS text AS $$ SELECT $1::text || '_' || $2::text; $$ LANGUAGE SQL;
+  RETURNS text AS $$ SELECT $1::text || '_' || $2::text; $$ LANGUAGE SQL
+  SET work_mem = '256kB';
 DROP FUNCTION func_stats;
 
 -- Rules
@@ -277,12 +278,26 @@ SELECT pg_stat_statements_reset() IS NOT NULL AS t;
 
 -- SET statements.
 -- These use two different strings, still they count as one entry.
+CREATE ROLE regress_stat_set_1;
+CREATE ROLE regress_stat_set_2;
 SET work_mem = '1MB';
 Set work_mem = '1MB';
 SET work_mem = '2MB';
+SET work_mem = DEFAULT;
+SET work_mem TO DEFAULT;
+SET work_mem FROM CURRENT;
+BEGIN;
+SET LOCAL work_mem = '128kB';
+SET LOCAL work_mem = '256kB';
+SET LOCAL work_mem = DEFAULT;
+SET LOCAL work_mem TO DEFAULT;
+SET LOCAL work_mem FROM CURRENT;
+COMMIT;
 RESET work_mem;
 SET enable_seqscan = off;
 SET enable_seqscan = on;
+SET SESSION work_mem = '300kB';
+SET SESSION work_mem = '400kB';
 RESET enable_seqscan;
 -- SET TRANSACTION ISOLATION
 BEGIN;
@@ -290,15 +305,38 @@ SET TRANSACTION ISOLATION LEVEL READ COMMITTED;
 SET TRANSACTION ISOLATION LEVEL REPEATABLE READ;
 SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
 COMMIT;
--- SET SESSION CHARACTERISTICS
+-- SET SESSION AUTHORIZATION
 SET SESSION SESSION AUTHORIZATION DEFAULT;
+SET SESSION AUTHORIZATION 'regress_stat_set_1';
+SET SESSION AUTHORIZATION 'regress_stat_set_2';
 RESET SESSION AUTHORIZATION;
 BEGIN;
 SET LOCAL SESSION AUTHORIZATION DEFAULT;
+SET LOCAL SESSION AUTHORIZATION 'regress_stat_set_1';
+SET LOCAL SESSION AUTHORIZATION 'regress_stat_set_2';
 RESET SESSION AUTHORIZATION;
 COMMIT;
+-- SET SESSION CHARACTERISTICS
+SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY;
+SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY, READ ONLY;
+SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY, READ WRITE;
+-- SET XML OPTION
+SET XML OPTION DOCUMENT;
+SET XML OPTION CONTENT;
+-- SET TIME ZONE
+SET TIME ZONE 'America/New_York';
+SET TIME ZONE 'Asia/Tokyo';
+SET TIME ZONE DEFAULT;
+SET TIME ZONE LOCAL;
+SET TIME ZONE 'CST7CDT,M4.1.0,M10.5.0';
+RESET TIME ZONE;
+-- SET NAMES -- client_encoding
+SET NAMES 'latin1';
+SET NAMES 'latin2';
 
 SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
+DROP ROLE regress_stat_set_1;
+DROP ROLE regress_stat_set_2;
 SELECT pg_stat_statements_reset() IS NOT NULL AS t;
 
 --
@@ -329,3 +367,11 @@ DROP TABLE pgss_ctas;
 DROP TABLE pgss_select_into;
 
 SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+
+-- Special cases.  Keep these ones at the end to avoid issues in this script.
+SET SCHEMA 'foo';
+SET SCHEMA 'public';
+RESET ALL;
+SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
-- 
2.45.2

From 167959884d8a336c34617b61e55c591760fd54ab Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Tue, 24 Sep 2024 16:46:58 +0900
Subject: [PATCH v2 2/2] Normalize queries starting with SET

---
 src/include/nodes/parsenodes.h                | 20 +++++++++++++---
 src/backend/nodes/queryjumblefuncs.c          | 19 +++++++++++++++
 src/backend/parser/gram.y                     | 24 +++++++++++++++++++
 contrib/pg_stat_statements/expected/dml.out   |  2 +-
 .../expected/level_tracking.out               |  2 +-
 .../pg_stat_statements/expected/utility.out   | 23 +++++++-----------
 contrib/pg_stat_statements/expected/wal.out   |  2 +-
 7 files changed, 71 insertions(+), 21 deletions(-)

diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index e62ce1b753..dd8238c40f 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2616,11 +2616,25 @@ typedef enum VariableSetKind
 
 typedef struct VariableSetStmt
 {
+	pg_node_attr(custom_query_jumble)
+
 	NodeTag		type;
 	VariableSetKind kind;
-	char	   *name;			/* variable to be set */
-	List	   *args;			/* List of A_Const nodes */
-	bool		is_local;		/* SET LOCAL? */
+	/* variable to be set */
+	char	   *name;
+	/* List of A_Const nodes */
+	List	   *args;
+
+	/*
+	 * True if arguments should be accounted for in query jumbling.  We use a
+	 * separate flag rather than query_jumble_ignore on "args" as several
+	 * grammar flavors of SET rely on a list of values that are parsed
+	 * directly from the grammar's keywords.
+	 */
+	bool		include_args;
+	/* SET LOCAL? */
+	bool		is_local;
+	ParseLoc	location pg_node_attr(query_jumble_location);
 } VariableSetStmt;
 
 /* ----------------------
diff --git a/src/backend/nodes/queryjumblefuncs.c b/src/backend/nodes/queryjumblefuncs.c
index 129fb44709..2db65cb384 100644
--- a/src/backend/nodes/queryjumblefuncs.c
+++ b/src/backend/nodes/queryjumblefuncs.c
@@ -57,6 +57,7 @@ static void RecordConstLocation(JumbleState *jstate, int location);
 static void _jumbleNode(JumbleState *jstate, Node *node);
 static void _jumbleA_Const(JumbleState *jstate, Node *node);
 static void _jumbleList(JumbleState *jstate, Node *node);
+static void _jumbleVariableSetStmt(JumbleState *jstate, Node *node);
 
 /*
  * Given a possibly multi-statement source string, confine our attention to the
@@ -352,3 +353,21 @@ _jumbleA_Const(JumbleState *jstate, Node *node)
 		}
 	}
 }
+
+static void
+_jumbleVariableSetStmt(JumbleState *jstate, Node *node)
+{
+	VariableSetStmt *expr = (VariableSetStmt *) node;
+
+	JUMBLE_FIELD(kind);
+	JUMBLE_STRING(name);
+
+	/*
+	 * Account for the list of arguments in query jumbling only if told by the
+	 * parser.
+	 */
+	if (expr->include_args)
+		JUMBLE_NODE(args);
+	JUMBLE_FIELD(is_local);
+	JUMBLE_LOCATION(location);
+}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index ab304ca989..b451d4db94 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -1647,6 +1647,8 @@ set_rest:
 					n->kind = VAR_SET_MULTI;
 					n->name = "TRANSACTION";
 					n->args = $2;
+					n->include_args = true;
+					n->location = -1;
 					$$ = n;
 				}
 			| SESSION CHARACTERISTICS AS TRANSACTION transaction_mode_list
@@ -1656,6 +1658,8 @@ set_rest:
 					n->kind = VAR_SET_MULTI;
 					n->name = "SESSION CHARACTERISTICS";
 					n->args = $5;
+					n->include_args = true;
+					n->location = -1;
 					$$ = n;
 				}
 			| set_rest_more
@@ -1669,6 +1673,7 @@ generic_set:
 					n->kind = VAR_SET_VALUE;
 					n->name = $1;
 					n->args = $3;
+					n->location = @3;
 					$$ = n;
 				}
 			| var_name '=' var_list
@@ -1678,6 +1683,7 @@ generic_set:
 					n->kind = VAR_SET_VALUE;
 					n->name = $1;
 					n->args = $3;
+					n->location = @3;
 					$$ = n;
 				}
 			| var_name TO DEFAULT
@@ -1686,6 +1692,7 @@ generic_set:
 
 					n->kind = VAR_SET_DEFAULT;
 					n->name = $1;
+					n->location = -1;
 					$$ = n;
 				}
 			| var_name '=' DEFAULT
@@ -1694,6 +1701,7 @@ generic_set:
 
 					n->kind = VAR_SET_DEFAULT;
 					n->name = $1;
+					n->location = -1;
 					$$ = n;
 				}
 		;
@@ -1706,6 +1714,7 @@ set_rest_more:	/* Generic SET syntaxes: */
 
 					n->kind = VAR_SET_CURRENT;
 					n->name = $1;
+					n->location = -1;
 					$$ = n;
 				}
 			/* Special syntaxes mandated by SQL standard: */
@@ -1715,6 +1724,8 @@ set_rest_more:	/* Generic SET syntaxes: */
 
 					n->kind = VAR_SET_VALUE;
 					n->name = "timezone";
+					n->location = -1;
+					n->include_args = true;
 					if ($3 != NULL)
 						n->args = list_make1($3);
 					else
@@ -1736,6 +1747,7 @@ set_rest_more:	/* Generic SET syntaxes: */
 					n->kind = VAR_SET_VALUE;
 					n->name = "search_path";
 					n->args = list_make1(makeStringConst($2, @2));
+					n->location = @2;
 					$$ = n;
 				}
 			| NAMES opt_encoding
@@ -1744,6 +1756,7 @@ set_rest_more:	/* Generic SET syntaxes: */
 
 					n->kind = VAR_SET_VALUE;
 					n->name = "client_encoding";
+					n->location = @2;
 					if ($2 != NULL)
 						n->args = list_make1(makeStringConst($2, @2));
 					else
@@ -1757,6 +1770,7 @@ set_rest_more:	/* Generic SET syntaxes: */
 					n->kind = VAR_SET_VALUE;
 					n->name = "role";
 					n->args = list_make1(makeStringConst($2, @2));
+					n->location = @2;
 					$$ = n;
 				}
 			| SESSION AUTHORIZATION NonReservedWord_or_Sconst
@@ -1766,6 +1780,7 @@ set_rest_more:	/* Generic SET syntaxes: */
 					n->kind = VAR_SET_VALUE;
 					n->name = "session_authorization";
 					n->args = list_make1(makeStringConst($3, @3));
+					n->location = @3;
 					$$ = n;
 				}
 			| SESSION AUTHORIZATION DEFAULT
@@ -1774,6 +1789,7 @@ set_rest_more:	/* Generic SET syntaxes: */
 
 					n->kind = VAR_SET_DEFAULT;
 					n->name = "session_authorization";
+					n->location = -1;
 					$$ = n;
 				}
 			| XML_P OPTION document_or_content
@@ -1783,6 +1799,8 @@ set_rest_more:	/* Generic SET syntaxes: */
 					n->kind = VAR_SET_VALUE;
 					n->name = "xmloption";
 					n->args = list_make1(makeStringConst($3 == XMLOPTION_DOCUMENT ? "DOCUMENT" : "CONTENT", @3));
+					n->include_args = true;
+					n->location = -1;
 					$$ = n;
 				}
 			/* Special syntaxes invented by PostgreSQL: */
@@ -1793,6 +1811,7 @@ set_rest_more:	/* Generic SET syntaxes: */
 					n->kind = VAR_SET_MULTI;
 					n->name = "TRANSACTION SNAPSHOT";
 					n->args = list_make1(makeStringConst($3, @3));
+					n->location = @3;
 					$$ = n;
 				}
 		;
@@ -1900,6 +1919,7 @@ reset_rest:
 
 					n->kind = VAR_RESET;
 					n->name = "timezone";
+					n->location = -1;
 					$$ = n;
 				}
 			| TRANSACTION ISOLATION LEVEL
@@ -1908,6 +1928,7 @@ reset_rest:
 
 					n->kind = VAR_RESET;
 					n->name = "transaction_isolation";
+					n->location = -1;
 					$$ = n;
 				}
 			| SESSION AUTHORIZATION
@@ -1916,6 +1937,7 @@ reset_rest:
 
 					n->kind = VAR_RESET;
 					n->name = "session_authorization";
+					n->location = -1;
 					$$ = n;
 				}
 		;
@@ -1927,6 +1949,7 @@ generic_reset:
 
 					n->kind = VAR_RESET;
 					n->name = $1;
+					n->location = -1;
 					$$ = n;
 				}
 			| ALL
@@ -1934,6 +1957,7 @@ generic_reset:
 					VariableSetStmt *n = makeNode(VariableSetStmt);
 
 					n->kind = VAR_RESET_ALL;
+					n->location = -1;
 					$$ = n;
 				}
 		;
diff --git a/contrib/pg_stat_statements/expected/dml.out b/contrib/pg_stat_statements/expected/dml.out
index f6ac8da5ca..acc2c5e524 100644
--- a/contrib/pg_stat_statements/expected/dml.out
+++ b/contrib/pg_stat_statements/expected/dml.out
@@ -82,7 +82,7 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
      2 |    4 | SELECT * FROM pgss_dml_tab WHERE a > $1 ORDER BY a
      1 |    8 | SELECT * FROM pgss_dml_tab WHERE a IN ($1, $2, $3, $4, $5)
      1 |    1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
-     1 |    0 | SET pg_stat_statements.track_utility = FALSE
+     1 |    0 | SET pg_stat_statements.track_utility = $1
      6 |    6 | UPDATE pgss_dml_tab SET b = $1 WHERE a = $2
      1 |    3 | UPDATE pgss_dml_tab SET b = $1 WHERE a > $2
 (10 rows)
diff --git a/contrib/pg_stat_statements/expected/level_tracking.out b/contrib/pg_stat_statements/expected/level_tracking.out
index d8dd8a2dee..bb65e98ce0 100644
--- a/contrib/pg_stat_statements/expected/level_tracking.out
+++ b/contrib/pg_stat_statements/expected/level_tracking.out
@@ -64,7 +64,7 @@ SELECT toplevel, calls, query FROM pg_stat_statements
           |       | END; $$
  f        |     1 | SELECT $1::TEXT
  t        |     1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
- t        |     1 | SET pg_stat_statements.track = 'all'
+ t        |     1 | SET pg_stat_statements.track = $1
 (7 rows)
 
 -- Procedure with multiple utility statements.
diff --git a/contrib/pg_stat_statements/expected/utility.out b/contrib/pg_stat_statements/expected/utility.out
index 58ba2018c1..3d58543fdf 100644
--- a/contrib/pg_stat_statements/expected/utility.out
+++ b/contrib/pg_stat_statements/expected/utility.out
@@ -108,7 +108,7 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
      1 |    0 | CREATE FOREIGN TABLE foreign_stats (a int) SERVER server_stats
      1 |    0 | CREATE FUNCTION func_stats(a text DEFAULT 'a_data', b text DEFAULT lower('b_data'))+
        |      |   RETURNS text AS $$ SELECT $1::text || '_' || $2::text; $$ LANGUAGE SQL           +
-       |      |   SET work_mem = '256kB'
+       |      |   SET work_mem = $1
      1 |    0 | CREATE FUNCTION trigger_func_stats () RETURNS trigger LANGUAGE plpgsql             +
        |      |   AS $$ BEGIN return OLD; end; $$
      1 |    0 | CREATE INDEX pt_stats2_index ON ONLY pt_stats2 (a)
@@ -623,20 +623,16 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
      1 |    0 | SET LOCAL SESSION AUTHORIZATION 'regress_stat_set_1'
      1 |    0 | SET LOCAL SESSION AUTHORIZATION 'regress_stat_set_2'
      1 |    0 | SET LOCAL SESSION AUTHORIZATION DEFAULT
-     1 |    0 | SET LOCAL work_mem = '128kB'
-     1 |    0 | SET LOCAL work_mem = '256kB'
+     2 |    0 | SET LOCAL work_mem = $1
      2 |    0 | SET LOCAL work_mem = DEFAULT
      1 |    0 | SET LOCAL work_mem FROM CURRENT
-     1 |    0 | SET NAMES 'latin1'
-     1 |    0 | SET NAMES 'latin2'
+     2 |    0 | SET NAMES $1
      1 |    0 | SET SESSION AUTHORIZATION 'regress_stat_set_1'
      1 |    0 | SET SESSION AUTHORIZATION 'regress_stat_set_2'
      1 |    0 | SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY
      1 |    0 | SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY, READ ONLY
      1 |    0 | SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY, READ WRITE
      1 |    0 | SET SESSION SESSION AUTHORIZATION DEFAULT
-     1 |    0 | SET SESSION work_mem = '300kB'
-     1 |    0 | SET SESSION work_mem = '400kB'
      1 |    0 | SET TIME ZONE 'America/New_York'
      1 |    0 | SET TIME ZONE 'Asia/Tokyo'
      1 |    0 | SET TIME ZONE 'CST7CDT,M4.1.0,M10.5.0'
@@ -646,13 +642,11 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
      1 |    0 | SET TRANSACTION ISOLATION LEVEL SERIALIZABLE
      1 |    0 | SET XML OPTION CONTENT
      1 |    0 | SET XML OPTION DOCUMENT
-     1 |    0 | SET enable_seqscan = off
-     1 |    0 | SET enable_seqscan = on
-     2 |    0 | SET work_mem = '1MB'
-     1 |    0 | SET work_mem = '2MB'
+     2 |    0 | SET enable_seqscan = $1
+     5 |    0 | SET work_mem = $1
      2 |    0 | SET work_mem = DEFAULT
      1 |    0 | SET work_mem FROM CURRENT
-(41 rows)
+(35 rows)
 
 DROP ROLE regress_stat_set_1;
 DROP ROLE regress_stat_set_2;
@@ -738,9 +732,8 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
 -------+------+----------------------------------------------------
      1 |    0 | RESET ALL
      1 |    1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
-     1 |    0 | SET SCHEMA 'foo'
-     1 |    0 | SET SCHEMA 'public'
-(4 rows)
+     2 |    0 | SET SCHEMA $1
+(3 rows)
 
 SELECT pg_stat_statements_reset() IS NOT NULL AS t;
  t 
diff --git a/contrib/pg_stat_statements/expected/wal.out b/contrib/pg_stat_statements/expected/wal.out
index 34a2bf5b03..977e382d84 100644
--- a/contrib/pg_stat_statements/expected/wal.out
+++ b/contrib/pg_stat_statements/expected/wal.out
@@ -18,7 +18,7 @@ FROM pg_stat_statements ORDER BY query COLLATE "C";
  DELETE FROM pgss_wal_tab WHERE a > $1                        |     1 |    1 | t                   | t                     | t
  INSERT INTO pgss_wal_tab VALUES(generate_series($1, $2), $3) |     1 |   10 | t                   | t                     | t
  SELECT pg_stat_statements_reset() IS NOT NULL AS t           |     1 |    1 | f                   | f                     | f
- SET pg_stat_statements.track_utility = FALSE                 |     1 |    0 | f                   | f                     | t
+ SET pg_stat_statements.track_utility = $1                    |     1 |    0 | f                   | f                     | t
  UPDATE pgss_wal_tab SET b = $1 WHERE a > $2                  |     1 |    3 | t                   | t                     | t
 (5 rows)
 
-- 
2.45.2

---

From: Michael Paquier <[email-masked]>
Date: Wed, 25 Sep 2024 12:10:02 +0900

> depending on the feedback we'd get.

Applied 0001 for now to expand the tests, with one tweak: the removal
of SET NAMES.  It was proving tricky to use something else than UTF-8,
the CI complaining on Windows.  True that this could use like unaccent
and an alternate output in a different file, but I'm not inclined to
take the cost just for this specific query pattern.

The remaining 0002 is attached for now.  I am planning to wrap that
next week after a second lookup, except if there are any comments, of
course.
--
Michael
From 40983157423d93334f30c70def5bf311fcff80f6 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Wed, 25 Sep 2024 12:08:04 +0900
Subject: [PATCH v3] Normalize queries starting with SET

---
 src/include/nodes/parsenodes.h                | 20 +++++++++++++---
 src/backend/nodes/queryjumblefuncs.c          | 19 +++++++++++++++
 src/backend/parser/gram.y                     | 24 +++++++++++++++++++
 contrib/pg_stat_statements/expected/dml.out   |  2 +-
 .../expected/level_tracking.out               |  2 +-
 .../pg_stat_statements/expected/utility.out   | 20 ++++++----------
 contrib/pg_stat_statements/expected/wal.out   |  2 +-
 7 files changed, 70 insertions(+), 19 deletions(-)

index e62ce1b753..9e76713f2d 100644
 
+	bool		jumble_args;
 
index 129fb44709..5e43fd9229 100644
 
 /*
+	if (expr->jumble_args)
index b1d4642c59..4aa8646af7 100644
+					n->jumble_args = true;
+					n->jumble_args = true;

 
 
 					n->kind = VAR_SET_VALUE;
 					n->name = "timezone";
+					n->location = -1;
+					n->jumble_args = true;
 					if ($3 != NULL)
 
 					n->kind = VAR_SET_VALUE;
 
+					n->jumble_args = true;
 
 					n->kind = VAR_RESET;
 					n->name = "timezone";
+					n->location = -1;
 					$$ = n;
 				}
 
 					n->kind = VAR_RESET;
 
 					n->kind = VAR_RESET;
 
 					n->kind = VAR_RESET;
 					n->name = $1;
+					n->location = -1;
 					$$ = n;
 				}
 			| ALL
 
 					n->kind = VAR_RESET_ALL;
+					n->location = -1;
 					$$ = n;
 				}
 		;
 
index 8d3fd77201..060d4416dd 100644
@@ -620,8 +620,7 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
@@ -630,8 +629,6 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
@@ -641,13 +638,11 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
-(39 rows)
+(34 rows)
 
@@ -733,9 +728,8 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";

-- 
2.45.2

---

From: Michael Paquier <[email-masked]>
Date: Mon, 30 Sep 2024 15:24:04 +0900

> next week after a second lookup, except if there are any comments, of

And done with that, after a second round, tweaking some comments.

Thanks Greg for sending the patch and pushing this feature forward.
This finishes what I had on my TODO bucket in terms of normalization
when this stuff has begun in v16 with 3db72ebcbe20.
--
Michael