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 →
-
Fix typo in comment
- a3994ec6acb2 18.0 landed
-
Make query jumbling also squash PARAM_EXTERN params
- c2da1a5d6325 18.0 landed
-
Fix squashing algorithm for query texts
- 0f65f3eec478 18.0 landed
-
pg_stat_statements: Fix parameter number gaps in normalized queries
- 3c03b8cd7979 13.22 landed
- 8a1459f62ad1 14.19 landed
- 130300a15407 15.14 landed
- 7e8b44f4e0e6 16.10 landed
- 290e8ab32ac5 17.6 landed
- 35a428f30b15 18.0 landed
> 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.