Re: Inadequate parallel-safety check for SubPlans

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Wed, Apr 12, 2017 at 2:41 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> This is 100% wrong. It's failing to recurse into the subexpressions of
>> the SubPlan, which could very easily include parallel-unsafe function
>> calls.

> My understanding (apparently flawed?) is that the parallel_safe flag
> on the SubPlan is supposed to encapsulate whether than SubPlan is
> entirely parallel safe, so that no recursion is needed. If the
> parallel_safe flag on the SubPlan is being set wrong, doesn't that
> imply that the Path from which the subplan was created had the wrong
> value of this flag, too?

> ...

> Oh, I see. The testexpr is separate from the Path. Oops.

Right. Although you're nudging up against an alternate idea that
I just had: we could define the parallel_safe flag on the SubPlan as
encapsulating not only whether the child plan is safe, but whether
the contained testexpr (and args) are safe. If we were to make
an is_parallel_safe() check on the testexpr before injecting
PARAM_EXEC Params into it, and record the results in the SubPlan,
maybe that would lead to a simpler answer.

OTOH, that might not work out nicely; and I have a feeling that
we'd end up needing a whitelist anyway later, to handle the
case of correlated subplans.

> Sounds reasonable. Do you want to have a go at that?

Yeah. I'm knocking off for the day a bit early today, but I'll have
a look at it tomorrow, unless anyone in the Far East beats me to it.

regards, tom lane