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>, Junwang Zhao <zhjwpku@gmail.com>, Álvaro Herrera <alvherre@kurilemu.de>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-05-22T09:03:46Z
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 Wed, May 21, 2025 at 08:22:19PM GMT, Sami Imseih wrote: > > > > At the same time AFAICT there isn't much more code paths > > > > to worry about in case of a LocationExpr as a node > > > > > > I can imagine there are others like value expressions, > > > row expressions, json array expressions, etc. that we may > > > want to also normalize. > > > Exactly. When using a node, one can explicitly wrap whatever is needed > > into it, while otherwise one would need to find a new way to piggy back > > on A_Expr in a new context. > > Looking at the VALUES expression case, we will need to carry the info > with SelectStmt and ultimately to RangeTblEntry which is where the > values_list is, so either approach we take RangeTblEntry will need the > LocationExpr pointer or the additional ParseLoc info I am suggesting. > A_Expr is not used in the values list case. Right, that's precisely my point -- introducing a new node will allow to to use the same generalized mechanism in such scenarios as well, instead of every time inventing something new. > > I'll take a look at the proposed change, but a bit later. > > Here is a v4 to compare with v3. > > 0001- is the infrastructure to track the boundaries > 0002- the changes to jumbling Just to call this out, I don't think there is an agreement on squashing Params, which you have added into 0002. Let's discuss this change separately from the 18 open item. --- Here is a short summary of the open item: * An issue has been discovered with the squashing feature in 18, which can lead to invalid normalized queries in pg_stat_statement. * The proposed fix extends gram.y functionality capturing the end location for list expressions to address that. * There is a disagreement on how exactly to capture the location, the options are introducing a new node LocationExpr or piggy back on an existing A_Expr. I find the former more flexible and less invasive, but looks like there are also other opinions. Now, both flavour of the proposed solution could be still concidered too invasive to be applied as a bug fix. I personally don't see it like this, but I'm obviously biased. This leads us to following decisions to be made: * Is modifying parser (either adding a new node or modifying an existing one) acceptable at this stage? I guess it would be enough to collect couple of votes yes/no in this thread. * If it's not acceptable, the feature could be reverted in 18, and the fix could be applied to the master branch only. I'm fine with both outcomes (apply the fix to both 18 and master, or revert in 18 and apply the fix on master), and leave the decision to Álvaro (sorry for causing all the troubles). It's fair to say that reverting the feature will be the least risky move.