Re: Inadequate parallel-safety check for SubPlans

On Mon, Apr 17, 2017 at 10:54 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> writes:
>> On Sun, Apr 16, 2017 at 9:30 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> FYI, I have this on my to-look-at list, and expect to fix it before Robert
>>> returns from vacation.
>
>> Let me know if an initial patch by someone else can be helpful?
>
> Sure, have a go at it. I won't get to this for a day or so ...
>

I have ended up doing something along the lines suggested by you (or
at least what I have understood from your e-mail). Basically, pass
the safe-param-ids list to parallel safety function and decide based
on that if Param node reference in input expression is safe. Note
that I have added a new parameter paramids list to build_subplan to
make AlternativeSubPlan case consistent with SubPlan case even though
we should be able to do without that as it seems to be exercised only
for the correlated EXISTS case. Also, I think we don't need to check
parallel-safety for splan->args as that also is related to correlated
queries for which parallelism is not supported as of now.

I have also explored it to do it by checking parallel-safety of
testexpr before PARAM_EXEC params are replaced in testexpr, however
that won't work because we add PARAM_SUBLINK params at the time of
parse-analysis in transformSubLink(). I am not sure if it is a good
idea to test parallel-safety of testexpr at the time of parse-analysis
phase, so didn't pursue that path.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com