On Wed, Mar 17, 2021 at 12:24:44PM -0400, Bruce Momjian wrote:
> On Wed, Mar 17, 2021 at 05:16:50PM +0100, Pavel Stehule wrote:
> >
> >
> > st 17. 3. 2021 v 17:03 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
> >
> > Bruce Momjian <bruce@momjian.us> writes:
> > > On Wed, Mar 17, 2021 at 11:28:38AM -0400, Tom Lane wrote:
> > >> I still say that it's a serious mistake to sanctify a query ID
> > calculation
> > >> method that was designed only for pg_stat_statement's needs as the one
> > >> true way to do it. But that's what exposing it in a core view would do.
> >
> > > OK, I am fine with creating a new method, and maybe having
> > > pg_stat_statements use it. Is that the direction we should be going in?
> >
> > The point is that we've understood Query.queryId as something that
> > different extensions might calculate differently for their own needs.
> > In particular it's easy to imagine extensions that want an ID that is
> > less fuzzy than what pg_stat_statements wants. We never had a plan for
> > how two such extensions could co-exist, but at least it was possible
> > to use one if you didn't use another. If this gets moved into core
> > then there will basically be only one way that anyone can do it.
> >
> > Maybe what we need is a design for allowing more than one query ID.
> >
> >
> > Theoretically there can be a hook for calculation of queryid, that can be by
> > used extension. Default can be assigned with a method that is used by
> > pg_stat_statements.
>
> Yes, that is what the code patch says it does.
>
> > I don't think it is possible to use more different query id for
> > pg_stat_statements so this solution can be simple.
>
> Agreed.
Actually, putting the query identifer computation in the core makes it way more
tunable, even if it's conterintuitive. What it means is that you can now chose
to use usual pgss' algorithm or a different one for log_line_prefix and
pg_stat_activity.queryid, but also that you can now use pgss with a different
query id algorithm. That's another thing that user were asking for a long
time.
I originally suggested to make it clearer by having an enum GUC rather than a
boolean, say compute_queryid = [ none | core | external ], and if set to
external then a hook would be explicitely called. Right now, "none" and
"external" are binded with compute_queryid = off, and depends on whether an
extension is computing a queryid during post_parse_analyse_hook.
It could later be extended to suit other needs if we ever come to some
agreement (for instance "legacy", "logical_replication_stable" or whatever
better name we can find for something that doesn't depend on Oid).