Feedback-ID: i0fe9450f:Fastmail
Date: Tue, 25 Mar 2025 16:02:08 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Lukas Fittl <lukas@fittl.com>
Cc: Sami Imseih <samimseih@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
	Christoph Berg <myon@debian.org>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
	ma lz <ma100@hotmail.com>
Subject: Re: query_id: jumble names of temp tables for better
 pg_stat_statement UX
Message-ID: <Z-JU8EmGdlnNEGot@paquier.xyz>
References: <1831838.1742656359@sss.pgh.pa.us>
 <80506.1742660683@sss.pgh.pa.us>
 <Z99aIy9T0f-lulK5@paquier.xyz>
 <461405.1742691859@sss.pgh.pa.us>
 <Z9-sUvq8pE5uinEu@paquier.xyz>
 <CAA5RZ0t4XQ_SANiJ5VfuoKnAuxMaY2ggQWoGKkHo+U2H+Sh-Sw@mail.gmail.com>
 <1189112.1742869660@sss.pgh.pa.us>
 <CAA5RZ0uNofEXfEfNw3uRN3D3oXkFPQ_s+huLLHMKR_+MCk8RPQ@mail.gmail.com>
 <Z-IoMKL8I-r-xsuW@paquier.xyz>
 <CAP53PkzAPrHNHquT76mVjfStrz0B2hKhF+CbAZ_w9QJUJ_1oJQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="LMbqH2q/oNMLq+Li"
Content-Disposition: inline
In-Reply-To: <CAP53PkzAPrHNHquT76mVjfStrz0B2hKhF+CbAZ_w9QJUJ_1oJQ@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/Z-JU8EmGdlnNEGot%40paquier.xyz>
Precedence: bulk


--LMbqH2q/oNMLq+Li
Content-Type: multipart/mixed; boundary="CdIaZbBgfMcoFjU3"
Content-Disposition: inline


--CdIaZbBgfMcoFjU3
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Mon, Mar 24, 2025 at 11:09:06PM -0700, Lukas Fittl wrote:
> For what its worth, +1 on the current proposal in this thread (and doing it
> without a GUC), i.e. merging a query that references the same table alias,
> ignoring different schemas.

Thanks for the feedback.  I have looked again at the first patch to
add custom_query_jumble as a node field attribute, adjusted some
comments, and applied it as 5ac462e2b7ac.

Attached is the second one, with more tests coverage with attribute
aliases (these being ignored exists in stable branches, but why not
while on it) and table aliases, and the fixes for the issues pointed
out by Christoph.  I'll double-check all that again tomorrow. Please
find an updated version attached for now.
--
Michael

--CdIaZbBgfMcoFjU3
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v5-0001-Add-custom-query-jumble-function-for-RangeTblEntr.patch"
Content-Transfer-Encoding: quoted-printable

=46rom 68d363fbee484b40308a00a85329364ff0901e9b Mon Sep 17 00:00:00 2001
=46rom: Michael Paquier <michael@paquier.xyz>
Date: Tue, 25 Mar 2025 15:40:10 +0900
Subject: [PATCH v5] Add custom query jumble function for RangeTblEntry.eref

---
 src/include/nodes/parsenodes.h                |  11 +-
 src/backend/nodes/queryjumblefuncs.c          |  19 ++
 .../pg_stat_statements/expected/select.out    | 236 ++++++++++++++++++
 contrib/pg_stat_statements/sql/select.sql     |  69 +++++
 4 files changed, 332 insertions(+), 3 deletions(-)

diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 23c9e3c5abf2..a87f949b389e 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1050,8 +1050,13 @@ typedef struct RangeTblEntry
 	 */
 	/* user-written alias clause, if any */
 	Alias	   *alias pg_node_attr(query_jumble_ignore);
