On Wed, Mar 16, 2016 at 12:57 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Yeah, I was thinking about the same thing. The comment block above
> where you're looking would need some adjustment.
OK, how about this?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index e37bdfd..f08f0ea 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -1396,18 +1396,21 @@ create_projection_plan(PlannerInfo *root, ProjectionPath *best_path)
tlist = build_path_tlist(root, &best_path->path);
/*
- * Although the ProjectionPath node wouldn't have been made unless its
- * pathtarget is different from the subpath's, it can still happen that
- * the constructed tlist matches the subplan's. (An example is that
- * MergeAppend doesn't project, so we would have thought that we needed a
- * projection to attach resjunk sort columns to its output ... but
- * create_merge_append_plan might have added those same resjunk sort
- * columns to both MergeAppend and its children.) So, if the desired
- * tlist is the same expression-wise as the subplan's, just jam it in
- * there. We'll have charged for a Result that doesn't actually appear in
- * the plan, but that's better than having a Result we don't need.
+ * We might not really need a Result node here. There are several ways
+ * that this can happen. For example, MergeAppend doesn't project, so we
+ * would have thought that we needed a projection to attach resjunk sort
+ * columns to its output ... but create_merge_append_plan might have
+ * added those same resjunk sort columns to both MergeAppend and its
+ * children. Alternatively, apply_projection_to_path might have created
+ * a projection path as the subpath of a Gather node even though the
+ * subpath was projection-capable. So, if the subpath is capable of
+ * projection or the desired tlist is the same expression-wise as the
+ * subplan's, just jam it in there. We'll have charged for a Result that
+ * doesn't actually appear in the plan, but that's better than having a
+ * Result we don't need.
*/
- if (tlist_same_exprs(tlist, subplan->targetlist))
+ if (is_projection_capable_path(best_path->subpath) ||
+ tlist_same_exprs(tlist, subplan->targetlist))
{
plan = subplan;
plan->targetlist = tlist;
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index b8ea316..5491f36 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -2222,6 +2222,30 @@ apply_projection_to_path(PlannerInfo *root,
path->total_cost += target->cost.startup - oldcost.startup +
(target->cost.per_tuple - oldcost.per_tuple) * path->rows;
+ /*
+ * If the path happens to be a Gather path, we'd like to arrange for the
+ * subpath to return the required target list so that workers can help
+ * project. But if there is something that is not parallel-safe in the
+ * target expressions, then we can't.
+ */
+ if (IsA(path, GatherPath) &&
+ !has_parallel_hazard((Node *) target->exprs, false))
+ {
+ GatherPath *gpath = (GatherPath *) path;
+
+ /*
+ * We always use create_projection_path here, even if the subpath is
+ * projection-capable, so as to avoid modifying the subpath in place.
+ * It seems unlikely at present that there could be any other
+ * references to the subpath anyway, but better safe than sorry.
+ */
+ gpath->subpath = (Path *)
+ create_projection_path(root,
+ gpath->subpath->parent,
+ gpath->subpath,
+ target);
+ }
+
return path;
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
