1 files changed, 63 insertions, 31 deletions

diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 0ff353fa11d..8fd933fd6bd 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -2168,6 +2168,7 @@ create_projection_path(PlannerInfo *root,
 					   PathTarget *target)
 {
 	ProjectionPath *pathnode = makeNode(ProjectionPath);
+	PathTarget *oldtarget = subpath->pathtarget;
 
 	pathnode->path.pathtype = T_Result;
 	pathnode->path.parent = rel;
@@ -2184,13 +2185,46 @@ create_projection_path(PlannerInfo *root,
 	pathnode->subpath = subpath;
 
 	/*
-	 * The Result node's cost is cpu_tuple_cost per row, plus the cost of
-	 * evaluating the tlist.  There is no qual to worry about.
+	 * We might not need a separate Result node.  If the input plan node type
+	 * can project, we can just tell it to project something else.  Or, if it
+	 * can't project but the desired target has the same expression list as
+	 * what the input will produce anyway, we can still give it the desired
+	 * tlist (possibly changing its ressortgroupref labels, but nothing else).
+	 * Note: in the latter case, create_projection_plan has to recheck our
+	 * conclusion; see comments therein.
 	 */
-	pathnode->path.rows = subpath->rows;
-	pathnode->path.startup_cost = subpath->startup_cost + target->cost.startup;
-	pathnode->path.total_cost = subpath->total_cost + target->cost.startup +
-		(cpu_tuple_cost + target->cost.per_tuple) * subpath->rows;
+	if (is_projection_capable_path(subpath) ||
+		equal(oldtarget->exprs, target->exprs))
+	{
+		/* No separate Result node needed */
+		pathnode->dummypp = true;
+
+		/*
+		 * Set cost of plan as subpath's cost, adjusted for tlist replacement.
+		 */
+		pathnode->path.rows = subpath->rows;
+		pathnode->path.startup_cost = subpath->startup_cost +
+			(target->cost.startup - oldtarget->cost.startup);
+		pathnode->path.total_cost = subpath->total_cost +
+			(target->cost.startup - oldtarget->cost.startup) +
+			(target->cost.per_tuple - oldtarget->cost.per_tuple) * subpath->rows;
+	}
+	else
+	{
+		/* We really do need the Result node */
+		pathnode->dummypp = false;
+
+		/*
+		 * The Result node's cost is cpu_tuple_cost per row, plus the cost of
+		 * evaluating the tlist.  There is no qual to worry about.
+		 */
+		pathnode->path.rows = subpath->rows;
+		pathnode->path.startup_cost = subpath->startup_cost +
+			target->cost.startup;
+		pathnode->path.total_cost = subpath->total_cost +
+			target->cost.startup +
+			(cpu_tuple_cost + target->cost.per_tuple) * subpath->rows;
+	}
 
 	return pathnode;
 }
@@ -2199,38 +2233,37 @@ create_projection_path(PlannerInfo *root,
  * apply_projection_to_path
  *	  Add a projection step, or just apply the target directly to given path.
  *
- * Most plan types include ExecProject, so we can implement a new projection
- * without an extra plan node: just replace the given path's pathtarget with
- * the desired one.  If the given path can't project, add a ProjectionPath.
+ * This has the same net effect as create_projection_path(), except that if
+ * a separate Result plan node isn't needed, we just replace the given path's
+ * pathtarget with the desired one.  This must be used only when the caller
+ * knows that the given path isn't referenced elsewhere and so can be modified
+ * in-place.
  *
- * We can also short-circuit cases where the targetlist expressions are
- * actually equal; this is not an uncommon case, since it may arise from
- * trying to apply a PathTarget with sortgroupref labeling to a derived
- * path without such labeling.
+ * If the input path is a GatherPath, we try to push the new target down to
+ * its input as well; this is a yet more invasive modification of the input
+ * path, which create_projection_path() can't do.
  *
- * This requires knowing that the source path won't be referenced for other
- * purposes (e.g., other possible paths), since we modify it in-place.  Note
- * also that we mustn't change the source path's parent link; so when it is
+ * Note that we mustn't change the source path's parent link; so when it is
  * add_path'd to "rel" things will be a bit inconsistent.  So far that has
  * not caused any trouble.
  *
  * 'rel' is the parent relation associated with the result
  * 'path' is the path representing the source of data
  * 'target' is the PathTarget to be computed
- * 'target_parallel' indicates that target expressions are all parallel-safe
  */
 Path *
 apply_projection_to_path(PlannerInfo *root,
 						 RelOptInfo *rel,
 						 Path *path,
-						 PathTarget *target,
-						 bool target_parallel)
+						 PathTarget *target)
 {
 	QualCost	oldcost;
 
-	/* Make a separate ProjectionPath if needed */
-	if (!is_projection_capable_path(path) &&
-		!equal(path->pathtarget->exprs, target->exprs))
+	/*
+	 * If given path can't project, we might need a Result node, so make a
+	 * separate ProjectionPath.
+	 */
+	if (!is_projection_capable_path(path))
 		return (Path *) create_projection_path(root, rel, path, target);
 
 	/*
@@ -2247,10 +2280,11 @@ apply_projection_to_path(PlannerInfo *root,
 	/*
 	 * 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
+	 * project.  But if there is something that is not parallel-safe in the
 	 * target expressions, then we can't.
 	 */
-	if (IsA(path, GatherPath) &&target_parallel)
+	if (IsA(path, GatherPath) &&
+		!has_parallel_hazard((Node *) target->exprs, false))
 	{
 		GatherPath *gpath = (GatherPath *) path;
 
@@ -2258,14 +2292,12 @@ apply_projection_to_path(PlannerInfo *root,
 		 * 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.
-		 * (create_projection_plan will only insert a Result node if the
-		 * subpath is not projection-capable, so we only include the cost of
-		 * that node if it will actually be inserted.  This is a bit grotty
-		 * but we can improve it later if it seems important.)
+		 * references to the subpath, but better safe than sorry.
+		 *
+		 * Note that we don't change the GatherPath's cost estimates; it might
+		 * be appropriate to do so, to reflect the fact that the bulk of the
+		 * target evaluation will happen in workers.
 		 */
-		if (!is_projection_capable_path(gpath->subpath))
-			gpath->path.total_cost += cpu_tuple_cost * gpath->subpath->rows;
 		gpath->subpath = (Path *)
 			create_projection_path(root,
 								   gpath->subpath->parent,