Date: Thu, 18 Mar 2021 09:47:29 -0400
From: Bruce Momjian <bruce@momjian.us>
To: Julien Rouhaud <rjuju123@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>,
	Pavel Stehule <pavel.stehule@gmail.com>,
	Tom Lane <tgl@sss.pgh.pa.us>,
	Alvaro Herrera <alvherre@alvh.no-ip.org>,
	Michael Paquier <michael@paquier.xyz>,
	torikoshia <torikoshia@oss.nttdata.com>,
	Atsushi Torikoshi <atorik@gmail.com>,
	Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp>,
	Tomas Vondra <tomas.vondra@2ndquadrant.com>,
	Evgeny Efimkin <efimkin@yandex-team.ru>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Feature improvement: can we add queryId for
 pg_catalog.pg_stat_activity view?
Message-ID: <20210318134729.GA8529@momjian.us>
References: <20210317151701.GA16650@momjian.us>
 <3518601.1615994918@sss.pgh.pa.us>
 <20210317154023.GB16650@momjian.us>
 <3519983.1615996898@sss.pgh.pa.us>
 <CAFj8pRBwOBo_3R3zNc+E2Z+skzRd+dwNVRLfn95R2P=qscp-SQ@mail.gmail.com>
 <20210317162444.GD16650@momjian.us>
 <20210317164907.c4uogoqvchnwcnjy@nol>
 <CA+Tgmoa5r2yxtuyHiYM3dVVEorr3vbVexYm9m+maMMiZtA25uA@mail.gmail.com>
 <20210317223216.GE16650@momjian.us>
 <20210317232956.veah6bax3mespdtq@nol>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210317232956.veah6bax3mespdtq@nol>
User-Agent: Mutt/1.10.1 (2018-07-13)
Archived-At: 
 <https://www.postgresql.org/message-id/20210318134729.GA8529%40momjian.us>
Precedence: bulk

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.

Also, should it be compute_queryid or compute_query_id?

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?

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

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