Horton

Re: pg_stat_statements and "IN" conditions

Dmitry Dolgov <9erthalion6@gmail.com>

View thread
From: Dmitry Dolgov <9erthalion6@gmail.com>
To: David Geier <geidav.pg@gmail.com>
Cc: 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>, Greg Stark <stark@mit.edu>, Pavel Trukhanov <pavel.trukhanov@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>
Date: 2023-02-11T10:47:07Z
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 Sat, Feb 11, 2023 at 11:03:36AM +0100, David Geier wrote:
> Hi,
>
> On 2/9/23 16:02, Dmitry Dolgov wrote:
> > > Unfortunately, rebase is needed again due to recent changes in queryjumblefuncs ( 9ba37b2cb6a174b37fc51d0649ef73e56eae27fc )
> I reviewed the last patch applied to some commit from Feb. 4th.

Thanks for looking. Few quick answers about high-level questions below,
the rest I'll incorporate in the new version.

> - There's a comment about find_const_walker(). I cannot find that function
> anywhere. What am I missing?
>
> [...]
>
> - Don't you intend to use the NUMERIC data column in SELECT * FROM
> test_merge_numeric WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10)? Otherwise,
> the test is identical to previous test cases and you're not checking for
> what happens with NUMERICs which are wrapped in FuncExpr because of the
> implicit coercion.
>
> - Don't we want to extend IsConstList() to allow a list of all implicitly
> coerced constants? It's inconsistent that otherwise e.g. NUMERICs don't
> work.
>
> [...]
>
> - Prepared statements are not supported as they contain INs with Param
> instead of Const nodes. While less likely, I've seen applications that use
> prepared statements in conjunction with queries generated through a UI which
> ended up with tons of prepared queries with different number of elements in
> the IN clause. Not necessarily something that must go into this patch but
> maybe worth thinking about.

The original version of the patch was doing all of this, i.e. handling
numerics, Param nodes, RTE_VALUES. The commentary about
find_const_walker in tests is referring to a part of that, that was
dealing with evaluation of expression to see if it could be reduced to a
constant.

Unfortunately there was a significant push back from reviewers because
of those features. That's why I've reduced the patch to it's minimally
useful version, having in mind re-implementing them as follow-up patches
in the future. This is the reason as well why I left tests covering all
this missing functionality -- as breadcrumbs to already discovered
cases, important for the future extensions.

> - Why do we actually only want to merge constants? Why don't we ignore the
> type of element in the IN and merge whatever is there? Is this because the
> original jumbling logic as of now only has support for constants?
>
> - Ideally we would even remove duplicates. That would even improve
> cardinality estimation but I guess we don't want to spend the cycles on
> doing so in the planner?

I believe these points are beyond the patch goals, as it's less clear
(at least to me) if it's safe or desirable to do so.