[sqlsmith] Failed assertion in create_gather_path
Hi,
sqlsmith found a query that triggers the following assertion in master
as of 039eb6e92f:
TRAP: FailedAssertion("!(subpath->parallel_safe)", File: "pathnode.c", Line: 1813)
Backtrace and recipe against the regression database below.
regards,
Andreas
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1 0x00007f25474cf42a in __GI_abort () at abort.c:89
#2 0x0000556c14b75bb3 in ExceptionalCondition (conditionName=conditionName@entry=0x556c14d16d68 "!(subpath->parallel_safe)",
errorType=errorType@entry=0x556c14bc4dbd "FailedAssertion", fileName=fileName@entry=0x556c14d16d43 "pathnode.c",
lineNumber=lineNumber@entry=1813) at assert.c:54
#3 0x0000556c149ca01d in create_gather_path (root=root@entry=0x556c16bfb7a0, rel=rel@entry=0x7f253e36f418, subpath=0x7f253e37d9d8,
target=0x7f253e36f650, required_outer=required_outer@entry=0x0, rows=rows@entry=0x0) at pathnode.c:1813
#4 0x0000556c1498a3d7 in generate_gather_paths (root=root@entry=0x556c16bfb7a0, rel=rel@entry=0x7f253e36f418,
override_rows=override_rows@entry=false) at allpaths.c:2564
#5 0x0000556c1498a7b0 in set_rel_pathlist (root=root@entry=0x556c16bfb7a0, rel=0x7f253e36f418, rti=rti@entry=2, rte=0x556c16bfb420)
at allpaths.c:497
#6 0x0000556c1498b09d in set_base_rel_pathlists (root=<optimized out>) at allpaths.c:310
#7 make_one_rel (root=root@entry=0x556c16bfb7a0, joinlist=joinlist@entry=0x7f253e374450) at allpaths.c:180
#8 0x0000556c149abfac in query_planner (root=root@entry=0x556c16bfb7a0, tlist=tlist@entry=0x7f253e3eb4a0,
qp_callback=qp_callback@entry=0x556c149acb90 <standard_qp_callback>, qp_extra=qp_extra@entry=0x7ffe8088a200) at planmain.c:259
#9 0x0000556c149b0be5 in grouping_planner (root=root@entry=0x556c16bfb7a0, inheritance_update=inheritance_update@entry=false,
tuple_fraction=<optimized out>, tuple_fraction@entry=0) at planner.c:1914
#10 0x0000556c149b31a1 in subquery_planner (glob=glob@entry=0x556c16c234d0, parse=parse@entry=0x556c16bfaeb8,
parent_root=parent_root@entry=0x0, hasRecursion=hasRecursion@entry=false, tuple_fraction=tuple_fraction@entry=0)
at planner.c:984
#11 0x0000556c149b4356 in standard_planner (parse=0x556c16bfaeb8, cursorOptions=256, boundParams=<optimized out>) at planner.c:405
#12 0x0000556c14a680dd in pg_plan_query (querytree=0x556c16bfaeb8, cursorOptions=256, boundParams=0x0) at postgres.c:808
#13 0x0000556c14a681be in pg_plan_queries (querytrees=<optimized out>, cursorOptions=cursorOptions@entry=256,
boundParams=boundParams@entry=0x0) at postgres.c:874
#14 0x0000556c14a686a9 in exec_simple_query (
query_string=0x556c16b2b438 "...") at postgres.c:1049
#15 0x0000556c14a6a341 in PostgresMain (argc=<optimized out>, argv=argv@entry=0x556c16b56ad8, dbname=<optimized out>,
username=<optimized out>) at postgres.c:4149
#16 0x0000556c1474eac4 in BackendRun (port=0x556c16b4c030) at postmaster.c:4409
#17 BackendStartup (port=0x556c16b4c030) at postmaster.c:4081
#18 ServerLoop () at postmaster.c:1754
#19 0x0000556c149ec017 in PostmasterMain (argc=3, argv=0x556c16b257d0) at postmaster.c:1362
#20 0x0000556c1475006d in main (argc=3, argv=0x556c16b257d0) at main.c:228
set min_parallel_table_scan_size to 0;
select
66 as c0,
ref_1.cid as c1,
pg_catalog.min(
cast((select timetzcol from public.brintest limit 1 offset 3)
as timetz)) over (partition by ref_1.name order by ref_1.name) as c2,
ref_0.c as c3
from
public.prt1_l as ref_0
right join public.my_property_normal as ref_1
on (ref_0.a <= ref_0.a)
where EXISTS (
select
ref_2.y as c0,
ref_2.y as c1,
sample_0.random as c2,
ref_1.tel as c3,
ref_0.a as c4,
sample_0.random as c5,
ref_2.y as c6,
ref_2.x as c7,
case when (true <> (select pg_catalog.bool_and(n) from testxmlschema.test2)
)
and (sample_0.seqno = (select int_four from public.test_type_diff2_c3 limit 1 offset 1)
) then ref_2.y else ref_2.y end
as c8,
sample_0.seqno as c9,
ref_1.name as c10,
ref_0.a as c11,
(select nslots from public.hub limit 1 offset 2)
as c12,
ref_1.name as c13
from
public.hash_name_heap as sample_0 tablesample system (8.2)
left join public.tt6 as ref_2
on ((((cast(null as tinterval) <= (select f1 from public.tinterval_tbl limit 1 offset 79)
)
and (ref_2.y is not NULL))
or (((false)
and ((cast(null as tsquery) > (select keyword from public.test_tsquery limit 1 offset 34)
)
or ((((select pg_catalog.jsonb_agg(sl_name) from public.shoelace_obsolete)
<@ cast(null as jsonb))
or (EXISTS (
select
100 as c0,
ref_0.a as c1,
sample_0.seqno as c2,
ref_0.a as c3,
sample_0.seqno as c4,
ref_0.a as c5,
(select a from public.prt3_n limit 1 offset 30)
as c6,
ref_2.y as c7,
ref_1.cid as c8,
ref_2.y as c9
from
public.num_exp_mul as sample_1 tablesample system (7.1)
where true
limit 89)))
and (cast(null as _aclitem) @> cast(null as aclitem)))))
and ((select timecol from public.brintest limit 1 offset 96)
cast(null as "time"))))
and (cast(null as timestamptz) < cast(null as "timestamp")))
where ((EXISTS (
select
sample_2.int_four as c0,
sample_0.seqno as c1,
43 as c2
from
public.test_type_diff2_c1 as sample_2 tablesample bernoulli (2.3)
where (sample_0.random ~~ ref_1.name)
and (ref_2.y <> ref_2.y)
limit 98))
and (sample_0.random is NULL))
and (cast(null as point) <@ (select b from public.quad_box_tbl limit 1 offset 5)
)
limit 61);
Andreas Seltenreich wrote:
Hi,
sqlsmith found a query that triggers the following assertion in master
as of 039eb6e92f:TRAP: FailedAssertion("!(subpath->parallel_safe)", File: "pathnode.c", Line: 1813)
Backtrace and recipe against the regression database below.
Uh, that's pretty strange -- that patch did not touch the planner at
all, and the relcache.c changes are pretty trivial.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
Andreas Seltenreich wrote:
sqlsmith found a query that triggers the following assertion in master
as of 039eb6e92f:
TRAP: FailedAssertion("!(subpath->parallel_safe)", File: "pathnode.c", Line: 1813)
Uh, that's pretty strange -- that patch did not touch the planner at
all, and the relcache.c changes are pretty trivial.
I don't think Andreas meant that the bug is necessarily new in 039eb6e92f,
only that that's the version he happened to be testing.
regards, tom lane
Tom Lane writes:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
Andreas Seltenreich wrote:
as of 039eb6e92f:
TRAP: FailedAssertion("!(subpath->parallel_safe)", File: "pathnode.c", Line: 1813)Uh, that's pretty strange -- that patch did not touch the planner at
all, and the relcache.c changes are pretty trivial.I don't think Andreas meant that the bug is necessarily new in 039eb6e92f,
only that that's the version he happened to be testing.
Right. I'm not testing often enough yet to be able to report on commit
granularity :-). I'll try for a less ambiguos wording in future
reports.
Hi,
At some places, I have observed that we are adding a partial path even when
rel's consider_parallel is false. Due to this, the partial path added has
parallel_safe set to false and then later in create_gather_path() assertion
fails.
Attached patch to fix that.
On Sun, Apr 8, 2018 at 12:26 AM, Andreas Seltenreich <seltenreich@gmx.de>
wrote:
Tom Lane writes:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
Andreas Seltenreich wrote:
as of 039eb6e92f:
TRAP: FailedAssertion("!(subpath->parallel_safe)", File:"pathnode.c", Line: 1813)
Uh, that's pretty strange -- that patch did not touch the planner at
all, and the relcache.c changes are pretty trivial.I don't think Andreas meant that the bug is necessarily new in
039eb6e92f,
only that that's the version he happened to be testing.
Right. I'm not testing often enough yet to be able to report on commit
granularity :-). I'll try for a less ambiguos wording in future
reports.
--
Jeevan Chalke
Technical Architect, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Attachments:
0001-Add-partial-path-only-when-rel-s-consider_parallel-i.patchtext/x-patch; charset=US-ASCII; name=0001-Add-partial-path-only-when-rel-s-consider_parallel-i.patchDownload
From 86f430e35583c6bfdd43c4f31e06ab83e8404e9a Mon Sep 17 00:00:00 2001
From: Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Date: Sun, 8 Apr 2018 12:52:13 +0530
Subject: [PATCH] Add partial path only when rel's consider_parallel is true.
Otherwise, it will result in an assertion failure later in the
create_gather_path().
---
src/backend/optimizer/path/allpaths.c | 4 ++++
src/backend/optimizer/prep/prepunion.c | 2 +-
src/backend/optimizer/util/pathnode.c | 3 +++
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index c4e4db1..aa24345 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2196,6 +2196,10 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
pathkeys, required_outer));
}
+ /* If parallelism is not possible, return. */
+ if (!rel->consider_parallel || !bms_is_empty(required_outer))
+ return;
+
/* If consider_parallel is false, there should be no partial paths. */
Assert(sub_final_rel->consider_parallel ||
sub_final_rel->partial_pathlist == NIL);
diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c
index 5236ab3..f54807d 100644
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -331,7 +331,7 @@ recurse_set_operations(Node *setOp, PlannerInfo *root,
* to build a partial path for this relation. But there's no point in
* considering any path but the cheapest.
*/
- if (final_rel->partial_pathlist != NIL)
+ if (final_rel->consider_parallel && final_rel->partial_pathlist != NIL)
{
Path *partial_subpath;
Path *partial_path;
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 416b3f9..fa332de 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -770,6 +770,9 @@ add_partial_path(RelOptInfo *parent_rel, Path *new_path)
/* Check for query cancel. */
CHECK_FOR_INTERRUPTS();
+ /* Path to be added must be parallel safe. */
+ Assert(new_path->parallel_safe);
+
/*
* As in add_path, throw out any paths which are dominated by the new
* path, but throw out the new path if some existing path dominates it.
--
1.8.3.1
On Sun, Apr 8, 2018 at 1:04 PM, Jeevan Chalke
<jeevan.chalke@enterprisedb.com> wrote:
Hi,
At some places, I have observed that we are adding a partial path even when
rel's consider_parallel is false. Due to this, the partial path added has
parallel_safe set to false and then later in create_gather_path() assertion
fails.
Few Comments:
1.
@@ -2196,6 +2196,10 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
pathkeys, required_outer));
}
+ /* If parallelism is not possible, return. */
+ if (!rel->consider_parallel || !bms_is_empty(required_outer))
+ return;In this case shouldn't we set the rel's consider_parallel flag
correctly rather than avoiding adding the path to it as we do in
recurse_set_operations?
2.
@@ -331,7 +331,7 @@ recurse_set_operations(Node *setOp, PlannerInfo *root,
* to build a partial path for this relation. But there's no point in
* considering any path but the cheapest.
*/
- if (final_rel->partial_pathlist != NIL)
+ if (final_rel->consider_parallel && final_rel->partial_pathlist != NIL)What problem did you see here or is it just for additional safety?
Ideally, if the consider_parallel is false for a rel, it's partial
path list should also be NULL.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On Mon, Apr 9, 2018 at 5:52 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Sun, Apr 8, 2018 at 1:04 PM, Jeevan Chalke
<jeevan.chalke@enterprisedb.com> wrote:Hi,
At some places, I have observed that we are adding a partial path even when
rel's consider_parallel is false. Due to this, the partial path added has
parallel_safe set to false and then later in create_gather_path() assertion
fails.Few Comments:
1.
@@ -2196,6 +2196,10 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
pathkeys, required_outer));
}+ /* If parallelism is not possible, return. */ + if (!rel->consider_parallel || !bms_is_empty(required_outer)) + return;In this case shouldn't we set the rel's consider_parallel flag
correctly rather than avoiding adding the path to it
In the above sentence, I mean to say *partial* path.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On Mon, Apr 9, 2018 at 5:52 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Sun, Apr 8, 2018 at 1:04 PM, Jeevan Chalke
<jeevan.chalke@enterprisedb.com> wrote:Hi,
At some places, I have observed that we are adding a partial path even
when
rel's consider_parallel is false. Due to this, the partial path added has
parallel_safe set to false and then later in create_gather_path()assertion
fails.
Few Comments:
1.
@@ -2196,6 +2196,10 @@ set_subquery_pathlist(PlannerInfo *root,
RelOptInfo *rel,
pathkeys, required_outer));
}+ /* If parallelism is not possible, return. */ + if (!rel->consider_parallel || !bms_is_empty(required_outer)) + return;In this case shouldn't we set the rel's consider_parallel flag
correctly rather than avoiding adding the path to it as we do in
recurse_set_operations?
In recurse_set_operations() we are building a new rel and setting its
properties from the final_rel. consider_parallel there is just copied from
the final_rel.
However, in set_subquery_pathlist(), rel is the input parameter here and we
are trying to add a partial path to it without looking at its
consider_parallel field. This patch does that.
And if we want to set consider_parallel for the rel, then it should have
been done prior to this function itself. And I am not sure where that
exactly but not in this function I guess.
2. @@ -331,7 +331,7 @@ recurse_set_operations(Node *setOp, PlannerInfo *root, * to build a partial path for this relation. But there's no point in * considering any path but the cheapest. */ - if (final_rel->partial_pathlist != NIL) + if (final_rel->consider_parallel && final_rel->partial_pathlist != NIL)What problem did you see here or is it just for additional safety?
Ideally, if the consider_parallel is false for a rel, it's partial
path list should also be NULL.
I actually wanted to have rel->consider_parallel in the condition (yes, for
additional safety) as we are adding a partial path into rel. But then
observed that it is same as that of final_rel->consider_parallel and thus
used it along with other condition.
I have observed at many places that we do check consider_parallel flag
before adding a partial path to it. Thus for consistency added here too,
but yes, it just adds an additional safety here.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Jeevan Chalke
Technical Architect, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
On Tue, Apr 10, 2018 at 2:59 AM, Jeevan Chalke
<jeevan.chalke@enterprisedb.com> wrote:
I actually wanted to have rel->consider_parallel in the condition (yes, for
additional safety) as we are adding a partial path into rel. But then
observed that it is same as that of final_rel->consider_parallel and thus
used it along with other condition.I have observed at many places that we do check consider_parallel flag
before adding a partial path to it. Thus for consistency added here too, but
yes, it just adds an additional safety here.
Thanks to Andreas for reporting this issue and to Jeevan for fixing
it. My commit 0927d2f46ddd4cf7d6bf2cc84b3be923e0aedc52 seems clearly
to blame.
The change to set_subquery_pathlist() looks correct, but can we add a
simple test case? Right now if I keep the new Assert() in
add_partial_path() and leave out the rest of the changes, the
regression tests still pass. That's not so good. Also, I think I
would be inclined to wrap the if-statement around the rest of the
function instead of returning early.
The new Assert() in add_partial_path() is an excellent idea. I had
the same thought before, but I didn't do anything about it. That was
a bad idea; your plan is better. In fact, I suggest an additional
Assert() that any relation to which we're adding a partial path is
marked consider_parallel, like this:
+ /* Path to be added must be parallel safe. */
+ Assert(new_path->parallel_safe);
+
+ /* Relation should be OK for parallelism, too. */
+ Assert(parent_rel->consider_parallel);Regarding recurse_set_operations, since the rel->consider_parallel is
always the same as final_rel->consider_parallel there's really no
value in testing it. If it were possible for rel->consider_parallel
to be false even when final_rel->consider_parallel were true then the
test would be necessary for correctness. That might or might not
happen in the future, so I guess it wouldn't hurt to include this for
future-proofing, but it's not actually a bug fix, so it also wouldn't
hurt to leave it out. I could go either way, I guess.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Tue, Apr 10, 2018 at 11:14 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Apr 10, 2018 at 2:59 AM, Jeevan Chalke
<jeevan.chalke@enterprisedb.com> wrote:I actually wanted to have rel->consider_parallel in the condition (yes,
for
additional safety) as we are adding a partial path into rel. But then
observed that it is same as that of final_rel->consider_parallel and thus
used it along with other condition.I have observed at many places that we do check consider_parallel flag
before adding a partial path to it. Thus for consistency added here too,but
yes, it just adds an additional safety here.
Thanks to Andreas for reporting this issue and to Jeevan for fixing
it. My commit 0927d2f46ddd4cf7d6bf2cc84b3be923e0aedc52 seems clearly
to blame.The change to set_subquery_pathlist() looks correct, but can we add a
simple test case?
I have tried adding simple testcase in this version of the patch. This test
hits the Assertion added in add_partial_path() like you have tried.
Right now if I keep the new Assert() in
add_partial_path() and leave out the rest of the changes, the
regression tests still pass. That's not so good. Also, I think I
would be inclined to wrap the if-statement around the rest of the
function instead of returning early.
OK.
Wrapped if block instead of returning mid-way.
The new Assert() in add_partial_path() is an excellent idea. I had
the same thought before, but I didn't do anything about it. That was
a bad idea; your plan is better. In fact, I suggest an additional
Assert() that any relation to which we're adding a partial path is
marked consider_parallel, like this:+ /* Path to be added must be parallel safe. */ + Assert(new_path->parallel_safe); + + /* Relation should be OK for parallelism, too. */ + Assert(parent_rel->consider_parallel);
Yep.
Added this new one too.
Regarding recurse_set_operations, since the rel->consider_parallel is
always the same as final_rel->consider_parallel there's really no
value in testing it. If it were possible for rel->consider_parallel
to be false even when final_rel->consider_parallel were true then the
test would be necessary for correctness. That might or might not
happen in the future, so I guess it wouldn't hurt to include this for
future-proofing,
In that case, we should have rel in a condition rather than final_rel as we
are adding a path into rel. For future-proofing added check on
lateral_relids too.
but it's not actually a bug fix, so it also wouldn't
hurt to leave it out. I could go either way, I guess.--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Jeevan Chalke
Technical Architect, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Attachments:
0001-Add-partial-path-only-when-rel-s-consider_parallel-i.patchtext/x-patch; charset=US-ASCII; name=0001-Add-partial-path-only-when-rel-s-consider_parallel-i.patchDownload
From 708026f34f6a5d087660930f9cb05aa3e08c130b Mon Sep 17 00:00:00 2001
From: Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Date: Wed, 11 Apr 2018 14:08:13 +0530
Subject: [PATCH] Add partial path only when rel's consider_parallel is true.
Otherwise, it will result in an assertion failure later in the
create_gather_path().
---
src/backend/optimizer/path/allpaths.c | 41 +++++++++++++++------------
src/backend/optimizer/prep/prepunion.c | 3 +-
src/backend/optimizer/util/pathnode.c | 6 ++++
src/test/regress/expected/select_parallel.out | 19 +++++++++++++
src/test/regress/sql/select_parallel.sql | 6 ++++
5 files changed, 56 insertions(+), 19 deletions(-)
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 3ba3f87..c3d0634 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2242,26 +2242,31 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
pathkeys, required_outer));
}
- /* If consider_parallel is false, there should be no partial paths. */
- Assert(sub_final_rel->consider_parallel ||
- sub_final_rel->partial_pathlist == NIL);
-
- /* Same for partial paths. */
- foreach(lc, sub_final_rel->partial_pathlist)
+ /* If outer rel allows parallelism, do same for partial paths. */
+ if (rel->consider_parallel && bms_is_empty(required_outer))
{
- Path *subpath = (Path *) lfirst(lc);
- List *pathkeys;
-
- /* Convert subpath's pathkeys to outer representation */
- pathkeys = convert_subquery_pathkeys(root,
- rel,
- subpath->pathkeys,
- make_tlist_from_pathtarget(subpath->pathtarget));
+ /* If consider_parallel is false, there should be no partial paths. */
+ Assert(sub_final_rel->consider_parallel ||
+ sub_final_rel->partial_pathlist == NIL);
- /* Generate outer path using this subpath */
- add_partial_path(rel, (Path *)
- create_subqueryscan_path(root, rel, subpath,
- pathkeys, required_outer));
+ /* Same for partial paths. */
+ foreach(lc, sub_final_rel->partial_pathlist)
+ {
+ Path *subpath = (Path *) lfirst(lc);
+ List *pathkeys;
+
+ /* Convert subpath's pathkeys to outer representation */
+ pathkeys = convert_subquery_pathkeys(root,
+ rel,
+ subpath->pathkeys,
+ make_tlist_from_pathtarget(subpath->pathtarget));
+
+ /* Generate outer path using this subpath */
+ add_partial_path(rel, (Path *)
+ create_subqueryscan_path(root, rel, subpath,
+ pathkeys,
+ required_outer));
+ }
}
}
diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c
index 8d86e98..61d0770 100644
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -330,7 +330,8 @@ recurse_set_operations(Node *setOp, PlannerInfo *root,
* to build a partial path for this relation. But there's no point in
* considering any path but the cheapest.
*/
- if (final_rel->partial_pathlist != NIL)
+ if (rel->consider_parallel && bms_is_empty(rel->lateral_relids) &&
+ final_rel->partial_pathlist != NIL)
{
Path *partial_subpath;
Path *partial_path;
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index bd9442c..561466c 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -770,6 +770,12 @@ add_partial_path(RelOptInfo *parent_rel, Path *new_path)
/* Check for query cancel. */
CHECK_FOR_INTERRUPTS();
+ /* Path to be added must be parallel safe. */
+ Assert(new_path->parallel_safe);
+
+ /* Relation should be OK for parallelism, too. */
+ Assert(parent_rel->consider_parallel);
+
/*
* As in add_path, throw out any paths which are dominated by the new
* path, but throw out the new path if some existing path dominates it.
diff --git a/src/test/regress/expected/select_parallel.out b/src/test/regress/expected/select_parallel.out
index fb209de..a07cd50 100644
--- a/src/test/regress/expected/select_parallel.out
+++ b/src/test/regress/expected/select_parallel.out
@@ -955,4 +955,23 @@ ORDER BY 1, 2, 3;
------------------------------+---------------------------+-------------+--------------
(0 rows)
+-- test interation between subquery and partial_paths
+SET LOCAL min_parallel_table_scan_size TO 0;
+CREATE VIEW tenk1_vw_sec WITH (security_barrier) AS SELECT * FROM tenk1;
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM tenk1_vw_sec WHERE EXISTS (SELECT 1 WHERE unique1 = 0);
+ QUERY PLAN
+-------------------------------------------------------------------
+ Subquery Scan on tenk1_vw_sec
+ Filter: (alternatives: SubPlan 1 or hashed SubPlan 2)
+ -> Gather
+ Workers Planned: 4
+ -> Parallel Index Only Scan using tenk1_unique1 on tenk1
+ SubPlan 1
+ -> Result
+ One-Time Filter: (tenk1_vw_sec.unique1 = 0)
+ SubPlan 2
+ -> Result
+(10 rows)
+
rollback;
diff --git a/src/test/regress/sql/select_parallel.sql b/src/test/regress/sql/select_parallel.sql
index ac26d68..7db75b0 100644
--- a/src/test/regress/sql/select_parallel.sql
+++ b/src/test/regress/sql/select_parallel.sql
@@ -383,4 +383,10 @@ ORDER BY 1;
SELECT * FROM information_schema.foreign_data_wrapper_options
ORDER BY 1, 2, 3;
+-- test interation between subquery and partial_paths
+SET LOCAL min_parallel_table_scan_size TO 0;
+CREATE VIEW tenk1_vw_sec WITH (security_barrier) AS SELECT * FROM tenk1;
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM tenk1_vw_sec WHERE EXISTS (SELECT 1 WHERE unique1 = 0);
+
rollback;
--
1.8.3.1
On Wed, Apr 11, 2018 at 6:00 AM, Jeevan Chalke
<jeevan.chalke@enterprisedb.com> wrote:
[ new patch ]
Committed. Apologies for the delay.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company