Horton

Re: pg_stat_statements and "IN" conditions

Michael Paquier <michael@paquier.xyz>

View thread
From: Michael Paquier <michael@paquier.xyz>
To: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Nathan Bossart <nathandbossart@gmail.com>, "Gregory Stark (as CFM)" <stark.cfm@gmail.com>, David Geier <geidav.pg@gmail.com>, Sergei Kornilov <sk@zsrv.org>, 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>
Date: 2023-10-13T08:07:00Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Introduce squashing of constant lists in query jumbling

  2. Make documentation builds reproducible

  3. Include values of A_Const nodes in query jumbling

  4. Teach planner about more monotonic window functions

  5. Split up guc.c for better build speed and ease of maintenance.

On Tue, Jul 04, 2023 at 09:02:56PM +0200, Dmitry Dolgov wrote:
> On Mon, Jul 03, 2023 at 09:46:11PM -0700, Nathan Bossart wrote:
>> 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.

FWIW, I am going to side with Nathan on this one, but not completely
either.  I was looking at the patch and it brings too much complexity
for a monitoring feature in this code path.  In my experience, I've
seen people complain about IN/ANY never strimmed down to a single
parameter in pg_stat_statements but I still have to hear from somebody
outside this thread that they'd like to reduce an IN clause depending
on the number of items, or something else.

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

Now, it doesn't mean that this approach with the "powers" will never
happen, but based on the set of opinions I am gathering on this thread
I would suggest to rework the patch as follows:
- First implement an on/off switch that reduces the lists in IN and/or
ANY to one parameter.  Simply.
- Second, refactor the powers routine.
- Third, extend the on/off switch, or just implement a threshold with
a second switch.

When it comes to my opinion, I am not seeing any objections to the
feature as a whole, and I'm OK with the first step.  I'm also OK to
keep the door open for more improvements in controlling how these
IN/ANY lists show up, but there could be more than just the number of
items as parameter (say the query size, different behaviors depending
on the number of clauses in queries, subquery context or CTEs/WITH,
etc. just to name a few things coming in mind).
--
Michael