Re: pg_stat_statements and "IN" conditions

From: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>
To: Sami Imseih <samimseih(at)gmail(dot)com>
Cc: Julien Rouhaud <rjuju123(at)gmail(dot)com>, Álvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Kirill Reshke <reshkekirill(at)gmail(dot)com>, Sergei Kornilov <sk(at)zsrv(dot)org>, yasuo(dot)honda(at)gmail(dot)com, tgl(at)sss(dot)pgh(dot)pa(dot)us, smithpb2250(at)gmail(dot)com, vignesh21(at)gmail(dot)com, michael(at)paquier(dot)xyz, nathandbossart(at)gmail(dot)com, stark(dot)cfm(at)gmail(dot)com, geidav(dot)pg(at)gmail(dot)com, marcos(at)f10(dot)com(dot)br, robertmhaas(at)gmail(dot)com, david(at)pgmasters(dot)net, pgsql-hackers(at)postgresql(dot)org, pavel(dot)trukhanov(at)gmail(dot)com, Sutou Kouhei <kou(at)clear-code(dot)com>
Subject: Re: pg_stat_statements and "IN" conditions
Date: 2025-02-18 09:36:36
Message-ID: alh3fgcnvbuahbw3deq2xdf3ohzjslhlwzi7jx2kvhr5taaufw@6krf6kpwdjx5
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Mon, Feb 17, 2025 at 01:50:00PM GMT, Sami Imseih wrote:
> > This test was to catch a crash that was happening in older version of
> > the patch, so it doesn't have to verify the actual pgss entry.
>
> It seems odd to keep this test because of crash behavior experienced
> in a previous version of the patch. if the crash reason was understood
> and resolved, why keep it?

A preventive measure. As you could notice, the patch has long history,
and certain issues could be accidentally reintroduced.

> > > 2/ Looking at IsMergeableConst, I am not sure why we care about
> > > things like function volatility, implicit cast or funcid > FirstGenbkiObjectId?
> >
> > Function volatility is important to establish how constant is the
> > result, for now we would like to exclude not immutable functions. The
> > implicit cast and builtin check are there to limit squashing and exclude
> > explicit or user-created functions (the second is probably an overkill,
> > but this could be gradually relatex later). Or are you not sure about
> > something different?
>
> My thoughts are when dealing with FuncExpr, if the first arg in the list of
> func->args is a Const, shouldn't that be enough to tell us that we have
> a mergeable value. If it's not a Const, it may be another FuncExpr, so
> that tells us we don't have a mergeable list. Why would this not be enough?

It's not a question about whether it's possible to implement this, but
about whether it makes sense. In case of plain constants it's
straightforward -- they will not change anything meaningfully and hence
could be squashed from the query. Now for a function, that might return
different values for the same set of constant arguments, it's much less
obvious and omitting such expressions might have unexpected consequences.

> See the attached 0001-experiement-on-top-of-v27.patch
> which applies on top of v27 and produces the results like below.

Btw, if you would like to share a code delta, please do not post it as a
patch or diff. This hijacks the CI pipeline, because CFbot thinks that's
a new version of the original patch.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Vladlen Popolitov 2025-02-18 09:47:54 Re: PoC. The saving of the compiled jit-code in the plan cache
Previous Message Yuki Seino 2025-02-18 09:33:11 Re: Add “FOR UPDATE NOWAIT” lock details to the log.