postgres.git / summary / log / commit / refs
commit 16ebab68862bb5d3595b8c8df083f650d9d7cd20
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Wed Apr 12 20:06:49 2017 +0000
First released in: 10.0
Avoid transferring parallel-unsafe subplans to parallel workers.
Commit 5e6d8d2bb allowed parallel workers to execute parallel-safe
subplans, but it transmitted the query's entire list of subplans to
the worker(s). Since execMain.c blindly does ExecInitNode and later
ExecEndNode on every list element, this resulted in parallel-unsafe plan
nodes nonetheless getting started up and shut down in parallel workers.
That seems mostly harmless as far as core plan node types go (but
maybe not so much for Gather?). But it resulted in postgres_fdw
opening and then closing extra remote connections, and it's likely
that other non-parallel-safe FDWs or custom scan providers would have
worse reactions.
To fix, just make ExecSerializePlan replace parallel-unsafe subplans
with NULLs in the cut-down plan tree that it transmits to workers.
This relies on ExecInitNode and ExecEndNode to do nothing on NULL
input, but they do anyway. If anything else is touching the dropped
subplans in a parallel worker, that would be a bug to be fixed.
(This thus provides a strong guarantee that we won't try to do
something with a parallel-unsafe subplan in a worker.)
This is, I think, the last fix directly occasioned by Andreas Seltenreich's
bug report of a few days ago.
Tom Lane and Amit Kapila
Discussion: https://postgr.es/m/87tw5x4vcu.fsf@credativ.de
src/backend/executor/execParallel.c | 25 +++++++++++++++++++++----
src/include/nodes/plannodes.h | 3 ++-
2 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c
index 469a32c7b0d..5fd4ee0b824 100644
--- a/src/backend/executor/execParallel.c
+++ b/src/backend/executor/execParallel.c
@@ -123,7 +123,7 @@ static char *
ExecSerializePlan(Plan *plan, EState *estate)
{
PlannedStmt *pstmt;
- ListCell *tlist;
+ ListCell *lc;
/* We can't scribble on the original plan, so make a copy. */
plan = copyObject(plan);
@@ -137,9 +137,9 @@ ExecSerializePlan(Plan *plan, EState *estate)
* accordingly. This is sort of a hack; there might be better ways to do
* this...
*/
- foreach(tlist, plan->targetlist)
+ foreach(lc, plan->targetlist)
{
- TargetEntry *tle = (TargetEntry *) lfirst(tlist);
+ TargetEntry *tle = lfirst_node(TargetEntry, lc);
tle->resjunk = false;
}
@@ -162,7 +162,24 @@ ExecSerializePlan(Plan *plan, EState *estate)
pstmt->rtable = estate->es_range_table;
pstmt->resultRelations = NIL;
pstmt->nonleafResultRelations = NIL;
- pstmt->subplans = estate->es_plannedstmt->subplans;
+
+ /*
+ * Transfer only parallel-safe subplans, leaving a NULL "hole" in the list
+ * for unsafe ones (so that the list indexes of the safe ones are
+ * preserved). This positively ensures that the worker won't try to run,
+ * or even do ExecInitNode on, an unsafe subplan. That's important to
+ * protect, eg, non-parallel-aware FDWs from getting into trouble.
+ */
+ pstmt->subplans = NIL;
+ foreach(lc, estate->es_plannedstmt->subplans)
+ {
+ Plan *subplan = (Plan *) lfirst(lc);
+
+ if (subplan && !subplan->parallel_safe)
+ subplan = NULL;
+ pstmt->subplans = lappend(pstmt->subplans, subplan);
+ }
+
pstmt->rewindPlanIDs = NULL;
pstmt->rowMarks = NIL;
pstmt->relationOids = NIL;
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index d046e628702..cba915572ef 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -68,7 +68,8 @@ typedef struct PlannedStmt
/* rtable indexes of non-leaf target relations for INSERT/UPDATE/DELETE */
List *nonleafResultRelations;
- List *subplans; /* Plan trees for SubPlan expressions */
+ List *subplans; /* Plan trees for SubPlan expressions; note
+ * that some could be NULL */
Bitmapset *rewindPlanIDs; /* indices of subplans that require REWIND */
[parent: f6f9f8a24cdf]