-	/* expanded reference names */
-	Alias	   *eref pg_node_attr(query_jumble_ignore);
+
+	/*
+	 * Expanded reference names.  This uses a custom query jumble function so
+	 * as the table name is included in the computation, not its list of
+	 * columns.
+	 */
+	Alias	   *eref pg_node_attr(custom_query_jumble);
=20
 	RTEKind		rtekind;		/* see above */
=20
@@ -1094,7 +1099,7 @@ typedef struct RangeTblEntry
 	 * tables to be invalidated if the underlying table is altered.
 	 */
 	/* OID of the relation */
-	Oid			relid;
+	Oid			relid pg_node_attr(query_jumble_ignore);
 	/* inheritance requested? */
 	bool		inh;
 	/* relation kind (see pg_class.relkind) */
diff --git a/src/backend/nodes/queryjumblefuncs.c b/src/backend/nodes/query=
jumblefuncs.c
index f8b0f91704ba..62d6cfb7ac15 100644
--- a/src/backend/nodes/queryjumblefuncs.c
+++ b/src/backend/nodes/queryjumblefuncs.c
@@ -67,6 +67,9 @@ static void _jumbleElements(JumbleState *jstate, List *el=
ements);
 static void _jumbleA_Const(JumbleState *jstate, Node *node);
 static void _jumbleList(JumbleState *jstate, Node *node);
 static void _jumbleVariableSetStmt(JumbleState *jstate, Node *node);
+static void _jumbleRangeTblEntry_eref(JumbleState *jstate,
+									  RangeTblEntry *rte,
+									  Alias *expr);
=20
 /*
  * Given a possibly multi-statement source string, confine our attention t=
o the
@@ -516,3 +519,19 @@ _jumbleVariableSetStmt(JumbleState *jstate, Node *node)
 	JUMBLE_FIELD(is_local);
 	JUMBLE_LOCATION(location);
 }
+
+/*
+ * Custom query jumble function for RangeTblEntry.eref.
+ */
+static void
+_jumbleRangeTblEntry_eref(JumbleState *jstate,
+						  RangeTblEntry *rte,
+						  Alias *expr)
+{
+	JUMBLE_FIELD(type);
+
+	/*
+	 * This includes only the table name, the list of column names is ignored.
+	 */
+	JUMBLE_STRING(aliasname);
+}
diff --git a/contrib/pg_stat_statements/expected/select.out b/contrib/pg_st=
at_statements/expected/select.out
index 37a30af034a6..bf05d521e866 100644
--- a/contrib/pg_stat_statements/expected/select.out
+++ b/contrib/pg_stat_statements/expected/select.out
@@ -413,3 +413,239 @@ SELECT pg_stat_statements_reset() IS NOT NULL AS t;
  t
 (1 row)
=20
+-- Temporary tables, grouped together.
+BEGIN;
+  CREATE TEMP TABLE temp_t (id int) ON COMMIT DROP;
+  SELECT * FROM temp_t;
+ id=20
+----
+(0 rows)
+
+COMMIT;
+BEGIN;
+  CREATE TEMP TABLE temp_t (id int) ON COMMIT DROP;
+  SELECT * FROM temp_t;
+ id=20
+----
+(0 rows)
+
+COMMIT;
+SELECT calls, query FROM pg_stat_statements ORDER BY query COLLATE "C";
+ calls |                                 query                            =
     =20
