Re: make Gather node projection-capable - Mailing list pgsql-hackers

From Robert Haas
Subject Re: make Gather node projection-capable
Date
Msg-id CA+Tgmoby4UG3720a=Dyt7Yjv-fBGG7+T0VFd1sjw=66LOHbdWA@mail.gmail.com
Whole thread Raw
In response to Re: make Gather node projection-capable  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Responses Re: make Gather node projection-capable  (Simon Riggs <simon@2ndQuadrant.com>)
List pgsql-hackers
On Thu, Oct 22, 2015 at 3:18 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
>> You probably would, but sometimes that might not be possible; for
>> example, the tlist might contain a parallel-restricted function (which
>> therefore has to run in the leader).
>
> I don't understand your reply.  Failing to parallelize the child node
> does not prevent it from doing the projection itself, does it?

OK, now I'm confused.  If you can't parallelize the child, there's no
Gather node, and this discussion is irrelevant.  The case that matters
is something like:

SELECT pg_backend_pid(), a, b, c FROM foo WHERE x = 1;

What happens today is that the system first produces a SeqScan plan
with an all-Var tlist, which may either be a physical tlist or the
exact columns needed.  Then, after join planning, which is trivial
here, we substitute for that tlist the output tlist of the plan.
Since SeqScan is projection-capable, we just emit a single-node plan
tree: SeqScan.  But say we choose a PartialSeqScan plan.  The tlist
can't be pushed down because pg_backend_pid() must run in the leader.
So, if Gather can't do projection, we'll end up with
Result->Gather->PartialSeqScan.  If we make Gather projection-capable,
we can just end up with Gather->PartialSeqScan.

I prefer the second outcome.  TBH, the major reason is that I've just
been experimenting with injecting single-copy Gather nodes into Plan
trees above the join nest and below any upper plan nodes that get
stuck on top and seeing which regression tests fail.  Since we do a
lot of EXPLAIN-ing in the plan, I hacked EXPLAIN not to show the
Gather nodes.  But it does show the extra Result nodes which get
generated because Gather isn't projection-capable, and that causes a
huge pile of spurious test failures.  Even with the patch I posted
applies, there are some residual failures that don't look simple to
resolve, because sometimes an initplan or subplan attaches to the
Gather node since something is being projected there.  So if you just
have EXPLAIN look through those nodes and show their children instead,
you still don't get the same plans you would without the test code in
all cases, but it helps a lot.

> That said, I don't understand Tom's comment either.  Surely the planner
> is going to choose to do the projection in the innermost node possible,
> so that the children nodes are going to do the projections most of the
> time.  But if for whatever reason this fails to happen, wouldn't it make
> more sense to do it at Gather than having to put a Result on top?

The planner doesn't seem to choose to do projection in the innermost
node possible.  The final tlist only gets projected at the top of the
join tree.  Beneath that, it seems like we project in order to avoid
carrying Vars through nodes where that would be a needless expense,
but that's just dropping columns, not computing anything.  That having
been said, I don't think that takes anything away from your chain of
reasoning here, and I agree with your conclusion.  There seems to be
little reason to force a Result node atop a Gather node when we don't
do that for most other node types.

Also, the patch I'm proposing I think actually makes things quite a
bit cleaner than the status quo, because the current code initializes
the projection info and then ignores the projection info itself, while
using the slot that gets set up by initializing the projection info.
That just seems goofy.  If there's some reason not to do what I'm
proposing here, I'm happy to do whatever we agree is better, but I
don't think leaving it the way it is now makes any sense.

> Projections are a weird construct as implemented, yeah.  I imagine it's
> because of performance reasons, because having separate Result nodes
> everywhere would be a lot slower, wouldn't it?

I'm not sure.  It'd be my guess that this is why it wasn't made a
separate node to begin with, but I don't know.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



pgsql-hackers by date:

Previous
From: dinesh kumar
Date:
Subject: Re: [PROPOSAL] DIAGNOSTICS = SKIPPED_ROW_COUNT
Next
From: Peter Eisentraut
Date:
Subject: Re: [PATCH] SQL function to report log message