Amit Kapila <amit.kapila16@gmail.com> writes:
> On Fri, Mar 4, 2016 at 11:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> (BTW, I found what seemed to be a couple of pre-existing bugs of
>> the same kind, eg create_mergejoin_path was different from the
>> other two kinds of join as to setting parallel_degree.)
> I think the reason for keeping parallel_degree as zero for mergejoin path
> is that currently it can't participate in parallelism.
Is there some reason why hash and nestloop are safe but merge isn't?
>> + RecursiveUnionPath *
>> + create_recursiveunion_path(PlannerInfo *root,
>> + ...
>> + pathnode->path.parallel_safe =
>> + leftpath->parallel_safe && rightpath->parallel_safe;
> I think here we should use rel->consider_parallel to set parallel_safe as
> is done in create_mergejoin_path.
Well, the "rel" is going to be an upperrel that will have been
manufactured by fetch_upper_rel, and it will contain no useful
information about parallelism, so I'm not real sure what that
would buy.
This does bring up what seems to me probably a pre-existing bug in
the parallel query planning stuff: what about parallel-safe vs
parallel-unsafe functions in join quals, or other expressions that
have to be evaluated at places above the scan level? I would expect
to see upper path nodes needing to account for parallel-safety
of the specific expressions they need to execute. However, the
existing join path node types don't have any provision for this,
so I did not feel that it was incumbent on me to fix it for the
path node types I'm adding.
> + * It's only needed atop a node that doesn't support projection
> "needed atop a node", seems unclear to me, typo?
Seems perfectly good English to me.
regards, tom lane