> On Fri, Feb 14, 2025 at 09:06:24AM GMT, Sami Imseih wrote:
> I think it will be sad to not include this very common case from
> the start, because it is going to be one of the most common
> cases.
>
> Wouldn't doing something like this inside IsMergeableConst
> """
> if (!IsA(arg, Const) && !IsA(arg, Param))
> """
>
> instead of
> """
> if (!IsA(arg, Const))
> """
>
> be sufficient?
That's exactly what the original rejected implementation was doing. I
guess to answer this question fully I need to do some more detailed
investigation, I'm probably not aware about everything at this point.
> I perhap meant "missing chunk" instead of "trimming". To me it just
> looked like a trimmed text, which was wrong. Looks like v25
> deals with that better at least. I am just not sure about all that we are doing
> here as I believe it may open up big changes for bugs generating the normalized
> query texts. I'm a bit worried about that. IMO, we are better off just
> adding a comment
> at the start of a query that this query text such as "/*
> query_id_squash_values */"
> and keeping all the parameter symbols in-place.
I see what you mean, but keeping everything in place is partially
defeating purpose of the patch. The idea is not only to make those
queries to have the same query_id, but also to reduce the size of
queries themselves. E.g. the use case scenario that has triggered the
patch was about queries having dozens of thousands of such constants,
so that the size of them was a burden on its own.
