force_parallel_mode = regress has a blind spot

Started by Tom Laneover 6 years ago2 messageshackers
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

I have just found out the hard way (cf commits 22864f6e0, 776a2c887)
that if one uses EXPLAIN with both ANALYZE and VERBOSE selected,
the output is not the same between force_parallel_mode = off and
force_parallel_mode = regress. This seems to me to be quite broken;
what's the point of force_parallel_mode = regress if it doesn't
produce the same output?

The reason there's a problem is that ExplainNode() will show
per-worker detail if both es->analyze and es->verbose are set,
even when the only reason there's a worker process is that
force_parallel_mode injected a supposedly-invisible Gather.

I don't see any way to fix this that doesn't involve some sort
of "action at a distance". One could imagine hiding the per-worker
detail if we're underneath a Gather that has invisible set to
true, but it's not really clear to me that that would do the
right things in a plan with multiple Gather nodes. Any thoughts
about that?

regards, tom lane

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#1)
Re: force_parallel_mode = regress has a blind spot

I wrote:

I have just found out the hard way (cf commits 22864f6e0, 776a2c887)
that if one uses EXPLAIN with both ANALYZE and VERBOSE selected,
the output is not the same between force_parallel_mode = off and
force_parallel_mode = regress. This seems to me to be quite broken;
what's the point of force_parallel_mode = regress if it doesn't
produce the same output?
The reason there's a problem is that ExplainNode() will show
per-worker detail if both es->analyze and es->verbose are set,
even when the only reason there's a worker process is that
force_parallel_mode injected a supposedly-invisible Gather.
I don't see any way to fix this that doesn't involve some sort
of "action at a distance". One could imagine hiding the per-worker
detail if we're underneath a Gather that has invisible set to
true, but it's not really clear to me that that would do the
right things in a plan with multiple Gather nodes. Any thoughts
about that?

I took a closer look and decided that I was overthinking the problem.
The current implementation of hiding invisible Gathers only works for
a single Gather at the very top of the plan tree, so there's no need
for this adjustment to be smarter than that. If we ever want to
relax that, it'll be time enough to consider the possibility of
hiding or not hiding workers in different parts of the tree.

For now, I propose the attached patch, which aside from fixing the
problem in 776a2c887 allows removal of a kluge in a different test case.

I'm not sure whether to back-patch this. There seems no immediate
need to do so, but I wonder if someone might back-patch a test case
that depends on this fix.

regards, tom lane

Attachments:

ensure-invisible-Gathers-have-invisible-workers.patchtext/x-diff; charset=us-ascii; name=ensure-invisible-Gathers-have-invisible-workers.patchDownload+28-13