Date: Tue, 4 Jul 2023 21:02:56 +0200
From: Dmitry Dolgov <9erthalion6@gmail.com>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: "Gregory Stark (as CFM)" <stark.cfm@gmail.com>,
	David Geier <geidav.pg@gmail.com>, Sergei Kornilov <sk@zsrv.org>,
	Michael Paquier <michael@paquier.xyz>,
	Alvaro Herrera <alvherre@alvh.no-ip.org>,
	Marcos Pegoraro <marcos@f10.com.br>,
	vignesh C <vignesh21@gmail.com>,
	Robert Haas <robertmhaas@gmail.com>, Zhihong Yu <zyu@yugabyte.com>,
	David Steele <david@pgmasters.net>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>,
	Pavel Trukhanov <pavel.trukhanov@gmail.com>,
	Tom Lane <tgl@sss.pgh.pa.us>
Subject: Re: pg_stat_statements and "IN" conditions
Message-ID: <20230704190256.arszjcwvrjekxj25@ddolgov.remote.csb>
References: <20230211104707.grsicemegr7d3mgh@erthalion.local>
 <20230211120820.y7krq37jubnjsa5b@erthalion.local>
 <07919318-6c35-a13f-6a1d-98686a3e38de@gmail.com>
 <20230217154602.uf3a3cgqeoeysycz@erthalion.local>
 <8702fa3e-5aac-dcf0-161b-faa2b9eadad6@gmail.com>
 <20230226104619.jfmwrpd5zrkmp3eu@erthalion.local>
 <CAM-w4HO29FpoX5oYR7tuxP9sZA1s3zLV94c_79h_uQeNHUjhyQ@mail.gmail.com>
 <20230314190432.l4oe3wa3pvt5b7hx@erthalion.local>
 <20230319122734.xkba536kut3g4gcy@erthalion.local>
 <20230704044611.GA3373000@nathanxps13>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20230704044611.GA3373000@nathanxps13>
Archived-At: <https://www.postgresql.org/message-id/20230704190256.arszjcwvrjekxj25%40ddolgov.remote.csb>
Precedence: bulk

> On Mon, Jul 03, 2023 at 09:46:11PM -0700, Nathan Bossart wrote:

Thanks for reviewing.

> On Sun, Mar 19, 2023 at 01:27:34PM +0100, Dmitry Dolgov wrote:
> > +        If this parameter is on, two queries with an array will get the same
> > +        query identifier if the only difference between them is the number of
> > +        constants, both numbers is of the same order of magnitude and greater or
> > +        equal 10 (so the order of magnitude is greather than 1, it is not worth
> > +        the efforts otherwise).
>
> IMHO this adds way too much complexity to something that most users would
> expect to be an on/off switch.

This documentation is exclusively to be precise about how does it work.
Users don't have to worry about all this, and pretty much turn it
on/off, as you've described. I agree though, I could probably write this
text a bit differently.

> If I understand Álvaro's suggestion [0] correctly, he's saying that in
> addition to allowing "on" and "off", it might be worth allowing
> something like "powers" to yield roughly the behavior described above.
> I don't think he's suggesting that this "powers" behavior should be
> the only available option.

Independently of what Álvaro was suggesting, I find the "powers"
approach more suitable, because it answers my own concerns about the
previous implementation. Having "on"/"off" values means we would have to
scratch heads coming up with a one-size-fit-all default value, or to
introduce another option for the actual cut-off threshold. I would like
to avoid both of those options, that's why I went with "powers" only.

> Also, it seems counterintuitive that queries with fewer than 10
> constants are not merged.

Why? What would be your intuition using this feature?

> In the interest of moving this patch forward, I would suggest making it a
> simple on/off switch in 0002 and moving the "powers" functionality to a new
> 0003 patch.  I think separating out the core part of this feature might
> help reviewers.  As you can see, I got distracted by the complicated
> threshold logic and ended up focusing my first round of review there.

I would disagree. As I've described above, to me "powers" seems to be a
better fit, and the complicated logic is in fact reusing one already
existing function. Do those arguments sound convincing to you?