From 8a1459f62ad16c5fee792faa83ac7ab70a291939 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 29 May 2025 11:26:31 +0900 Subject: [PATCH] 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 Discussion: https://postgr.es/m/CAA5RZ0tzxvWXsacGyxrixdhy3tTTDfJQqxyFBRFh31nNHBQ5qA@mail.gmail.com Backpatch-through: 13 --- .../expected/pg_stat_statements.out | 29 +++++++++++++++++++ .../pg_stat_statements/pg_stat_statements.c | 4 ++- .../sql/pg_stat_statements.sql | 8 +++++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/contrib/pg_stat_statements/expected/pg_stat_statements.out b/contrib/pg_stat_statements/expected/pg_stat_statements.out index b52d1877223..0c03565f2e6 100644 --- a/contrib/pg_stat_statements/expected/pg_stat_statements.out +++ b/contrib/pg_stat_statements/expected/pg_stat_statements.out @@ -398,6 +398,35 @@ SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C"; SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C" | 0 | 0 (6 rows) +-- normalization of constants and parameters, with constant locations +-- recorded 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 WHERE (3, 4) IN ((5, 6), (8, 7)); +-- +(0 rows) + +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)) | 2 + SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 + SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0 +(4 rows) + -- -- queries with locking clauses -- diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 1ec5525276a..f2332312ba3 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -2640,6 +2640,7 @@ generate_normalized_query(JumbleState *jstate, const char *query, n_quer_loc = 0, /* Normalized query byte location */ last_off = 0, /* Offset from start for previous tok */ last_tok_len = 0; /* Length (in bytes) of that tok */ + int num_constants_replaced = 0; /* * Get constants' lengths (core system only gives us locations). Note @@ -2683,7 +2684,8 @@ generate_normalized_query(JumbleState *jstate, const char *query, /* And insert a param symbol in place of the constant token */ n_quer_loc += sprintf(norm_query + n_quer_loc, "$%d", - i + 1 + jstate->highest_extern_param_id); + num_constants_replaced + 1 + jstate->highest_extern_param_id); + num_constants_replaced++; quer_loc = off + tok_len; last_off = off; diff --git a/contrib/pg_stat_statements/sql/pg_stat_statements.sql b/contrib/pg_stat_statements/sql/pg_stat_statements.sql index dffd2c8c187..45f9cd29b77 100644 --- a/contrib/pg_stat_statements/sql/pg_stat_statements.sql +++ b/contrib/pg_stat_statements/sql/pg_stat_statements.sql @@ -201,6 +201,14 @@ SELECT PLUS_ONE(1); SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C"; +-- normalization of constants and parameters, with constant locations +-- recorded 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 WHERE (3, 4) IN ((5, 6), (8, 7)); +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + -- -- queries with locking clauses -- -- 2.39.5