Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 26 Mar 2025 09:34:02 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Sami Imseih <samimseih@gmail.com>, Christoph Berg <myon@debian.org>,
	Lukas Fittl <lukas@fittl.com>,
	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-NLelZdH3GaLRNK@paquier.xyz>
References: <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>
 <Z-JU8EmGdlnNEGot@paquier.xyz>
 <Z-LOknecFpjVjhkP@msg.df7cb.de>
 <CAA5RZ0sy8gZzK0qNScY-C_jOpqQb6Y3EQaXHu0Ti9pon-4jJBg@mail.gmail.com>
 <Z-M32Cm7FM-1gpxB@paquier.xyz>
 <1324036.1742945060@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="w/VwHMZEYM+oxSrI"
Content-Disposition: inline
In-Reply-To: <1324036.1742945060@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/Z-NLelZdH3GaLRNK%40paquier.xyz>
Precedence: bulk


--w/VwHMZEYM+oxSrI
Content-Type: multipart/mixed; boundary="8WY1EzE0ckOfLc3/"
Content-Disposition: inline


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

On Tue, Mar 25, 2025 at 07:24:20PM -0400, Tom Lane wrote:
> fails to honor $query_jumble_ignore as the other if-branches do.
> Perhaps it's unlikely that a node would have both query_jumble_custom
> and query_jumble_ignore annotations, but still, the script would emit
> completely incorrect code if it did.  Also, the "# node type" comment
> should probably be moved down to within the first "elsif" block.

Oops, sorry about that.  Fixed both of these in 27ee6ede6bc9.

> I'd change "semantically similar queries" to "otherwise-similar
> queries"; I think writing "semantically" will just confuse people.

If I get the difference right, semantically would apply to concepts
related to linguistics, but that's not what we have here, so you are
using a more general term.

Thanks for the suggestions.
--
Michael

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

=46rom 30f8066989eac6f8c72034d3fb5150368c2821a9 Mon Sep 17 00:00:00 2001
=46rom: Michael Paquier <michael@paquier.xyz>
Date: Wed, 26 Mar 2025 09:17:41 +0900
Subject: [PATCH v7] Add custom query jumble function for RangeTblEntry.eref

---
 src/include/nodes/parsenodes.h                | 11 +++++++---
 src/backend/nodes/queryjumblefuncs.c          | 19 ++++++++++++++++++
 doc/src/sgml/pgstatstatements.sgml            |  9 +++++++--
 .../pg_stat_statements/expected/select.out    | 20 ++++++++-----------
 4 files changed, 42 insertions(+), 17 deletions(-)

diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 23c9e3c5abf2..df331b1c0d99 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
+	 * that the table name is included in the computation, but 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/doc/src/sgml/pgstatstatements.sgml b/doc/src/sgml/pgstatstatem=
ents.sgml
index f4e384e95aea..625b84ebfefd 100644
--- a/doc/src/sgml/pgstatstatements.sgml
+++ b/doc/src/sgml/pgstatstatements.sgml
@@ -675,8 +675,13 @@ calls | 2
    things, the internal object identifiers appearing in this representatio=
n.
    This has some counterintuitive implications.  For example,
    <filename>pg_stat_statements</filename> will consider two apparently-id=
entical
-   queries to be distinct, if they reference a table that was dropped
-   and recreated between the executions of the two queries.
+   queries to be distinct, if they reference for example a function that w=
as
+   dropped and recreated between the executions of the two queries.
+   Conversely, if a table is dropped and recreated between the
+   executions of queries, two apparently-identical queries may be
+   considered the same. However, if the alias for a table is different
+   for otherwise-similar queries, these queries will be considered
+   distinct.
    The hashing process is also sensitive to differences in
    machine architecture and other facets of the platform.
    Furthermore, it is not safe to assume that <structfield>queryid</struct=
