Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Jun 27, 2016 at 4:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> * Not following what you did to apply_projection_to_path, and the comment
>> therein isn't helping.
> Gee, I wonder why not? :-)
> The basic problem here is that applying a projection to a path can
> render a formerly parallel-safe path no longer parallel-safe. If we
> jam a parallel-restricted target list into a formerly parallel-safe
> path, we'd better also clear path->parallel_safe. Currently,
> apply_projection_to_path only needs to call has_parallel_hazard for an
> input which is a GatherPath, which isn't too expensive because most
> paths are not GatherPaths and if we get one that is, well, we can hope
> parallel query will win enough during execution to make up for the
> extra planning cost. But if we want the consider_parallel and
> parallel_safe flags to be set correctly for all upper rels and paths,
> it seems that we need to do it always - hence the dismayed comment.
> Thoughts?
Seems to me that it should generally be the case that consider_parallel
would already be clear on the parent rel if the tlist isn't parallel safe,
and if it isn't we probably have a bug elsewhere. If it makes you feel
better, maybe you could add Assert(!has_parallel_hazard(...)) here?
I guess this means that apply_projection_to_path would need to clear
parallel_safe if rel->consider_parallel isn't true. This would correspond
to situations where we are taking a parallel-safe path for a lower-level
relation that has consider_parallel true, and repurposing it for a new
upperrel that has consider_parallel false. Maybe it'd be better to
not do that but just force use of a separate ProjectionPath if the
parallel_safe flag needs to change.
(I think 8b9d323cb may have made this a little less messy than it
was when you did your draft patch, btw.)
regards, tom lane