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

On Thu, Sep 13, 2018 at 5:30 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Wed, Sep 5, 2018 at 10:03 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > On Sun, Sep 2, 2018 at 10:18 AM Amit Kapila <amit.kapila16@gmail.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.
>

Do you agree with my proposal to backpatch commit - 655393a022 to 9.6?

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