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

From: Julien Rouhaud <rjuju123(at)gmail(dot)com>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Hannu Krosing <hannuk(at)google(dot)com>, 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>, 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-04-01 18:28:02
Message-ID: 20210401182802.d7o632hxdgwita32@nol
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Apr 01, 2021 at 01:59:15PM -0400, Bruce Momjian wrote:
> On Thu, Apr 1, 2021 at 01:56:42PM -0400, Bruce Momjian wrote:
> > You are using:
> >
> > /* ----------
> > * pgstat_get_my_queryid() -
> > *
> > * Return current backend's query identifier.
> > */
> > uint64
> > pgstat_get_my_queryid(void)
> > {
> > if (!MyBEEntry)
> > return 0;
> >
> > return MyBEEntry->st_queryid;
> > }
> >
> > Looking at log_statement:
> >
> > /* Log immediately if dictated by log_statement */
> > if (check_log_statement(parsetree_list))
> > {
> > ereport(LOG,
> > (errmsg("statement: %s", query_string),
> > errhidestmt(true),
> > errdetail_execute(parsetree_list)));
> > was_logged = true;
> > }
> >
> > it uses the global variable query_string.

Unless I'm missing something query_string isn't a global variable, it's a
parameter passed to exec_simple_query() from postgresMain().

It's then passed to the stats collector to be able to be displayed in
pg_stat_activity through pgstat_report_activity() a bit like what I do for the

There's a global variable debug_query_string, but it's only for debugging

> > I wonder if the query hash
> > should be a global variable too --- this would more clearly match how we
> > handle top-level info like query_string. Digging into the stats system
> > to get top-level info does seem odd.

The main difference is that there's a single top level query_string,
even if it contains multiple statements. But there would be multiple queryid
calculated in that case and we don't want to change it during a top level
multi-statements execution, so we can't use the same approach.

Also, the query_string is directly logged from this code path, while the
queryid is logged as a log_line_prefix, and almost all the code there also
retrieve information from some shared structure.

And since it also has to be available in pg_stat_activity, having a single
source of truth looked like a better approach.

> Also, if you go in that direction, make sure the hash it set in the same
> places the query string is set, though I am unclear how extensions would
> handle that.

It should be transparent for application, it's extracting the first queryid
seen for each top level statement and export it. The rest of the code still
continue to see the queryid that corresponds to the really executed single

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2021-04-01 18:35:54 Re: libpq debug log
Previous Message Bruce Momjian 2021-04-01 17:59:15 Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?