On Sat, Mar 12, 2022 at 9:11 AM Dmitry Dolgov <9erthali...@gmail.com> wrote: > Here is the limited version of list collapsing functionality, which > doesn't utilize eval_const_expressions and ignores most of the stuff > except ArrayExprs. Any thoughts/more suggestions?
The proposed commit message says this commit intends to "Make Consts contribute nothing to the jumble hash if they're part of a series and at position further that specified threshold." I'm not sure whether that's what the patch actually implements because I can't immediately understand the new logic you've added, but I think if we did what that sentence said then, supposing the threshold is set to 1, it would result in producing the same hash for "x in (1,2)" that we do for "x in (1,3)" but a different hash for "x in (2,3)" which does not sound like what we want. What I would have thought we'd do is: if the list is all constants and long enough to satisfy the threshold then nothing in the list gets jumbled. I'm a little surprised that there's not more context-awareness in this code. It seems that it applies to every ArrayExpr found in the query, which I think would extend to cases beyond something = IN(whatever). In particular, any use of ARRAY[] in the query would be impacted. Now, the comments seem to imply that's pretty intentional, but from the user's point of view, WHERE x in (1,3) and x = any(array[1,3]) are two different things. If anything like this is to be adopted, we certainly need to be precise about exactly what it is doing and which cases are covered. I thought of looking at the documentation to see whether you'd tried to clarify this there, and found that you hadn't written any. In short, I think this patch is not really very close to being in committable shape even if nobody were objecting to the concept. -- Robert Haas EDB: http://www.enterprisedb.com
