public inbox for [email protected]  
help / color / mirror / Atom feed
From: Fujii Masao <[email protected]>
To: [email protected]
To: [email protected]
Subject: Re: BUG #19528: Assert failure in generate_normalized_query() via Squashed Array Literals
Date: Sun, 21 Jun 2026 00:18:19 +0900
Message-ID: <CAHGQGwEGpf3eWyLMqQcxP11NyhvBNCFzmZP1EXM5F1Ca4NaLMQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>

On Sat, Jun 20, 2026 at 7:53 PM PG Bug reporting form
<[email protected]> wrote:
> `" /*, ... */"` is 11 bytes, so a squashed entry writes `"$N"` (2 bytes)
> plus `" /*, ... */"` (11 bytes) = 13 bytes to replace a 1-byte constant, a
> net expansion of 12 bytes. The budget of 10 bytes is exceeded by 2 bytes per
> squashed location. With N squashed array elements the buffer overflows by 2N
> bytes.

This analysis doesn't seem correct to me.

For "ARRAY[1,2]", the recorded location covers the entire list text,
"1,2" (3 bytes), which is replaced with "$n /*, ... */". For a
single-digit placeholder, "$1 /*, ... */" is 13 bytes, so replacing
"1,2" results in a net growth of 10 bytes.

That still fits within the existing budget. The budget is first
exceeded when the placeholder number reaches two digits:
"$10 /*, ... */" is 14 bytes, giving a net growth of 11 bytes.
Therefore, the smallest reproducer is 10 squashed "ARRAY[1,2]"
entries, where the shortfall seems only 1 byte overall, not 2 bytes per
squashed location.


> ### Fix
>
> Account for the extra 11 bytes appended when a squashed suffix is written.
> One correct approach:
>
> ```c
> norm_query_buflen = query_len + jstate->clocations_count *
>     (10 + (jstate->has_squashed_lists ? 11 : 0));
> ```
>
> A simpler conservative fix that covers the worst case (`"$2147483648 /*, ...
> */"` = 22 bytes replacing a 1-byte constant, net 21 bytes):
>
> ```c
> norm_query_buflen = query_len + jstate->clocations_count * 22;
> ```

How about keeping the current "clocations_count * 10" budget for
ordinary replacements, counting squashed entries separately, and
adding only the extra space they may need, for example:

    clocations_count * 10 + squashed_count * 9

The existing "+10" already covers ordinary placeholders. For squashed
entries, the worst-case additional overhead is 9 bytes, from replacing
the shortest possible source text "1,2" (3 bytes) with the longest
possible placeholder "$2147483647 /*, ... */" (22 bytes), for a total
growth of 19 bytes. Attached patch implements this approach.

Thoughts?

Regards,

-- 
Fujii Masao


Attachments:

  [application/octet-stream] v1-0001-pg_stat_statements-add-room-for-squashed-placehol.patch (5.1K, ../CAHGQGwEGpf3eWyLMqQcxP11NyhvBNCFzmZP1EXM5F1Ca4NaLMQ@mail.gmail.com/2-v1-0001-pg_stat_statements-add-room-for-squashed-placehol.patch)
  download | inline diff:
From 0b22d7130d7f1315aa3d916520f2dd4aa0cfa91b Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Sat, 20 Jun 2026 23:11:41 +0900
Subject: [PATCH v1] pg_stat_statements: add room for squashed placeholders

---
 .../pg_stat_statements/expected/squashing.out | 24 +++++++++++++++++++
 .../pg_stat_statements/pg_stat_statements.c   | 17 ++++++++++++-
 contrib/pg_stat_statements/sql/squashing.sql  |  7 ++++++
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/contrib/pg_stat_statements/expected/squashing.out b/contrib/pg_stat_statements/expected/squashing.out
index 8438235a2ce..057f56c4741 100644
--- a/contrib/pg_stat_statements/expected/squashing.out
+++ b/contrib/pg_stat_statements/expected/squashing.out
@@ -79,6 +79,30 @@ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
  SELECT pg_stat_statements_reset() IS NOT NULL AS t    |     1
 (3 rows)
 
