Re: [COMMITTERS] pgsql: Simplify plpgsql's check for simple expressions.

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Tue, Aug 15, 2017 at 6:40 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> (In fact, a quick look shows a counterexample: if we pick a MinMaxAgg
>> path, that's parallel unsafe, but the original query might've been
>> completely safe.)

> I'm quite confused here. What's parallel-unsafe about a MinMaxAgg?
> There might be some reason why it's parallel-restricted, but it
> shouldn't be parallel-unsafe.

Well, it has subplans, so formally I think it's restricted not unsafe
--- but the parallel_safe marking on constructed paths/plans is only
safe/not-safe, not a three-way. By the time we get back up to
standard_planner() and are considering whether to plaster a Gather on
top, it doesn't really matter whether that subtree is unsafe or merely
restricted; either way, you can't send it to a worker.

> More generally, there is no way for parallelModeOK to go from true to
> false after it's initially set.

Sure, and it's not necessary, because that's just a plan-time flag
indicating whether it's worth the trouble to look for parallel plans.
It is not an indicator that we will or must end up choosing a parallel
plan. parallelModeNeeded is a different animal: it's a planner output
(which AFAICS is never consulted within the planner, so there's no need
to set it early) telling the executor whether to do
EnterParallelMode/ExitParallelMode.

> If there were, it would be a bug,
> because we might plan some subquery thinking that parallelModeOK is
> true, use a Gather node, and then later plan some other subquery that
> changes to parallelModeOK from true to false, making the plan that's
> already written in stone no longer valid. This is exactly why we have
> to have max_parallel_hazard() walk the ENTIRE query tree, including
> all subqueries, before we get started.

> Planning can obviously introduce elements into the query that prevent
> parallelism from being used for that part of the query, and the only
> thing there is to make sure that such things never make it into a
> partial path. But it can't just decide that parallelism is no longer
> allowed *anywhere* in the query.

These statements are true, but none of them seem at all relevant to
my point.

regards, tom lane