On 2021-Mar-31, Bruce Momjian wrote:
> On Wed, Mar 31, 2021 at 11:25:32AM +0800, Julien Rouhaud wrote:
> > On Thu, Mar 25, 2021 at 10:36:38AM +0800, Julien Rouhaud wrote:
> > > On Wed, Mar 24, 2021 at 01:02:00PM -0300, Alvaro Herrera wrote:
> > > > 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.
> >
> > So is the current approach ok? If not I'm afraid that detecting and caching
> > the top level queryid in the executor parts would lead to some code
> > duplication.
>
> I assume it is since Alvaro didn't reply. I am planning to apply this
> soon.
I'm afraid I don't know enough about how parallel query works to make a
good assessment on this being a good approach or not -- and no time at
present to figure it all out.
--
Álvaro Herrera 39°49'30"S 73°17'W
"I think my standards have lowered enough that now I think 'good design'
is when the page doesn't irritate the living f*ck out of me." (JWZ)
