Re: queryId constant squashing does not support prepared statements

Dmitry Dolgov <9erthalion6@gmail.com>

From: Dmitry Dolgov <9erthalion6@gmail.com>
To: Sami Imseih <samimseih@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Álvaro Herrera <alvherre@kurilemu.de>, Junwang Zhao <zhjwpku@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-05-23T14:29:45Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Fix typo in comment

  2. Make query jumbling also squash PARAM_EXTERN params

  3. Fix squashing algorithm for query texts

  4. pg_stat_statements: Fix parameter number gaps in normalized queries

> On Fri, May 23, 2025 at 09:05:54AM GMT, Sami Imseih wrote:
> > > On Thu, May 22, 2025 at 10:23:31PM GMT, Sami Imseih wrote:
> > > > This does not get squashed:
> > > > Q: select where 2 in (1, 4) and
> > > > 1 in (5, cast(7 as int), 6, (cast(8 as int)), 9, 10, (cast(8 as text))::int);
> > > > R: select where $1 in ($2 /*, ... */) and
> > > > $3 in ($4, cast($5 as int), $6, (cast($7 as int)), $8, $9, (cast($10 as text))::int)
> > >
> > > This is interesting actually. This is the behavior on HEAD, and I don't get why
> > > the first list with the casts does not get squashed, while the second one does.
> > > I will check IsSquashableConst tomorrow unless Dmitry gets to it first.
> >
> > IsSquashableConst has intentionally a limited set of test for
> > "constantness", in particular it does not recurse. The case above
> >
> >     (cast(8 as text))::int
> >
> > features two CoerceViaIO expressions one inside another, hence
> > IsSquashableConst returns false.
>
> Should we be doing something like this? to unwrap RelabelType or
> CoerceViaIO until we have a different type of node to check
> for later on. We can guard the loop and break out after x amount
> of times as well. At minimum, we should try to unwrap at least
> 2 times for some of the common real-world scenarios.
>
> What do you think?
>
> ```
> while (IsA(element, RelabelType) || IsA(element, CoerceViaIO))
> {
>        if (IsA(element, RelabelType))
>            element = (Node *) ((RelabelType *) element)->arg;
>        else if (IsA(element, CoerceViaIO))
>             element = (Node *) ((CoerceViaIO *) element)->arg;
> }
> ```

I think it's better to recursively call IsSquashableConst on the nested
expression (arg or args for FuncExpr). Something like that was done in
the original patch version and was concidered too much at that time, but
since it looks like all the past concerns are lifted, why not. Do not
forget check_stack_depth.