Skip to content

Commit 35a428f

Browse files
michaelpqmichaelpq
committed
pg_stat_statements: Fix parameter number gaps in normalized queries
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() where normalized query strings are generated, these locations are not excluded from consideration. This could increment the parameter number counter for every recorded occurrence at such a location, leading to an incorrect normalization in certain cases with gaps in the numbers reported. For example, take this query: SELECT WHERE '1' IN ('2'::int, '3'::int::text) Before this commit, it would be normalized like that, with gaps in the parameter numbers: SELECT WHERE $1 IN ($3::int, $4::int::text) However the correct, less confusing one should be like that: SELECT WHERE $1 IN ($2::int, $3::int::text) This commit fixes the computation of the parameter numbers to 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. The underlying query IDs are not changed, neither are the normalized strings for existing PGSS hash entries. New entries with fresh normalized queries would automatically get reshaped based on the new parameter numbering. Issue discovered while discussing a separate problem for HEAD, but this affects all the stable branches. Author: Sami Imseih <samimseih@gmail.com> Discussion: https://postgr.es/m/CAA5RZ0tzxvWXsacGyxrixdhy3tTTDfJQqxyFBRFh31nNHBQ5qA@mail.gmail.com Backpatch-through: 13
1 parent 089f27c commit 35a428f

File tree

5 files changed

+114
-7
lines changed

5 files changed

+114
-7
lines changed

‎contrib/pg_stat_statements/expected/extended.out

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,3 +68,61 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
6868
1 | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
6969
(4 rows)
7070

71+
-- Various parameter numbering patterns
72+
SELECT pg_stat_statements_reset() IS NOT NULL AS t;
73+
t
74+
---
75+
t
76+
(1 row)
77+
78+
-- Unique query IDs with parameter numbers switched.
79+
SELECT WHERE ($1::int, 7) IN ((8, $2::int), ($3::int, 9)) \bind '1' '2' '3' \g
80+
--
81+
(0 rows)
82+
83+
SELECT WHERE ($2::int, 10) IN ((11, $3::int), ($1::int, 12)) \bind '1' '2' '3' \g
84+
--
85+
(0 rows)
86+
87+
SELECT WHERE $1::int IN ($2::int, $3::int) \bind '1' '2' '3' \g
88+
--
89+
(0 rows)
90+
91+
SELECT WHERE $2::int IN ($3::int, $1::int) \bind '1' '2' '3' \g
92+
--
93+
(0 rows)
94+
95+
SELECT WHERE $3::int IN ($1::int, $2::int) \bind '1' '2' '3' \g
96+
--
97+
(0 rows)
98+
99+
-- Two groups of two queries with the same query ID.
100+
SELECT WHERE '1'::int IN ($1::int, '2'::int) \bind '1' \g
101+
--
102+
(1 row)
103+
104+
SELECT WHERE '4'::int IN ($1::int, '5'::int) \bind '2' \g
105+
--
106+
(0 rows)
107+
108+
SELECT WHERE $2::int IN ($1::int, '1'::int) \bind '1' '2' \g
109+
--
110+
(0 rows)
111+
112+
SELECT WHERE $2::int IN ($1::int, '2'::int) \bind '3' '4' \g
113+
--
114+
(0 rows)
115+
116+
SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
117+
query | calls
118+
--------------------------------------------------------------+-------
119+
SELECT WHERE $1::int IN ($2::int, $3::int) | 1
120+
SELECT WHERE $2::int IN ($1::int, $3::int) | 2
121+
SELECT WHERE $2::int IN ($1::int, $3::int) | 2
122+
SELECT WHERE $2::int IN ($3::int, $1::int) | 1
123+
SELECT WHERE $3::int IN ($1::int, $2::int) | 1
124+
SELECT WHERE ($1::int, $4) IN (($5, $2::int), ($3::int, $6)) | 1
125+
SELECT WHERE ($2::int, $4) IN (($5, $3::int), ($1::int, $6)) | 1
126+
SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1
127+
(8 rows)
128+

‎contrib/pg_stat_statements/expected/select.out

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,35 @@ SELECT pg_stat_statements_reset() IS NOT NULL AS t;
238238
t
239239
(1 row)
240240

