Re: queryId constant squashing does not support prepared statements

On Thu, May 08, 2025 at 03:50:32PM -0500, Sami Imseih wrote:
> On Thu, May 8, 2025 at 2:36 PM Dmitry Dolgov <9erthalion6(at)gmail(dot)com> wrote:
>> To clarify, I had in mind something like in the attached patch. The
>> idea is to make start/end location capturing relatively independent from
>> the constants squashing. The new parsing node conveys the location
>> information, which is then getting transformed to be a part of an
>> ArrayExpr. It's done for in_expr only here, something similar would be
>> needed for array_expr as well. Feedback is appreciated.
>
> Thanks! I took a quick look at v1-0001 and it feels like a much better approach
> than the quick hack I put together earlier. I will look thoroughly.

SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
- query | calls
-----------------------------------------------------+-------
- SELECT ARRAY[$1 /*, ... */] | 1
- SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1
+ query | calls
+-------------------------------------------------------+-------
+ SELECT ARRAY[$1, $2, $3, $4, $5, $6, $7, $8, $9, $10] | 1
+ SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1
(2 rows)

Yes, we are going to need more than that for such cases if we want to
cover all the ground we're aiming for.

Putting that aside, the test coverage for ARRAY[] elements is also
very limited on HEAD with one single test only with a set of
constants. We really should improve that, tracking more patterns and
more mixed combinations to see what gets squashed and what is not. So
this should be extended with more cases, including expressions,
parameters and sublinks, with checks on pg_stat_statements.calls to
see how the counters are aggregated. That's going to be important
when people play with this code to track how things change when
manipulating the element jumbling. I'd suggest to do that separately
of the rest.
--
Michael