Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?

On Thu, Mar 18, 2021 at 09:47:29AM -0400, Bruce Momjian wrote:
> On Thu, Mar 18, 2021 at 07:29:56AM +0800, Julien Rouhaud wrote:
> > On Wed, Mar 17, 2021 at 06:32:16PM -0400, Bruce Momjian wrote:
> > > OK, is that what everyone wants? I think that is what the patch already
> > > does.
> >
> > Note exactly. Right now a custom queryid can be computed even if
> > compute_queryid is off, if some extension does that in post_parse_analyze_hook.
> >
> > I'm assuming that what Robert was thinking was more like:
> >
> > if (compute_queryid)
> > {
> > if (queryid_hook)
> > queryId = queryid_hook(...);
> > else
> > queryId = JumbeQuery(...);
> > }
> > else
> > queryId = 0;
> >
> > And that should be done *after* post_parse_analyse_hook so that it's clear that
> > this hook is no longer the place to compute queryid.
> >
> > Is that what should be done?
>
> No, I don't think so. I think having extensions change behavior
> controlled by GUCs is a bad interface.
>
> The docs are going to say that you have to enable compute_queryid to see
> the query id in pg_stat_activity and log_line_prefix, but if you install
> an extension, the query id will be visible even if you don't have
> compute_queryid enabled. I think you need to only honor the hook if
> compute_queryid is enabled, and update the pg_stat_statements docs to
> say you have to enable compute_queryid for pg_stat_statements to work.

I'm confused, what you described really looks like what I described.

Let me try to clarify:

- if compute_queryid is off, a queryid should never be seen no matter how hard
an extension tries

- if compute_queryid is on, the calculation will be done by the core
(using pgss JumbeQuery) unless an extension computed one already. The only
way to know what algorithm is used is to check the list of extension loaded.

- if some extension calculates a queryid during post_parse_analyze_hook, we
will always reset it.

Is that the approach you want?

Note that the only way to not honor the hook is iff the new GUC is disabled is
to have a new queryid_hook, as we can't stop calling post_parse_analyze_hook if
the new GUC is off, and we don't want to pay the queryid calculation overhead
if the admin explicitly said it wasn't needed.

> Also, should it be compute_queryid or compute_query_id?

Maybe compute_query_identifier?

> Also, the overhead of computing the query id was reported as 2% --- that
> seems quite high for what it does. Do we know why it is so high?

The 2% was a worst case scenario, for a query with a single join over
ridiculously small pg_class and pg_attribute, in read only. The whole workload
was in shared buffers so the planning and execution is quite fast. Adding some
complexity in the query really limited the overhead.

Note that this was done on an old laptop with quite slow CPU. Maybe
someone with a better hardware than a 5/6yo laptop could get some more
realistic results (I unfortunately don't have anything to try on).