Date: Thu, 25 Mar 2021 10:36:38 +0800
From: Julien Rouhaud <rjuju123@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Bruce Momjian <bruce@momjian.us>, Hannu Krosing <hannuk@google.com>,
	Robert Haas <robertmhaas@gmail.com>,
	Pavel Stehule <pavel.stehule@gmail.com>,
	Tom Lane <tgl@sss.pgh.pa.us>, 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: <20210325023638.rb5534543nkradyf@nol>
References: <20210324152049.cjv2op3jsgauutwv@nol>
 <20210324160200.GA21214@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210324160200.GA21214@alvherre.pgsql>
Archived-At: 
 <https://www.postgresql.org/message-id/20210325023638.rb5534543nkradyf%40nol>
Precedence: bulk

On Wed, Mar 24, 2021 at 01:02:00PM -0300, Alvaro Herrera wrote:
> On 2021-Mar-24, Julien Rouhaud wrote:
> 
> > From e08c9d5fc86ba722844d97000798de868890aba3 Mon Sep 17 00:00:00 2001
> > From: Bruce Momjian <bruce@momjian.us>
> > Date: Mon, 22 Mar 2021 17:43:23 -0400
> > Subject: [PATCH v20 2/3] Expose queryid in pg_stat_activity and
> 
> >  src/backend/executor/execMain.c               |   9 ++
> >  src/backend/executor/execParallel.c           |  14 ++-
> >  src/backend/executor/nodeGather.c             |   3 +-
> >  src/backend/executor/nodeGatherMerge.c        |   4 +-
> 
> Hmm...
> 
> I find it odd that there's executor code that acquires the current query
> ID from pgstat, after having been put there by planner or ExecutorStart
> itself.  Seems like a modularity violation.  I wonder if it would make
> more sense to have the value maybe in struct EState (or perhaps there's
> a better place -- but I don't think they have a way to reach the
> QueryDesc anyhow), put there by ExecutorStart, so that places such as
> execParallel, nodeGather etc don't have to fetch it from pgstat but from
> EState.

The current queryid is already available in the Estate, as the underlying
PlannedStmt contains it.  The problem is that we want to display the top level
queryid, not the current query one, and the top level queryid is held in
pgstat.