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

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Julien Rouhaud <rjuju123(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz>, torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>, Atsushi Torikoshi <atorik(at)gmail(dot)com>, Tatsuro Yamada <tatsuro(dot)yamada(dot)tf(at)nttcom(dot)co(dot)jp>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Evgeny Efimkin <efimkin(at)yandex-team(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?
Date: 2021-03-19 13:29:06
Message-ID: 20210319132906.GG3572@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Mar 19, 2021 at 11:16:50AM +0800, Julien Rouhaud wrote:
> On Thu, Mar 18, 2021 at 03:23:49PM -0400, Bruce Momjian wrote:
> > On Fri, Mar 19, 2021 at 02:06:56AM +0800, Julien Rouhaud wrote:
> >
> > The above text is the part that made me think an extension could display
> > a query id even if disabled by the GUC.
>
> With the last version of the patch I sent it was the case.
>
> > Oh, OK. I can see an extension setting the query id on its own --- we
> > can't prevent that from happening. It is probably enough to tell
> > extensions to honor the GUC, since they would want it enabled so it
> > displays in pg_stat_activity and log_line_prefix.
>
> Ok. So no new hook, and we keep using post_parse_analyze_hook as the official
> way to have custom queryid implementation, with this new behavior:
>
> > > - if some extension calculates a queryid during post_parse_analyze_hook, we
> > > will always reset it.
> >
> > OK, good.
>
> Now that I'm back on the code I remember why I did it this way. It's
> unfortunately not really possible to make things work this way.
>
> pg_stat_statements' post_parse_analyze_hook relies on a queryid already being
> computed, as it's where we know where the constants are recorded. It means:
>
> - we have to call post_parse_analyze_hook *after* doing core queryid
> calculation
> - if users want to use a third party module to calculate a queryid, they'll
> have to make sure that the module's post_parse_analyze_hook is called
> *before* pg_stat_statements' one.
> - even if they do so, they'll still have to pay the price of core queryid
> calculation

OK, that makes perfect sense. I think the best solution is to document
that compute_query_id just controls the built-in computation of the
query id, and that extensions can also compute it if this is off, and
pg_stat_activity and log_line_prefix will display built-in or extension
computed query ids.

It might be interesting someday to check if the hook changed a
pre-computed query id and warn the user in the logs, but that could
cause more log-spam problems than help. I am a little worried that
someone might have compute_query_id enabled and then install an
extension that overwrites it, but we will just have to document this
issue. Hopefully extensions will be clear that they are computing their
own query id.

--
Bruce Momjian <bruce(at)momjian(dot)us> https://momjian.us
EDB https://enterprisedb.com

If only the physical world exists, free will is an illusion.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Prabhat Sahu 2021-03-19 13:46:55 Re: [Patch] ALTER SYSTEM READ ONLY
Previous Message Hannu Krosing 2021-03-19 13:14:24 Re: pl/pgsql feature request: shorthand for argument and local variable references