Re: pg_stat_statements replaces SQL/JSON ON EMPTY / ON ERROR keywords with parameters

On Mon, Oct 19, 2026 at 05:16:42AM +0000, Staroverov Ilja wrote:

A correction to my previous message: it's not the number of columns
that was part of the query ID, but the references to the columns from
the rest of the query, such as the expansion of SELECT *. The commit
message now says that.

The test in 0002 now also checks that queries which differ only in
column names or path names are merged, and that columns without PATH
and with different names are not. The PLAN clause doesn't exist in
REL_19_STABLE, so the test doesn't use it; I checked those cases on
master by hand. The series applies to master and REL_19_STABLE, and
I've tested it on both.

When it comes to such problems, I'd suggest to implement the tests as
a first patch, then do the jumbling logic in a second patch that shows
the changes in PGSS. It's easier to see what you think is the "wrong"
information and what becomes the "right" information ("wrong" and
"right" mean what you think is OK or not OK in terms of PGSS
monitoring).

+     2 | SELECT * FROM JSON_TABLE($1::jsonb, $2                                +
+       |   COLUMNS (a int PATH $3 DEFAULT $4 ON EMPTY)) jt
+     1 | SELECT * FROM JSON_TABLE($1::jsonb, $2                                +
+       |   COLUMNS (a int PATH $3)) jt
+     2 | SELECT * FROM JSON_TABLE($1::jsonb, $2                                +
+       |   COLUMNS (a int PATH $3,                                             +
+       |            NESTED PATH $4 AS p1 COLUMNS (b int PATH $5))) jt
+     1 | SELECT * FROM JSON_TABLE($1::jsonb, $2                                +
+       |   COLUMNS (a int)) jt
+     1 | SELECT * FROM JSON_TABLE($1::jsonb, $2                                +
+       |   COLUMNS (a text PATH $3)) jt
+     1 | SELECT * FROM JSON_TABLE($1::jsonb, $2                                +
+       |   COLUMNS (b int)) jt

There are many more keyword patterns than the ones tested here, like
PASSING, PLAN (UNION, OUTER ..), FORMAT JSON, etc. None of them are
covered. I guess that you may want to worry about them.

Instead of bloating more select.sql, let's move the JSON tests into a
different input file. They can be isolated, and there are way more
patterns than what you are showing here.

Note that none of this is backpatchable, as this impacts the Node
definitions and the query IDs computed.
--
Michael

Hi Michael,

Thanks for the review.

When it comes to such problems, I'd suggest to implement the tests as
a first patch, then do the jumbling logic in a second patch that shows
the changes in PGSS.

Done in the attached v4. 0001 adds a separate test file, sqljson.sql,
so select.sql is no longer touched. Besides the cases from v3, it
covers:

- the SQL/JSON query functions: ON EMPTY and ON ERROR behaviors,
RETURNING, PASSING, WRAPPER, QUOTES and FORMAT JSON;
- JSON_TABLE: column types and behaviors, EXISTS, FORMAT JSON and FOR
ORDINALITY columns, column and path names, NESTED PATH, PLAN with
OUTER/INNER and UNION/CROSS, PLAN DEFAULT, table-level ON ERROR and
PASSING;
- the SQL/JSON constructors and predicates: JSON_OBJECT, JSON_ARRAY,
JSON_OBJECTAGG, JSON_ARRAYAGG, JSON(), JSON_SCALAR, JSON_SERIALIZE
and IS JSON.

Its expected output shows the current behavior. 0002 is the behavior
keyword fix, and its diff of the expected output shows, for example,
"$2 ARRAY ON EMPTY" becoming "EMPTY ARRAY ON EMPTY", and three
identical "JSON_EXISTS(js, $1 $2 ON ERROR)" entries becoming TRUE,
FALSE and UNKNOWN ON ERROR. The new tests showed that the table-level
ON ERROR of JSON_TABLE was affected too, and 0002 fixes that as well.
0003 is the JSON_TABLE jumbling, and its diff shows the merged entries
being split: for example, eight queries with various column types and
behaviors were in a single entry, and so were PLAN (p0 OUTER p1) and
PLAN (p0 INNER p1). Each patch passes the tests on its own.

The constructors and predicates are fine as they are: queries that
differ in ABSENT ON NULL, WITH UNIQUE KEYS, RETURNING or FORMAT JSON
already get different query IDs, and the keywords are kept in the
normalized text.

Note that none of this is backpatchable, as this impacts the Node
definitions and the query IDs computed.

Agreed for 0003. 0002 changes neither: it only leaves the location of
the behavior Consts unset, and locations are recorded, not hashed, so
the query IDs stay the same. But it's your call whether that's worth
back-patching.

