> 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.