241+
-- normalization of constants and parameters, with constant locations
242+
-- recorded one or more times.
243+
SELECT pg_stat_statements_reset() IS NOT NULL AS t;
244+
t
245+
---
246+
t
247+
(1 row)
248+
249+
SELECT WHERE '1' IN ('1'::int, '3'::int::text);
250+
--
251+
(1 row)
252+
253+
SELECT WHERE (1, 2) IN ((1, 2), (2, 3));
254+
--
255+
(1 row)
256+
257+
SELECT WHERE (3, 4) IN ((5, 6), (8, 7));
258+
--
259+
(0 rows)
260+
261+
SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
262+
query | calls
263+
------------------------------------------------------------------------+-------
264+
SELECT WHERE $1 IN ($2::int, $3::int::text) | 1
265+
SELECT WHERE ($1, $2) IN (($3, $4), ($5, $6)) | 2
266+
SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1
267+
SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
268+
(4 rows)
269+
241270
--
242271
-- queries with locking clauses
243272
--

‎contrib/pg_stat_statements/pg_stat_statements.c

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2818,9 +2818,7 @@ generate_normalized_query(JumbleState *jstate, const char *query,
28182818
last_off = 0, /* Offset from start for previous tok */
28192819
last_tok_len = 0; /* Length (in bytes) of that tok */
28202820
bool in_squashed = false; /* in a run of squashed consts? */
2821-
int skipped_constants = 0; /* Position adjustment of later
2822-
* constants after squashed ones */
2823-
2821+
int num_constants_replaced = 0;
28242822

28252823
/*
28262824
* Get constants' lengths (core system only gives us locations). Note
@@ -2878,7 +2876,7 @@ generate_normalized_query(JumbleState *jstate, const char *query,
28782876

28792877
/* ... and then a param symbol replacing the constant itself */
28802878
n_quer_loc += sprintf(norm_query + n_quer_loc, "$%d",
2881-
i + 1 + jstate->highest_extern_param_id - skipped_constants);
2879+
num_constants_replaced++ + 1 + jstate->highest_extern_param_id);
28822880

28832881
/* In case previous constants were merged away, stop doing that */
28842882
in_squashed = false;
@@ -2902,12 +2900,10 @@ generate_normalized_query(JumbleState *jstate, const char *query,
29022900

29032901
/* ... and then start a run of squashed constants */
29042902
n_quer_loc += sprintf(norm_query + n_quer_loc, "$%d /*, ... */",
2905-
i + 1 + jstate->highest_extern_param_id - skipped_constants);
2903+
num_constants_replaced++ + 1 + jstate->highest_extern_param_id);
29062904

29072905
/* The next location will match the block below, to end the run */
29082906
in_squashed = true;
2909-
2910-
skipped_constants++;
29112907
}
29122908
else
29132909
{

‎contrib/pg_stat_statements/sql/extended.sql

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,19 @@ SELECT $1 \bind 'unnamed_val1' \g
1919
\bind_named stmt1 'stmt1_val1' \g
2020

2121
SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
22+
23+
-- Various parameter numbering patterns
24+
SELECT pg_stat_statements_reset() IS NOT NULL AS t;
25+
-- Unique query IDs with parameter numbers switched.
26+
SELECT WHERE ($1::int, 7) IN ((8, $2::int), ($3::int, 9)) \bind '1' '2' '3' \g
27+
SELECT WHERE ($2::int, 10) IN ((11, $3::int), ($1::int, 12)) \bind '1' '2' '3' \g
28+
SELECT WHERE $1::int IN ($2::int, $3::int) \bind '1' '2' '3' \g
29+
SELECT WHERE $2::int IN ($3::int, $1::int) \bind '1' '2' '3' \g
30+
SELECT WHERE $3::int IN ($1::int, $2::int) \bind '1' '2' '3' \g
31+
-- Two groups of two queries with the same query ID.
32+
SELECT WHERE '1'::int IN ($1::int, '2'::int) \bind '1' \g
33+
SELECT WHERE '4'::int IN ($1::int, '5'::int) \bind '2' \g
34+
SELECT WHERE $2::int IN ($1::int, '1'::int) \bind '1' '2' \g
35+
SELECT WHERE $2::int IN ($1::int, '2'::int) \bind '3' '4' \g
36+
37+
SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";

‎contrib/pg_stat_statements/sql/select.sql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,14 @@ DEALLOCATE pgss_test;
7979
SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
8080
SELECT pg_stat_statements_reset() IS NOT NULL AS t;
8181

82+
-- normalization of constants and parameters, with constant locations
83+
-- recorded one or more times.
84+
SELECT pg_stat_statements_reset() IS NOT NULL AS t;
85+
SELECT WHERE '1' IN ('1'::int, '3'::int::text);
86+
SELECT WHERE (1, 2) IN ((1, 2), (2, 3));
87+
SELECT WHERE (3, 4) IN ((5, 6), (8, 7));
88+
SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
89+
8290
--
8391
-- queries with locking clauses
8492
--

0 commit comments

Comments
 (0)