Re: Pushdown target list below gather node (WAS Re: WIP: Upper planner pathification)

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> That doesn't update the cost of the subpath, which it probably needs to
> do. I wonder if this shouldn't be implemented by recursing.

> if (IsA(path, GatherPath) && !has_parallel_hazard((Node *) target->exprs,
> false))
> apply_projection_to_path(root, something, ((GatherPath *)
> path)->subpath, target);

> Tom, any comments? I think it would be smart to push this into 9.6.

I agree with the idea that this problem should be solved, but there's
nothing much to like about the particular details here. If we're going
to push down stuff into the workers, I think adding a Result if needed
to do that is something we'd definitely want it to do. So more
along the lines of

if (IsA(path, GatherPath) &&
!has_parallel_hazard((Node *) target->exprs, false))
{
GatherPath *gpath = (GatherPath *) path;

gpath->subpath =
apply_projection_to_path(root,
gpath->subpath->parent,
gpath->subpath,
target);
}

However, I continue to doubt that apply_projection_to_path really ought to
know about parallel safety.

And there is a larger problem with this: I'm not sure that it's
appropriate for apply_projection_to_path to assume that the subpath is not
shared with any other purposes. If it is shared, and we update the
subpath's target in-place, we just broke the other path chains.

Now the existing uses of apply_projection_to_path don't have to worry
about that because they're always messing with paths whose parent rel
isn't going to be used in any other way. Maybe this one doesn't either
because the subpath would always be a partial path that won't have any
other potential application besides being a child of *this specific*
Gather path. But I'd like to see that addressed in a comment, if so.

If it's not so, maybe the answer is to always interpose a Result node,
in which case just use create_projection_path() in the above fragment.

regards, tom lane