git.postgresql.org Git - postgresql.git/commitdiff

git projects / postgresql.git / commitdiff

Prohibit pushing subqueries containing window function calculation to

author Amit Kapila <akapila@postgresql.org>

Tue, 4 Sep 2018 04:36:09 +0000 (10:06 +0530)

committer Amit Kapila <akapila@postgresql.org>

Tue, 4 Sep 2018 04:58:08 +0000 (10:28 +0530)

workers.

Allowing window function calculation in workers leads to inconsistent
results because if the input row ordering is not fully deterministic, the
output of window functions might vary across workers. The fix is to treat
them as parallel-restricted.

In the passing, improve the coding pattern in max_parallel_hazard_walker
so that it has a chain of mutually-exclusive if ... else if ... else if
... else if ... IsA tests.

Reported-by: Marko Tiikkaja
Bug: 15324
Author: Amit Kapila
Reviewed-by: Tom Lane
Backpatch-through: 9.6
Discussion: https://postgr.es/m/CAL9smLAnfPJCDUUG4ckX2iznj53V7VSMsYefzZieN93YxTNOcw@mail.gmail.com

src/backend/optimizer/util/clauses.c patch | blob | blame | history

src/test/regress/expected/select_parallel.out patch | blob | blame | history

src/test/regress/sql/select_parallel.sql patch | blob | blame | history

diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c

index a04ad6e99e61db0a8e758f256192714782a36469..ee6f4cdf4daa04a0c403249a39a052b0eb1c8e82 100644 (file)

--- a/src/backend/optimizer/util/clauses.c

+++ b/src/backend/optimizer/util/clauses.c

@@ -1190,12 +1190,26 @@ max_parallel_hazard_walker(Node *node, max_parallel_hazard_context *context)

return true;

}

- if (IsA(node, NextValueExpr))

+ else if (IsA(node, NextValueExpr))

{

if (max_parallel_hazard_test(PROPARALLEL_UNSAFE, context))

return true;

}

+ /*

+ * Treat window functions as parallel-restricted because we aren't sure

+ * whether the input row ordering is fully deterministic, and the output

+ * of window functions might vary across workers if not. (In some cases,

+ * like where the window frame orders by a primary key, we could relax

+ * this restriction. But it doesn't currently seem worth expending extra

+ * effort to do so.)

+ */

+ else if (IsA(node, WindowFunc))

+ {

+ if (max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context))

+ return true;

+ }

* As a notational convenience for callers, look through RestrictInfo.

diff --git a/src/test/regress/expected/select_parallel.out b/src/test/regress/expected/select_parallel.out

index f1b8cd43376710ee584c445cebecf0ba8168c3d8..13cdd3d5cca36f92d498f08501c62644cce18c2b 100644 (file)

--- a/src/test/regress/expected/select_parallel.out

+++ b/src/test/regress/expected/select_parallel.out

@@ -958,6 +958,32 @@ select count(*) from tenk1;

reset force_parallel_mode;

reset role;

+-- Window function calculation can't be pushed to workers.

+explain (costs off, verbose)

+ select count(*) from tenk1 a where (unique1, two) in

+ (select unique1, row_number() over() from tenk1 b);

+ QUERY PLAN

+----------------------------------------------------------------------------------------------

+ Aggregate

+ Output: count(*)

+ -> Hash Semi Join

+ Hash Cond: ((a.unique1 = b.unique1) AND (a.two = (row_number() OVER (?))))

+ -> Gather

+ Output: a.unique1, a.two

+ Workers Planned: 4

+ -> Parallel Seq Scan on public.tenk1 a

+ Output: a.unique1, a.two

+ -> Hash

+ Output: b.unique1, (row_number() OVER (?))

+ -> WindowAgg

+ Output: b.unique1, row_number() OVER (?)

+ -> Gather

+ Output: b.unique1

+ Workers Planned: 4

+ -> Parallel Index Only Scan using tenk1_unique1 on public.tenk1 b

+ Output: b.unique1

+(18 rows)

-- to increase the parallel query test coverage

SAVEPOINT settings;

SET LOCAL force_parallel_mode = 1;

diff --git a/src/test/regress/sql/select_parallel.sql b/src/test/regress/sql/select_parallel.sql

index 7771e055ab985364daac53740493ea5673d9fe28..3d1dffe61d29cbdedf6b706ee84f865592cf7781 100644 (file)

--- a/src/test/regress/sql/select_parallel.sql

+++ b/src/test/regress/sql/select_parallel.sql

@@ -362,6 +362,12 @@ select count(*) from tenk1;