--
Ilya Staroverov

On Wed, Oct 21, 2026 at 09:04:09AM +0000, Staroverov Ilja wrote:

Agreed for 0003. 0002 changes neither: it only leaves the location of
the behavior Consts unset, and locations are recorded, not hashed, so
the query IDs stay the same. But it's your call whether that's worth
back-patching.

v1 offers a nice coverage. I have noticed a couple of holes here and
there, but globally that was fine.

+ 2 | SELECT JSON_VALUE(js, $1 $2 ON EMPTY ERROR ON ERROR) FROM pgss_sqljson_tab

Urg, yeah. In this one $2 is a DEFAULT keyword. That's confusing..
Sami has mentioned that on another thread. It does not prevent doing
something with the tests first; query jumbing can sometimes feel
tricky to feel (not be!) right.

+     2 | SELECT JSON_VALUE(js, $1 DEFAULT $2 ON EMPTY) FROM pgss_sqljson_tab
+     1 | SELECT JSON_VALUE(js, $1 DEFAULT $2 ON ERROR) FROM pgss_sqljson_tab

These ones are separated due to the different last keywords, and
perhaps more arguable, but it's confusing to replace the
NULL/DEFAULT/EMPTY by dollar parameters. And we have quite a few more
of these incorrect patterns.

These SQL/JSON patterns are so various that adding a mention of the
defaults is a good idea. I've expanded that a bit more.

The ON ERROR and ON EMPTY patterns can also be combined together in a
single query. I've added more of them.

under NESTED, PATH is optional. Having patterns where it's gone seems
necessary to me. Similarly, no NESTED in a PLAN is the same a no
PLAN.

There were a few more patterns that were not treated in plans with
combinations of UNION, INNER, CROSS.. Things like UNION/UNION and
CROSS/CROSS included.

And a few more notes for things missing:
* WITHOUT UNIQUE
* JSON_ARRAYAGG(ORDER BY [DESC])
* JSON_TABLE(UNKNOWN ON ERROR)
* JSON_SERIALIZE(FORMAT)
* PASSING in JSON_QUERY

No backpatch can happen on that, I am afraid. The query ID stability
matters on stable branches, even if some if the keywords are currently
eaten due to the locations.
--
Michael

Hi Michael,

Thanks for expanding and committing the tests.

No backpatch can happen on that, I am afraid.

Understood.

Attached is v5, rebased on top of ae75d12b5, without the tests patch.
The only code change since v4 is in makeJsonTablePathScan() in 0002,
which now gets the location of the path from pathspec->location.

but it's confusing to replace the
NULL/DEFAULT/EMPTY by dollar parameters. And we have quite a few more
of these incorrect patterns.

0001 fixes all of them in the new tests, for example:

$1 $2 ON EMPTY ERROR ON ERROR
-> $1 NULL ON EMPTY ERROR ON ERROR

$1 $2 ARRAY ON EMPTY $3 OBJECT ON ERROR
-> $1 EMPTY ARRAY ON EMPTY EMPTY OBJECT ON ERROR

and the three "JSON_EXISTS(js, $1 $2 ON ERROR)" entries become TRUE,
FALSE and UNKNOWN ON ERROR. The entries and their call counts stay the
same, only the texts change. After 0001, the only parameter symbols
left in the ON EMPTY and ON ERROR clauses are the DEFAULT values, as
in "DEFAULT $2 ON EMPTY", where $2 is the value itself (1 or 2 in the
tests), a constant in the query text.

0002 is the JSON_TABLE jumbling. Its diff shows the merged entries
being split: for example, the ten queries with different column types
and behaviors were in a single entry, and so were the thirteen with
EXISTS, FORMAT JSON and FOR ORDINALITY columns. Queries with
equivalent plans are still merged: the one without a PLAN clause with
PLAN (p0 OUTER (p1 UNION p2)), and PLAN (p0 INNER (p1 UNION p2)) with
PLAN DEFAULT (INNER) and PLAN DEFAULT (UNION, INNER). So are queries
that differ only in column names, path names, or PATH after NESTED.
The exception is columns without PATH, as in "COLUMNS (a int)" and
"COLUMNS (b int)": their paths are derived from the names, so the
names are part of the query ID in that case.

One split in that diff may look unnecessary: "a jsonb PATH" and
"a jsonb FORMAT JSON PATH". Their JsonExprs differ only in the format
of the RETURNING clause, as do JSON_QUERY(js, '$' RETURNING jsonb) and
the same with FORMAT JSON, which already get different query IDs, so
I left that alone.

--
Ilya Staroverov