+-- small array literals still fit once squashed placeholders reach 2 digits
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT ARRAY[1,2] AS a1, ARRAY[1,2] AS a2, ARRAY[1,2] AS a3, ARRAY[1,2] AS a4,
+       ARRAY[1,2] AS a5, ARRAY[1,2] AS a6, ARRAY[1,2] AS a7, ARRAY[1,2] AS a8,
+       ARRAY[1,2] AS a9, ARRAY[1,2] AS a10;
+  a1   |  a2   |  a3   |  a4   |  a5   |  a6   |  a7   |  a8   |  a9   |  a10  
+-------+-------+-------+-------+-------+-------+-------+-------+-------+-------
+ {1,2} | {1,2} | {1,2} | {1,2} | {1,2} | {1,2} | {1,2} | {1,2} | {1,2} | {1,2}
+(1 row)
+
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+                                                         query                                                          | calls 
+------------------------------------------------------------------------------------------------------------------------+-------
+ SELECT ARRAY[$1 /*, ... */] AS a1, ARRAY[$2 /*, ... */] AS a2, ARRAY[$3 /*, ... */] AS a3, ARRAY[$4 /*, ... */] AS a4,+|     1
+        ARRAY[$5 /*, ... */] AS a5, ARRAY[$6 /*, ... */] AS a6, ARRAY[$7 /*, ... */] AS a7, ARRAY[$8 /*, ... */] AS a8,+| 
+        ARRAY[$9 /*, ... */] AS a9, ARRAY[$10 /*, ... */] AS a10                                                        | 
+ SELECT pg_stat_statements_reset() IS NOT NULL AS t                                                                     |     1
+(2 rows)
+
 -- built-in functions will be squashed
 -- the IN and ARRAY forms of this statement will have the same queryId
 SELECT pg_stat_statements_reset() IS NOT NULL AS t;
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 92315627916..bd79a7d1470 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -2822,6 +2822,7 @@ generate_normalized_query(const JumbleState *jstate, const char *query,
 				last_off = 0,	/* Offset from start for previous tok */
 				last_tok_len = 0;	/* Length (in bytes) of that tok */
 	int			num_constants_replaced = 0;
+	int			squashed_count = 0;
 	LocationLen *locs = NULL;
 
 	/*
@@ -2837,8 +2838,22 @@ generate_normalized_query(const JumbleState *jstate, const char *query,
 	 * certainly isn't more than 11 bytes, even if n reaches INT_MAX.  We
 	 * could refine that limit based on the max value of n for the current
 	 * query, but it hardly seems worth any extra effort to do so.
+	 *
+	 * Squashed lists need a little more room. The shortest squashed source
+	 * text is "1,2" (three bytes), while the longest squashed placeholder is
+	 * 22 bytes long (an INT_MAX placeholder plus the squash marker), so each
+	 * squashed list can need up to 9 bytes more than the base allowance of 10
+	 * bytes per entry.
 	 */
-	norm_query_buflen = query_len + jstate->clocations_count * 10;
+	for (int i = 0; i < jstate->clocations_count; i++)
+	{
+		/* Ignore duplicate locations that ComputeConstantLengths() disabled */
+		if (locs[i].squashed && locs[i].length >= 0)
+			squashed_count++;
+	}
+
+	norm_query_buflen = query_len + jstate->clocations_count * 10 +
+		squashed_count * 9;
 
 	/* Allocate result buffer */
 	norm_query = palloc(norm_query_buflen + 1);
diff --git a/contrib/pg_stat_statements/sql/squashing.sql b/contrib/pg_stat_statements/sql/squashing.sql
index fc9e6573873..2bd26b24eec 100644
--- a/contrib/pg_stat_statements/sql/squashing.sql
+++ b/contrib/pg_stat_statements/sql/squashing.sql
@@ -24,6 +24,13 @@ SELECT ARRAY[1, 2, 3, 4];
 SELECT ARRAY[1, 2, 3, 4, 5];
 SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
 
+-- small array literals still fit once squashed placeholders reach 2 digits
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+SELECT ARRAY[1,2] AS a1, ARRAY[1,2] AS a2, ARRAY[1,2] AS a3, ARRAY[1,2] AS a4,
+       ARRAY[1,2] AS a5, ARRAY[1,2] AS a6, ARRAY[1,2] AS a7, ARRAY[1,2] AS a8,
+       ARRAY[1,2] AS a9, ARRAY[1,2] AS a10;
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
 -- built-in functions will be squashed
 -- the IN and ARRAY forms of this statement will have the same queryId
 SELECT pg_stat_statements_reset() IS NOT NULL AS t;
-- 
2.53.0



view thread (3+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: BUG #19528: Assert failure in generate_normalized_query() via Squashed Array Literals
  In-Reply-To: <CAHGQGwEGpf3eWyLMqQcxP11NyhvBNCFzmZP1EXM5F1Ca4NaLMQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox