Robert Haas <robertmhaas@gmail.com> writes:
> One problem that I've realized that is related to this is that the way
> that the consider_parallel flag is being set for upper rels is almost
> totally bogus, which I believe accounts for your complaint at PGCon
> that force_parallel_mode was not doing as much as it ought to do.
Yeah, I just ran into a different reason to think that. I tried marking
MinMaxAggPaths in the same way as other relation-scan paths, ie
pathnode->path.parallel_safe = rel->consider_parallel;
but that did nothing because the just-created UPPERREL_GROUP_AGG rel
didn't have its consider_parallel flag set yet. Moreover, if I'd tried to
fix that by invoking set_grouped_rel_consider_parallel() from planagg.c,
it would still not work right because set_grouped_rel_consider_parallel()
is hard-wired to consider only partial aggregation, which is not what's
going on in a MinMaxAggPath. I'm not sure about the best rewrite here,
but I would urge you to make sure that consider_parallel on an upper rel
reflects only properties inherent to the rel (eg, safeness of quals that
must be applied there) and not properties of specific implementation
methods. Also, please make sure that whatever logic is involved remains
factored out where it can be called by an extension that might want to
create an upperrel sooner than the core code would do.
BTW, do we need wholePlanParallelSafe at all, and if so why? Can't
it be replaced by examining the parallel_safe flag on the selected
topmost Path?
regards, tom lane
