Re: BUG #15324: Non-deterministic behaviour from parallelised sub-query

On Wed, Sep 5, 2018 at 10:03 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Sun, Sep 2, 2018 at 10:18 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> > > Yeah, let me summarize the problems which require patches:
>> > > (a) Consider the presence of a LIMIT/OFFSET in a sub-select as making
>> > > it parallel-unsafe.
>> > >
>> >
>> > As mentioned up-thread, I have considered adding a check in
>> > max_parallel_hazard_walker, but it turns out that it will make the
>> > whole query parallel-unsafe even if one of the sub-selects has
>> > Limit/Offset. I think the better idea is to detect that during
>> > set_rel_consider_parallel. Attached patch
>> > prohibit_parallel_limit_subselect_v2 implements the fix for same.
>> >
>>
>> I was trying this patch on back-branches and found that it doesn't
>> apply cleanly beyond PG11, so created separate patches for 10 and 9.6.
>> Further, I found that the test for this patch was not failing for
>> 9.6 (without the patch) even though the code doesn't deal with this
>> problem. On further investigation, I found that it is because the
>> commit
>> 655393a022bd653e2b48dbf20b69236981e35195 has not been backpatched to
>> 9.6. I don't see any reason why we shouldn't backpatch this commit.
>> So, I have attached a patch (fix_parallel_hash_path_v1.patch) which we
>> can backpatch in 9.6.
>>
>> Robert, your input will be highly appreciated here especially for the
>> back patch (to 9.6) I am proposing?
>>
>
> I have rebased the HEAD patch and done some cosmetic changes like
> improved the test by giving aliases to table names and modified the
> comment a bit, otherwise, the core logic remains the same. As the
> back-branch patches are just the matter of rebasing them, I will do
> that before commit.
>
> I am still waiting for input, but if there is none, my plan is to
> commit this in a day or two and back-patch it as well. Along with
> this, I would also like to back-patch commit
> 655393a022bd653e2b48dbf20b69236981e35195 for the reasons mentioned
> above.

I have reviewed and tested the patch. The patch looks fine to me and
behaviour is as expected.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com