+-------+------------------------------------------------------------------=
------
+     2 | SELECT * FROM temp_t
+     0 | SELECT calls, query FROM pg_stat_statements ORDER BY query COLLAT=
E "C"
+     1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
+(3 rows)
+
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t=20
+---
+ t
+(1 row)
+
+-- search_path with various schemas and temporary tables
+CREATE SCHEMA pgss_schema_1;
+CREATE SCHEMA pgss_schema_2;
+-- Same attributes.
+CREATE TABLE pgss_schema_1.tab_search_same (a int, b int);
+CREATE TABLE pgss_schema_2.tab_search_same (a int, b int);
+CREATE TEMP TABLE tab_search_same (a int, b int);
+-- Different number of attributes, mapping types
+CREATE TABLE pgss_schema_1.tab_search_diff_1 (a int);
+CREATE TABLE pgss_schema_2.tab_search_diff_1 (a int, b int);
+CREATE TEMP TABLE tab_search_diff_1 (a int, b int, c int);
+-- Same number of attributes, different types
+CREATE TABLE pgss_schema_1.tab_search_diff_2 (a int);
+CREATE TABLE pgss_schema_2.tab_search_diff_2 (a text);
+CREATE TEMP TABLE tab_search_diff_2 (a bigint);
+-- First permanent schema
+SET search_path =3D 'pgss_schema_1';
+SELECT count(*) FROM tab_search_same;
+ count=20
+-------
+     0
+(1 row)
+
+SELECT a, b FROM tab_search_same;
+ a | b=20
+---+---
+(0 rows)
+
+SELECT count(*) FROM tab_search_diff_1;
+ count=20
+-------
+     0
+(1 row)
+
+SELECT count(*) FROM tab_search_diff_2;
+ count=20
+-------
+     0
+(1 row)
+
+SELECT a FROM tab_search_diff_2 AS t1;
+ a=20
+---
+(0 rows)
+
+SELECT a FROM tab_search_diff_2;
+ a=20
+---
+(0 rows)
+
+SELECT a AS a1 FROM tab_search_diff_2;
+ a1=20
+----
+(0 rows)
+
+-- Second permanent schema
+SET search_path =3D 'pgss_schema_2';
+SELECT count(*) FROM tab_search_same;
+ count=20
+-------
+     0
+(1 row)
+
+SELECT a, b FROM tab_search_same;
+ a | b=20
+---+---
+(0 rows)
+
+SELECT count(*) FROM tab_search_diff_1;
+ count=20
+-------
+     0
+(1 row)
+
+SELECT count(*) FROM tab_search_diff_2;
+ count=20
+-------
+     0
+(1 row)
+
+SELECT a FROM tab_search_diff_2 AS t1;
+ a=20
+---
+(0 rows)
+
+SELECT a FROM tab_search_diff_2;
+ a=20
+---
+(0 rows)
+
+SELECT a AS a1 FROM tab_search_diff_2;
+ a1=20
+----
+(0 rows)
+
+-- Temporary schema
+SET search_path =3D 'pg_temp';
+SELECT count(*) FROM tab_search_same;
+ count=20
+-------
+     0
+(1 row)
+
+SELECT a, b FROM tab_search_same;
+ a | b=20
+---+---
+(0 rows)
+
+SELECT count(*) FROM tab_search_diff_1;
+ count=20
+-------
+     0
+(1 row)
+
+SELECT count(*) FROM tab_search_diff_2;
+ count=20
+-------
+     0
+(1 row)
+
+SELECT a FROM tab_search_diff_2 AS t1;
+ a=20
+---
+(0 rows)
+
+SELECT a FROM tab_search_diff_2;
+ a=20
+---
+(0 rows)
+
+SELECT a AS a1 FROM tab_search_diff_2;
+ a1=20
+----
+(0 rows)
+
+RESET search_path;
+-- Schema qualifications
+SELECT count(*) FROM pgss_schema_1.tab_search_same;
+ count=20
+-------
+     0
+(1 row)
+
+SELECT a, b FROM pgss_schema_1.tab_search_same;
+ a | b=20
+---+---
+(0 rows)
+
+SELECT count(*) FROM pgss_schema_2.tab_search_diff_1;
+ count=20
+-------
+     0
+(1 row)
+
+SELECT count(*) FROM pg_temp.tab_search_diff_2;
+ count=20
+-------
+     0
+(1 row)
+
+SELECT a FROM pgss_schema_2.tab_search_diff_2 AS t1;
+ a=20
+---
+(0 rows)
+
+SELECT a FROM pgss_schema_2.tab_search_diff_2;
+ a=20
+---
+(0 rows)
+
+SELECT a AS a1 FROM pgss_schema_2.tab_search_diff_2;
+ a1=20
+----
+(0 rows)
+
+SELECT calls, query FROM pg_stat_statements ORDER BY query COLLATE "C";
+ calls |                                 query                            =
     =20
