Subject: Re: Multiple Query IDs for a rewritten parse tree

From: Julien Rouhaud <[email-masked]>
Date: Sat, 29 Jan 2022 15:51:33 +0800

Hi,

On Fri, Jan 28, 2022 at 05:51:56PM +0100, Dmitry Dolgov wrote:
> > On Fri, Jan 21, 2022 at 11:33:22AM +0500, Andrey V. Lepikhov wrote:
> > On 1/9/22 5:49 AM, Tom Lane wrote:
> > > The idea I'd been vaguely thinking about is to allow attaching a list
> > > of query-hash nodes to a Query, where each node would contain a "tag"
> > > identifying the specific hash calculation method, and also the value
> > > of the query's hash calculated according to that method.  We could
> > > probably get away with saying that all such hash values must be uint64.
> > > The main difference from your function-OID idea, I think, is that
> > > I'm envisioning the tags as being small integers with well-known
> > > values, similarly to the way we manage stakind values in pg_statistic.
> > > In this way, an extension that wants a hash that the core knows how
> > > to calculate doesn't need its own copy of the code, and similarly
> > > one extension could publish a calculation method for use by other
> > > extensions.
> >
> > To move forward, I have made a patch that implements this idea (see
> > attachment). It is a POC, but passes all regression tests.
> [...]
> > 4. We should reserve position of default in-core generator
> 
> From the discussion above I was under the impression that the core
> generator should be distinguished by a predefined kind.

I don't really like this approach.  IIUC, this patch as-is is meant to break
pg_stat_statements extensibility.  If kind == 0 is reserved for in-core queryid
then you can't use pg_stat_statement with a different queryid generator
anymore.  Unless you meant that kind == 0 is reserved for monitoring /
pg_stat_statement purpose and custom extension should register that specific
kind too if that's what they are supposed to implement?

The patch also reserves kind == -1 for pg_stat_statements internal purpose, and
I'm not really sure why that's needed.  Are additional extension that want to
agree with pg_stat_statements on what the monitoring queryid is
supposed to do that too?

I'm also unsure of how are extensions supposed to cooperate in general, as
I feel that queryids should be implemented for some "intent" (like monitoring,
planning optimization...).  That being said I'm not sure if e.g. AQO heuristics
are too specific for its need or if it could be shared with other extension
that might be dealing with similar concerns.

---

From: Dmitry Dolgov <[email-masked]>
Date: Sat, 29 Jan 2022 18:12:05 +0100

> On Sat, Jan 29, 2022 at 03:51:33PM +0800, Julien Rouhaud wrote:
> Hi,
>

Assuming there are multiple providers and consumers of queryIds, every
such consumer extension needs to know which type of queryId it wants to
use. E.g. in case of pg_stat_statements, it needs to be somehow
configured to know which of those kinds to take, to preserve
extensibility you're talking about. Does the answer make sense, or did
you mean something else?

Btw, the approach in this thread still doesn't give a clue what to do
when an extension needs to reuse some parts of core queryId generator,
as in case with pg_stat_statements and "IN" condition merging.

---

From: Julien Rouhaud <[email-masked]>
Date: Sun, 30 Jan 2022 01:48:20 +0800

Hi,

> extensibility you're talking about. Does the answer make sense, or did

I guess, but I don't think that the proposed approach does that.

The DBA should be able to configure a monitoring queryid provider, a planning
queryid provider... and the extensions should have a way to know which is
which.  And also I don't think that the DBA should be allowed to setup multiple
monitoring queryid providers, nor change them dynamically.

> as in case with pg_stat_statements and "IN" condition merging.

Indeed, as the query text normalization is not extensible.