Re: fixing consider_parallel for upper planner rels

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Mon, Jun 27, 2016 at 3:35 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Oh, I thought you were still actively working on it. What patch do
>> you want me to review?

> I'm looking for an opinion on the WIP patch attached to:
> https://www.postgresql.org/message-id/CA+TgmoZwJB9EaiBj-MEeAQ913WkKOz4aQ7VQnCfrDLs9WYhZdQ@mail.gmail.com

OK, I took a quick look. Some of the details are obsolete due to my
over-the-weekend hacking on parallel aggregation, but I think generally
you have the right idea that we should set upperrel consider_parallel
flags on the basis of "input rel has consider_parallel = true AND there
are no parallel hazards in operations to be performed at this level".
A few specific comments:

* Can't we remove wholePlanParallelSafe altogether, in favor of just
examining best_path->parallel_safe in standard_planner?

* In grouping_planner, I'm not exactly convinced that
final_rel->consider_parallel can just be copied up without any further
restrictions. Easiest counterexample is a parallel restricted function in
LIMIT.

* Why have the try_parallel_path flag at all in create_grouping_paths,
rather than just setting or not setting grouped_rel->consider_parallel?
If your thinking is that this is dealing with implementation restrictions
that might not apply to paths added by an extension, then OK, but the
variable needs to have a less generic name. Maybe try_parallel_aggregation.

* Copy/paste error in comment in create_distinct_paths: s/window/distinct/

* Not following what you did to apply_projection_to_path, and the comment
therein isn't helping.

> It may not be correct in detail, but I'd like to know whether you
> think it's going in the generally correct direction and what major
> concerns you might have before spending more time on it. Also, I'd
> like to know whether you think it's something we should try to put
> into 9.6 or whether you think we should leave it for next cycle.

I think we should try to get this right in 9.6. For one thing, the
more stuff we can easily exercise in parallel mode, the more likely
we are to find bugs before they reach the field.

regards, tom lane