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 <seltenre...@gmx.de>
wrote:
> Tom Lane writes:
> > Alvaro Herrera <alvhe...@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
From 86f430e35583c6bfdd43c4f31e06ab83e8404e9a Mon Sep 17 00:00:00 2001
From: Jeevan Chalke <jeevan.cha...@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
