Horton

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

Tom Lane <tgl@sss.pgh.pa.us>

View thread
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Amit Kapila <amit.kapila16@gmail.com>
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-03T13:49:17Z
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.

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.

			regards, tom lane