field>
diff --git a/contrib/pg_stat_statements/expected/select.out b/contrib/pg_st=
at_statements/expected/select.out
index 708c6b0e9c41..1eebc2898ab9 100644
--- a/contrib/pg_stat_statements/expected/select.out
+++ b/contrib/pg_stat_statements/expected/select.out
@@ -433,11 +433,10 @@ COMMIT;
 SELECT calls, query FROM pg_stat_statements ORDER BY query COLLATE "C";
  calls |                                 query                            =
     =20
 -------+------------------------------------------------------------------=
------
-     1 | SELECT * FROM temp_t
-     1 | SELECT * FROM temp_t
+     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
-(4 rows)
+(3 rows)
=20
 SELECT pg_stat_statements_reset() IS NOT NULL AS t;
  t=20
@@ -623,18 +622,15 @@ SELECT a AS a1 FROM pgss_schema_2.tab_search_diff_2;
 SELECT calls, query FROM pg_stat_statements ORDER BY query COLLATE "C";
  calls |                                 query                            =
     =20
 -------+------------------------------------------------------------------=
------
-     3 | SELECT a FROM pgss_schema_2.tab_search_diff_2 AS t1
-     9 | SELECT a FROM tab_search_diff_2 AS t1
-     1 | SELECT a, b FROM pgss_schema_1.tab_search_same
-     3 | SELECT a, b FROM tab_search_same
+     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"
-     1 | SELECT count(*) FROM pgss_schema_1.tab_search_same
-     1 | SELECT count(*) FROM pgss_schema_2.tab_search_diff_1
-     3 | SELECT count(*) FROM tab_search_diff_1
+     4 | SELECT count(*) FROM tab_search_diff_1
      4 | SELECT count(*) FROM tab_search_diff_2
-     3 | SELECT count(*) FROM tab_search_same
+     4 | SELECT count(*) FROM tab_search_same
      1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
-(11 rows)
+(8 rows)
=20
 DROP SCHEMA pgss_schema_1 CASCADE;
 NOTICE:  drop cascades to 3 other objects
--=20
2.49.0


--8WY1EzE0ckOfLc3/--

--w/VwHMZEYM+oxSrI
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmfjS3oACgkQnvQgOdby
QH1IWxAAhk65mQdQGkfV983AbAV0y+Bd4qWSaSYTNVuzZdhxB9+W/XxkGFA4Y78Y
JmumFkADZRJzfILIWQkPAkomHlGZsCQiMsDEMfFv7hZxxIK9DuTm7AL7Dwz57t9A
A6Jv+NSvSnDDVBLrJzY37aiy1JLcHw53h4yp+YeQD/XegZmUif6dpwJpd4cYbYlf
1+euu2PU/D1DwywweM9jkIkDkASZVqpMNEbT2ETNMe/IgTrH/RNG2JL5JD3xpbtJ
1EvEIaD0+NcTFe+ilpQrp3wDxB2Awuak61p7HmpEb+ZQxaPz1TCWQWWC6xzYSTJ6
eyupJxLXlxs/k6+ln4on+MNqhvhF4VQYDCHKUaDHY3Vuy0TU8xHx5NSpGO3VsNFK
Fnz9Pd3bLHP3CA74aONSVvfMK+s0ggDTOaZ3h9rnPmGXt33hUGYqr4wf4CtFTeSV
ckt44jjE5JuRVn1QozZn8AjaluYHdKEdIevYARK76jlhOO3CiZjiFA2d5U8KhSoR
7IlQV/fFRrsD9rWnIuxMgTOnh3bhmvE+JoRMCGWreGcqCKt76EmioqRMCY7tD/0A
IXXFEyvcywKbWLeKnB48pQRqpi/j5dsEFw+iwa/lVkUd4TkRgl2f3obYyyDoOPq2
acqlMgBp46NSqUITNphPAIlAldnhWkNsPHxr0jV3rdTJ3JaQXEc=
=j0hC
-----END PGP SIGNATURE-----

--w/VwHMZEYM+oxSrI--