Date: Mon, 14 Mar 2022 15:57:34 +0100
From: Dmitry Dolgov <9erthalion6@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Zhihong Yu <zyu@yugabyte.com>,
	David Steele <david@pgmasters.net>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>,
	Greg Stark <stark@mit.edu>,
	Pavel Trukhanov <pavel.trukhanov@gmail.com>
Subject: Re: pg_stat_statements and "IN" conditions
Message-ID: <20220314145734.x3re4hcr6ukmppwi@ddolgov.remote.csb>
References: <20210616140212.kt3n5wi3alfz5i5d@localhost>
 <20210930134930.d4vzwhvxhfpk2zxt@localhost>
 <CALNJ-vSJvaJbcXr46JMyWFT5rO3BdaHO2E=xnAbLNDw1cyUJ-A@mail.gmail.com>
 <20210930150957.tz3h3unjyrzatw6e@localhost>
 <4154685.1641337363@sss.pgh.pa.us>
 <20220105211111.5oh7ypjfonaqubuq@erthalion.local>
 <20220310163837.pzvr4yntdk33cbkb@ddolgov.remote.csb>
 <655954.1646932319@sss.pgh.pa.us>
 <20220312141030.5panohzsskr4zpkk@erthalion.local>
 <CA+TgmoYG+nL1-3xmejYx-dm__7JMpCaptYkCVernS7wrriHQ+w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CA+TgmoYG+nL1-3xmejYx-dm__7JMpCaptYkCVernS7wrriHQ+w@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/20220314145734.x3re4hcr6ukmppwi%40ddolgov.remote.csb>
Precedence: bulk

> On Mon, Mar 14, 2022 at 10:17:57AM -0400, Robert Haas wrote:
> On Sat, Mar 12, 2022 at 9:11 AM Dmitry Dolgov <9erthalion6@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.

Well, yeah, the commit message is somewhat clumsy in this regard. It
works almost in the way you've described, except if the list is all
constants and long enough to satisfy the threshold then *first N
elements (where N == threshold) will be jumbled -- to leave at least
some traces of it in pgss.

> 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'm not sure if I follow the last point. WHERE x in (1,3) and x =
any(array[1,3]) are two different things for sure, but in which way are
they going to be mixed together because of this change? My goal was to
make only the following transformation, without leaving any uncertainty:

WHERE x in (1, 2, 3, 4, 5) -> WHERE x in (1, 2, ...)
WHERE x = any(array[1, 2, 3, 4, 5]) -> WHERE x = any(array[1, 2, ...])

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

Sure, I'll add documentation. To be honest I'm not targeting PG15 with
this, just want to make some progress. Thanks for the feedback, I'm glad
to see it coming!