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

diff --git a/contrib/pg_stat_statements/expected/extended.out b/contrib/pg_stat_statements/expected/extended.out
index 04a05943372b9212e0d221a94df7fa4588fad9aa..7da308ba84f4f673b77bb1979b38d907949e4ac5 100644 (file)
--- a/contrib/pg_stat_statements/expected/extended.out
+++ b/contrib/pg_stat_statements/expected/extended.out
@@ -68,3 +68,61 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
      1 |    1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
 (4 rows)
 
+-- Various parameter numbering patterns
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
+(1 row)
+
+-- Unique query IDs with parameter numbers switched.
+SELECT WHERE ($1::int, 7) IN ((8, $2::int), ($3::int, 9)) \bind '1' '2' '3' \g
+--
+(0 rows)
+
+SELECT WHERE ($2::int, 10) IN ((11, $3::int), ($1::int, 12)) \bind '1' '2' '3' \g
+--
+(0 rows)
+
+SELECT WHERE $1::int IN ($2::int, $3::int) \bind '1' '2' '3' \g
+--
+(0 rows)
+
+SELECT WHERE $2::int IN ($3::int, $1::int) \bind '1' '2' '3' \g
+--
+(0 rows)
+
+SELECT WHERE $3::int IN ($1::int, $2::int) \bind '1' '2' '3' \g
+--
+(0 rows)
+
+-- Two groups of two queries with the same query ID.
+SELECT WHERE '1'::int IN ($1::int, '2'::int) \bind '1' \g
+--
+(1 row)
+
+SELECT WHERE '4'::int IN ($1::int, '5'::int) \bind '2' \g
+--
+(0 rows)
+
+SELECT WHERE $2::int IN ($1::int, '1'::int) \bind '1' '2' \g
+--
+(0 rows)
+
+SELECT WHERE $2::int IN ($1::int, '2'::int) \bind '3' '4' \g
+--
+(0 rows)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+                            query                             | calls 
+--------------------------------------------------------------+-------
+ SELECT WHERE $1::int IN ($2::int, $3::int)                   |     1
+ SELECT WHERE $2::int IN ($1::int, $3::int)                   |     2
+ SELECT WHERE $2::int IN ($1::int, $3::int)                   |     2
+ SELECT WHERE $2::int IN ($3::int, $1::int)                   |     1
+ SELECT WHERE $3::int IN ($1::int, $2::int)                   |     1
+ SELECT WHERE ($1::int, $4) IN (($5, $2::int), ($3::int, $6)) |     1
+ SELECT WHERE ($2::int, $4) IN (($5, $3::int), ($1::int, $6)) |     1
+ SELECT pg_stat_statements_reset() IS NOT NULL AS t           |     1
+(8 rows)
+

diff --git a/contrib/pg_stat_statements/expected/select.out b/contrib/pg_stat_statements/expected/select.out
index 09476a7b699e9539e1bb63b276345a522653f1e9..038ae1103645e33df70fea60b840b64c24aeb015 100644 (file)
--- a/contrib/pg_stat_statements/expected/select.out
+++ b/contrib/pg_stat_statements/expected/select.out
@@ -238,6 +238,35 @@ SELECT pg_stat_statements_reset() IS NOT NULL AS t;
  t
 (1 row)
 
+-- 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 d8fdf42df79354e9aec32130e816c4814304083a..c58f34e9f304614dfe8694c57421d83b98dcfa13 100644 (file)
--- 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/extended.sql b/contrib/pg_stat_statements/sql/extended.sql
index 1af0711020c41ea8a1e172e04e70deb0d8ed1a12..a366658a53a72540083939388e51abd459634acd 100644 (file)
--- a/contrib/pg_stat_statements/sql/extended.sql
+++ b/contrib/pg_stat_statements/sql/extended.sql
@@ -19,3 +19,19 @@ SELECT $1 \bind 'unnamed_val1' \g
 \bind_named stmt1 'stmt1_val1' \g
 
 SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- Various parameter numbering patterns
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+-- Unique query IDs with parameter numbers switched.
+SELECT WHERE ($1::int, 7) IN ((8, $2::int), ($3::int, 9)) \bind '1' '2' '3' \g
+SELECT WHERE ($2::int, 10) IN ((11, $3::int), ($1::int, 12)) \bind '1' '2' '3' \g
+SELECT WHERE $1::int IN ($2::int, $3::int) \bind '1' '2' '3' \g
+SELECT WHERE $2::int IN ($3::int, $1::int) \bind '1' '2' '3' \g
+SELECT WHERE $3::int IN ($1::int, $2::int) \bind '1' '2' '3' \g
+-- Two groups of two queries with the same query ID.
+SELECT WHERE '1'::int IN ($1::int, '2'::int) \bind '1' \g
+SELECT WHERE '4'::int IN ($1::int, '5'::int) \bind '2' \g
+SELECT WHERE $2::int IN ($1::int, '1'::int) \bind '1' '2' \g
+SELECT WHERE $2::int IN ($1::int, '2'::int) \bind '3' '4' \g
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";

diff --git a/contrib/pg_stat_statements/sql/select.sql b/contrib/pg_stat_statements/sql/select.sql
index c5e0b84ee5bf544b53a9c492b08d352c65b7b6f6..189d405512fcd271df2b553ca8d472a92753acf1 100644 (file)
--- a/contrib/pg_stat_statements/sql/select.sql
+++ b/contrib/pg_stat_statements/sql/select.sql
@@ -79,6 +79,14 @@ DEALLOCATE pgss_test;
 SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
 SELECT pg_stat_statements_reset() IS NOT NULL AS t;
 
+-- 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
 --