aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Rowley <drowley@postgresql.org>2022-01-25 21:14:27 +1300
committerDavid Rowley <drowley@postgresql.org>2022-01-25 21:14:27 +1300
commit357ff66153b4791d32c45724d8a0d3c91d89c937 (patch)
tree4cbab8ec2e82543f6156da198cceb66c0d40b14a
parent29d9da36b7b132476ef7b31a5dace7bc7410cc82 (diff)
downloadpostgresql-357ff66153b4791d32c45724d8a0d3c91d89c937.tar.gz
postgresql-357ff66153b4791d32c45724d8a0d3c91d89c937.zip
Consider parallel awareness when removing single-child Appends
8edd0e794 added some code to remove Append and MergeAppend nodes when they contained a single child node. As it turned out, this was unsafe to do when the Append/MergeAppend was parallel_aware and the child node was not. Removing the Append/MergeAppend, in this case, could lead to the child plan being called multiple times by parallel workers when it was unsafe to do so. Here we fix this by just not removing the Append/MergeAppend when the parallel_aware flag of the parent and child node don't match. Reported-by: Yura Sokolov Bug: #17335 Discussion: https://postgr.es/m/b59605fecb20ba9ea94e70ab60098c237c870628.camel%40postgrespro.ru Backpatch-through: 12, where 8edd0e794 was first introduced
-rw-r--r--src/backend/optimizer/plan/setrefs.c24
1 files changed, 20 insertions, 4 deletions
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 1bdf0451d50..c79a65ea4b1 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -1522,8 +1522,16 @@ set_append_references(PlannerInfo *root,
lfirst(l) = set_plan_refs(root, (Plan *) lfirst(l), rtoffset);
}
- /* Now, if there's just one, forget the Append and return that child */
- if (list_length(aplan->appendplans) == 1)
+ /*
+ * See if it's safe to get rid of the Append entirely. For this to be
+ * safe, there must be only one child plan and that child plan's parallel
+ * awareness must match that of the Append's. The reason for the latter
+ * is that the if the Append is parallel aware and the child is not then
+ * the calling plan may execute the non-parallel aware child multiple
+ * times.
+ */
+ if (list_length(aplan->appendplans) == 1 &&
+ ((Plan *) linitial(aplan->appendplans))->parallel_aware == aplan->plan.parallel_aware)
return clean_up_removed_plan_level((Plan *) aplan,
(Plan *) linitial(aplan->appendplans));
@@ -1586,8 +1594,16 @@ set_mergeappend_references(PlannerInfo *root,
lfirst(l) = set_plan_refs(root, (Plan *) lfirst(l), rtoffset);
}
- /* Now, if there's just one, forget the MergeAppend and return that child */
- if (list_length(mplan->mergeplans) == 1)
+ /*
+ * See if it's safe to get rid of the MergeAppend entirely. For this to
+ * be safe, there must be only one child plan and that child plan's
+ * parallel awareness must match that of the MergeAppend's. The reason
+ * for the latter is that the if the MergeAppend is parallel aware and the
+ * child is not then the calling plan may execute the non-parallel aware
+ * child multiple times.
+ */
+ if (list_length(mplan->mergeplans) == 1 &&
+ ((Plan *) linitial(mplan->mergeplans))->parallel_aware == mplan->plan.parallel_aware)
return clean_up_removed_plan_level((Plan *) mplan,
(Plan *) linitial(mplan->mergeplans));