+-------+------------------------------------------------------------------=
------
+     8 | SELECT a FROM tab_search_diff_2
+     4 | SELECT a FROM tab_search_diff_2 AS t1
+     4 | SELECT a, b FROM tab_search_same
+     0 | SELECT calls, query FROM pg_stat_statements ORDER BY query COLLAT=
E "C"
+     4 | SELECT count(*) FROM tab_search_diff_1
+     4 | SELECT count(*) FROM tab_search_diff_2
+     4 | SELECT count(*) FROM tab_search_same
+     1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
+(8 rows)
+
+DROP SCHEMA pgss_schema_1 CASCADE;
+NOTICE:  drop cascades to 3 other objects
+DETAIL:  drop cascades to table pgss_schema_1.tab_search_same
+drop cascades to table pgss_schema_1.tab_search_diff_1
+drop cascades to table pgss_schema_1.tab_search_diff_2
+DROP SCHEMA pgss_schema_2 CASCADE;
+NOTICE:  drop cascades to 3 other objects
+DETAIL:  drop cascades to table pgss_schema_2.tab_search_same
+drop cascades to table pgss_schema_2.tab_search_diff_1
+drop cascades to table pgss_schema_2.tab_search_diff_2
+DROP TABLE tab_search_same, tab_search_diff_1, tab_search_diff_2;
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t=20
+---
+ t
+(1 row)
+
diff --git a/contrib/pg_stat_statements/sql/select.sql b/contrib/pg_stat_st=
atements/sql/select.sql
index e0be58d5e24b..fbed557ec369 100644
--- a/contrib/pg_stat_statements/sql/select.sql
+++ b/contrib/pg_stat_statements/sql/select.sql
@@ -148,3 +148,72 @@ SELECT (
=20
 SELECT COUNT(*) FROM pg_stat_statements WHERE query LIKE '%SELECT GROUPING=
%';
 SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+
+-- Temporary tables, grouped together.
+BEGIN;
+  CREATE TEMP TABLE temp_t (id int) ON COMMIT DROP;
+  SELECT * FROM temp_t;
+COMMIT;
+BEGIN;
+  CREATE TEMP TABLE temp_t (id int) ON COMMIT DROP;
+  SELECT * FROM temp_t;
+COMMIT;
+SELECT calls, query FROM pg_stat_statements ORDER BY query COLLATE "C";
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+
+-- search_path with various schemas and temporary tables
+CREATE SCHEMA pgss_schema_1;
+CREATE SCHEMA pgss_schema_2;
+-- Same attributes.
+CREATE TABLE pgss_schema_1.tab_search_same (a int, b int);
+CREATE TABLE pgss_schema_2.tab_search_same (a int, b int);
+CREATE TEMP TABLE tab_search_same (a int, b int);
+-- Different number of attributes, mapping types
+CREATE TABLE pgss_schema_1.tab_search_diff_1 (a int);
+CREATE TABLE pgss_schema_2.tab_search_diff_1 (a int, b int);
+CREATE TEMP TABLE tab_search_diff_1 (a int, b int, c int);
+-- Same number of attributes, different types
+CREATE TABLE pgss_schema_1.tab_search_diff_2 (a int);
+CREATE TABLE pgss_schema_2.tab_search_diff_2 (a text);
+CREATE TEMP TABLE tab_search_diff_2 (a bigint);
+-- First permanent schema
+SET search_path =3D 'pgss_schema_1';
+SELECT count(*) FROM tab_search_same;
+SELECT a, b FROM tab_search_same;
+SELECT count(*) FROM tab_search_diff_1;
+SELECT count(*) FROM tab_search_diff_2;
+SELECT a FROM tab_search_diff_2 AS t1;
+SELECT a FROM tab_search_diff_2;
+SELECT a AS a1 FROM tab_search_diff_2;
+-- Second permanent schema
+SET search_path =3D 'pgss_schema_2';
+SELECT count(*) FROM tab_search_same;
+SELECT a, b FROM tab_search_same;
+SELECT count(*) FROM tab_search_diff_1;
+SELECT count(*) FROM tab_search_diff_2;
+SELECT a FROM tab_search_diff_2 AS t1;
+SELECT a FROM tab_search_diff_2;
+SELECT a AS a1 FROM tab_search_diff_2;
+-- Temporary schema
+SET search_path =3D 'pg_temp';
+SELECT count(*) FROM tab_search_same;
+SELECT a, b FROM tab_search_same;
+SELECT count(*) FROM tab_search_diff_1;
+SELECT count(*) FROM tab_search_diff_2;
+SELECT a FROM tab_search_diff_2 AS t1;
+SELECT a FROM tab_search_diff_2;
+SELECT a AS a1 FROM tab_search_diff_2;
+RESET search_path;
+-- Schema qualifications
+SELECT count(*) FROM pgss_schema_1.tab_search_same;
+SELECT a, b FROM pgss_schema_1.tab_search_same;
+SELECT count(*) FROM pgss_schema_2.tab_search_diff_1;
+SELECT count(*) FROM pg_temp.tab_search_diff_2;
+SELECT a FROM pgss_schema_2.tab_search_diff_2 AS t1;
+SELECT a FROM pgss_schema_2.tab_search_diff_2;
+SELECT a AS a1 FROM pgss_schema_2.tab_search_diff_2;
+SELECT calls, query FROM pg_stat_statements ORDER BY query COLLATE "C";
+DROP SCHEMA pgss_schema_1 CASCADE;
+DROP SCHEMA pgss_schema_2 CASCADE;
+DROP TABLE tab_search_same, tab_search_diff_1, tab_search_diff_2;
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
--=20
2.49.0


--CdIaZbBgfMcoFjU3--

--LMbqH2q/oNMLq+Li
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmfiVPAACgkQnvQgOdby
QH1k3w/9HHVxH5l09Ldjld3F1+kJsGLxIhD/muYIztWB71cp0+9OAZp5g2NdVu3H
LMG28y+1o8Rim4cbysJrROkgMKZdSE1UwaZS1CIsr5SXT8vA7Sm2gocsxbio50OJ
q9+OsXvlJAEYRfDpNjZBaPS1kA2okXX1w9B4+Qj2Tr+eUaqqMSb5nui1jJsTP2iu
E0dqg2sSyPLs//1A5zdZ9ymQYWqbTWVgFN58pd5FzukpHKJKOJE8cmJP9pb551Bq
sJkPy8knfdFGl9Oi/CuDNGailGWvb3UfZNeAilmOFdsyT3+4I++n/iu7xJuZWQwr
C+fVUVvmaR+t3os5fJ/uKwC55riMgHZWmD3cgf9DZ6VwHjRr6GCZ2XAAUZHoBojZ
6lQMfgM0P89wGYVjqi71y9YeZ1hSGBjy/koh7d2Inl1/M7huuSYWxLQrYjTTHXVN
1SoFkPs+OLiBsbpJcUbhMCvPz648cV43FotZksBWEa7dRQ1nySshwFd+vAuQQXpi
tydnw1ESWN4EV2KaNkBmGY7Z2DpLqVLEaUaA8tXaoYamFvSFSm7S+Wzy2NPYUdF1
A1yfbNx3jT8M+nThQUVp5EPra8Srbv6BwQCOSYk+Bn8nLG3sHRFPvYlMk0NUt2Dv
Hy1BFHDglFjo4XttwbMiE3BmlF+36p6Px7jhJJacUzYOYMfTfHk=
=SA2W
-----END PGP SIGNATURE-----

--LMbqH2q/oNMLq+Li--