v7-0001-Fix-broken-normalization-due-to-duplicate-constan.patch
application/octet-stream
Filename: v7-0001-Fix-broken-normalization-due-to-duplicate-constan.patch
Type: application/octet-stream
Part: 1
Patch
Same data as JSON:
GET /api/v1/attachments/:id/patch
the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes.
API reference →
Format: format-patch
Series: patch v7-0001
Subject: Fix broken normalization due to duplicate constant locations
| File | + | − |
|---|---|---|
| contrib/pg_stat_statements/expected/normalize.out | 27 | 0 |
| contrib/pg_stat_statements/Makefile | 1 | 1 |
| contrib/pg_stat_statements/meson.build | 1 | 0 |
| contrib/pg_stat_statements/pg_stat_statements.c | 3 | 7 |
| contrib/pg_stat_statements/sql/normalize.sql | 10 | 0 |
From 4e80ed16ba0a6554052246bd19f5509b13fb1514 Mon Sep 17 00:00:00 2001
From: Sami Imseih <simseih@amazon.com>
Date: Mon, 26 May 2025 21:41:17 -0500
Subject: [PATCH v7 1/4] Fix broken normalization due to duplicate constant
locations
pg_stat_statements anticipates that certain constant
locations may be recorded multiple times and attempts
to avoid calculating a length for these locations in
fill_in_constant_lengths.
However, during generate_normalized_query, these
locations are not excluded from consideration and
will increment the $n counter for every recorded
occurrence of such a location. In practice, this can
lead to incorrect normalization in certain cases.
select where '1' IN ('2'::int, '3'::int::text)
would be normalized to:
select where $1 IN ($3, $4)
instead of the correct:
select where $1 IN ($2, $3)
This is because the left-expression, '1' is used
as an argument in the OpExpr generated for every
element in the IN clause.
To correct, track the number of constants replaced
with an $n by a separate counter instead of the
iterator used to loop through the list of locations.
---
contrib/pg_stat_statements/Makefile | 2 +-
.../pg_stat_statements/expected/normalize.out | 27 +++++++++++++++++++
contrib/pg_stat_statements/meson.build | 1 +
.../pg_stat_statements/pg_stat_statements.c | 10 +++----
contrib/pg_stat_statements/sql/normalize.sql | 10 +++++++
5 files changed, 42 insertions(+), 8 deletions(-)
create mode 100644 contrib/pg_stat_statements/expected/normalize.out
create mode 100644 contrib/pg_stat_statements/sql/normalize.sql
diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index b2bd8794d2a..f08280bdcf7 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -20,7 +20,7 @@ LDFLAGS_SL += $(filter -lm, $(LIBS))
REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/pg_stat_statements/pg_stat_statements.conf
REGRESS = select dml cursors utility level_tracking planning \
user_activity wal entry_timestamp privileges extended \
- parallel cleanup oldextversions squashing
+ parallel cleanup oldextversions squashing normalize
# Disabled because these tests require "shared_preload_libraries=pg_stat_statements",
# which typical installcheck users do not have (e.g. buildfarm clients).
NO_INSTALLCHECK = 1
diff --git a/contrib/pg_stat_statements/expected/normalize.out b/contrib/pg_stat_statements/expected/normalize.out
new file mode 100644
index 00000000000..1e94dbb9b43
--- /dev/null
+++ b/contrib/pg_stat_statements/expected/normalize.out
@@ -0,0 +1,27 @@
+--
+-- Validate normalization of constants
+--
+-- Ensure that there are no gaps in the generated $n parameters. The following
+-- queries will record some constant location one or more times.
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t
+---
+ t
+(1 row)
+
+SELECT WHERE '1' IN ('1'::int, '3'::int::text);
+--
+(1 row)
+
+SELECT WHERE (1, 2) IN ((1, 2), (2, 3));
+--
+(1 row)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+----------------------------------------------------+-------
+ SELECT WHERE $1 IN ($2::int, $3::int::text) | 1
+ SELECT WHERE ($1, $2) IN (($3, $4), ($5, $6)) | 1
+ SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1
+(3 rows)
+
diff --git a/contrib/pg_stat_statements/meson.build b/contrib/pg_stat_statements/meson.build
index 01a6cbdcf61..931a5b29427 100644
--- a/contrib/pg_stat_statements/meson.build
+++ b/contrib/pg_stat_statements/meson.build
@@ -57,6 +57,7 @@ tests += {
'cleanup',
'oldextversions',
'squashing',
+ 'normalize',
],
'regress_args': ['--temp-config', files('pg_stat_statements.conf')],
# Disabled because these tests require
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index d8fdf42df79..c58f34e9f30 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -2818,9 +2818,7 @@ generate_normalized_query(JumbleState *jstate, const char *query,
last_off = 0, /* Offset from start for previous tok */
last_tok_len = 0; /* Length (in bytes) of that tok */
bool in_squashed = false; /* in a run of squashed consts? */
- int skipped_constants = 0; /* Position adjustment of later
- * constants after squashed ones */
-
+ int num_constants_replaced = 0;
/*
* Get constants' lengths (core system only gives us locations). Note
@@ -2878,7 +2876,7 @@ generate_normalized_query(JumbleState *jstate, const char *query,
/* ... and then a param symbol replacing the constant itself */
n_quer_loc += sprintf(norm_query + n_quer_loc, "$%d",
- i + 1 + jstate->highest_extern_param_id - skipped_constants);
+ num_constants_replaced++ + 1 + jstate->highest_extern_param_id);
/* In case previous constants were merged away, stop doing that */
in_squashed = false;
@@ -2902,12 +2900,10 @@ generate_normalized_query(JumbleState *jstate, const char *query,
/* ... and then start a run of squashed constants */
n_quer_loc += sprintf(norm_query + n_quer_loc, "$%d /*, ... */",
- i + 1 + jstate->highest_extern_param_id - skipped_constants);
+ num_constants_replaced++ + 1 + jstate->highest_extern_param_id);
/* The next location will match the block below, to end the run */
in_squashed = true;
-
- skipped_constants++;
}
else
{
diff --git a/contrib/pg_stat_statements/sql/normalize.sql b/contrib/pg_stat_statements/sql/normalize.sql
new file mode 100644
index 00000000000..1252c9bc53d
--- /dev/null
+++ b/contrib/pg_stat_statements/sql/normalize.sql
@@ -0,0 +1,10 @@
+--
+-- Validate normalization of constants
+--
+
+-- Ensure that there are no gaps in the generated $n parameters. The following
+-- queries will record some constant location one or more times.
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+SELECT WHERE '1' IN ('1'::int, '3'::int::text);
+SELECT WHERE (1, 2) IN ((1, 2), (2, 3));
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
\ No newline at end of file
--
2.39.5 (Apple Git-154)