> On Thu, May 01, 2025 at 09:29:13AM GMT, Michael Paquier wrote: > > I agree that the current solution we have in the tree feels incomplete > because we are not taking into account the most common cases that > users would care about. Now, allowing PARAM_EXTERN means that we > allow any expression to be detected as a squashable thing, and this > kinds of breaks the assumption of IsSquashableConst() where we want > only constants to be allowed because EXECUTE parameters can be any > kind of Expr nodes. At least that's the intention of the code on > HEAD. > > Now, I am not actually objecting about PARAM_EXTERN included or not if > there's a consensus behind it and my arguments are considered as not > relevant. The patch is written so as it claims that a PARAM_EXTERN > implies the expression to be a Const, but it may not be so depending > on what the execution path is given for the parameter. Or at least > the patch could be clearer and rename the parts about the "Const" > squashable APIs around queryjumblefuncs.c. > > [...] > > The PARAM_EXTERN part has been mentioned a couple of weeks ago here, > btw: > https://www.postgresql.org/message-id/CAA5RZ0tu6_KRiYJCFptf4_--wjFSu9cZMj1XNmOCqTNxu=v...@mail.gmail.com
In fact, this has been discussed much earlier in the thread above, as essentially the same implementation with T_Params, which is submitted here, was part of the original patch. The concern was always whether or not it will break any assumption about query identification, because this way much broader scope of expressions will be considered equivalent for query id computation purposes. At the same time after thinking about this concern more, I presume it already happens at a smaller scale -- when two queries happen to have the same number of parameters, they will be indistinguishable even if parameters are different in some way. > To be honest, the situation of HEAD makes me question whether we are > using the right approach for this facility. I did mention a couple of > months ago about an alternative, but it comes down to accept that any > expressions would be normalized, unfortunately I never got down to > study it in details as this touches around expr_list in the parser: we > could detect in the parser the start and end locations of an > expression list in a query string, then group all of them together > based on their location in the string. This would be also cheaper > than going through all the elements in the list, tweaking things when > dealing with a subquery.. Not entirely sure how that would work exactly, but after my experiments with the squashing patch I found it could be very useful to be able to identify the end location of an expression list in the parser.
