Horton

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

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Stephen Frost <sfrost@snowman.net>, Marko Tiikkaja <marko@joh.to>, Andres Freund <andres@anarazel.de>, Andrew Fletcher <andy@prestigedigital.com>, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Date: 2018-09-04T06:12:21Z
Lists: pgsql-bugs

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Don't allow LIMIT/OFFSET clause within sub-selects to be pushed to workers.

  2. Back-patch "Fix parallel hash join path search."

  3. Prohibit pushing subqueries containing window function calculation to

  4. Fix parallel hash join path search.

On Mon, Sep 3, 2018 at 7:19 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Amit Kapila <amit.kapila16@gmail.com> writes:
> > You haven't mentioned anything about backpatching, but I don't see any
> > problem with backpatching this fix.
>
> Yeah, we definitely need to back-patch, and that's another reason not
> to touch the catalog contents.
>
> > I will commit the attached patches in a day or so unless somebody sees
> > any problem.
>
> Looking more closely at the patch:
>
> * The general design in max_parallel_hazard_walker appears to be that
> after the initial check_functions_in_node test, the rest of it should be
> an if ... else if ... else if ... else if ... chain of mutually-exclusive
> IsA tests.  Whoever stuck in the NextValueExpr test (possibly me?) did so
> with a tin ear, and you've duplicated that mistake here.  Please make it
> "else if", and fix the NextValueExpr test to be "else if" while at it.
> (Or else get rid of all the "else"s, but that would be a shade less
> efficient unless the compiler is very very smart.)
>
> * The plan tree for the added test case is hard to read because it's
> unclear which tenk1 scan is which.  I'd suggest adding aliases to
> clarify that, eg
>
> +explain (costs off, verbose)
> +  select count(*) from tenk1 a where (unique1, two) in
> +    (select unique1, row_number() over() from tenk1 b);
>
> HEAD patch is OK otherwise.  I didn't look at the back-branch patches.
>

Thanks for the review.  After making the changes suggested by you, I
have pushed and back-patched it till 9.6.

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