public inbox for [email protected]  
help / color / mirror / Atom feed
From: Dmitry Dolgov <[email protected]>
To: Michael Paquier <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Marcos Pegoraro <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Sergei Kornilov <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Zhihong Yu <[email protected]>
Cc: David Steele <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Cc: Greg Stark <[email protected]>
Cc: Pavel Trukhanov <[email protected]>
Cc: Tom Lane <[email protected]>
Subject: Re: pg_stat_statements and "IN" conditions
Date: Sun, 5 Feb 2023 12:33:46 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

> On Sun, Feb 05, 2023 at 10:30:25AM +0900, Michael Paquier wrote:
> On Sat, Feb 04, 2023 at 06:08:41PM +0100, Dmitry Dolgov wrote:
> > Here is the rebased version. To adapt to the latest changes, I've marked
> > ArrayExpr with custom_query_jumble to implement this functionality, but
> > tried to make the actual merge logic relatively independent. Otherwise,
> > everything is the same.
>
> I was quickly looking at this patch, so these are rough impressions.
>
> +   bool        merged;         /* whether or not the location was marked as
> +                                  not contributing to jumble */
>
> This part of the patch is a bit disturbing..  We have node attributes
> to track if portions of a node should be ignored or have a location
> marked, still this "merged" flag is used as an extension to track if a
> location should be done or not.  Is that a concept that had better be
> controlled via a new node attribute?

Good question. I need to think a bit more if it's possible to leverage
node attributes mechanism, but at the moment I'm still inclined to
extend LocationLen. The reason is that it doesn't exactly serve the
tracking purpose, i.e. whether to capture a location (I have to update
the code commentary), it helps differentiate cases when locations A and
D are obtained from merging A B C D instead of just being A and D.

I'm thinking about this in the following way: the core jumbling logic is
responsible for deriving locations based on the input expressions; in
the case of merging we produce less locations; pgss have to represent
the result only using locations and has to be able to differentiate
simple locations and locations after merging.

> +--
> +-- Consts merging
> +--
> +CREATE TABLE test_merge (id int, data int);
> +-- IN queries
> +-- No merging
>
> Would it be better to split this set of tests into a new file?  FWIW,
> I have a patch in baking process that refactors a bit the whole,
> before being able to extend it so as we have more coverage for
> normalized utility queries, as of now the query strings stored by
> pg_stat_statements don't reflect that even if the jumbling computation
> marks the location of the Const nodes included in utility statements
> (partition bounds, queries of COPY, etc.).  I should be able to send
> that tomorrow, and my guess that you could take advantage of that
> even for this thread.

Sure, I'll take a look how I can benefit from your work, thanks.






view thread (12+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: pg_stat_statements and "IN" conditions
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox