1 files changed, 74 insertions, 74 deletions

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index bbf355dbf2c..680085096ae 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -1495,7 +1495,7 @@ inheritance_planner(PlannerInfo *root)
 		 * If this child rel was excluded by constraint exclusion, exclude it
 		 * from the result plan.
 		 */
-		if (IS_DUMMY_PATH(subpath))
+		if (IS_DUMMY_REL(sub_final_rel))
 			continue;
 
 		/*
@@ -3987,12 +3987,10 @@ create_ordinary_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
 	 * If this is the topmost grouping relation or if the parent relation is
 	 * doing some form of partitionwise aggregation, then we may be able to do
 	 * it at this level also.  However, if the input relation is not
-	 * partitioned, partitionwise aggregate is impossible, and if it is dummy,
-	 * partitionwise aggregate is pointless.
+	 * partitioned, partitionwise aggregate is impossible.
 	 */
 	if (extra->patype != PARTITIONWISE_AGGREGATE_NONE &&
-		input_rel->part_scheme && input_rel->part_rels &&
-		!IS_DUMMY_REL(input_rel))
+		IS_PARTITIONED_REL(input_rel))
 	{
 		/*
 		 * If this is the topmost relation or if the parent relation is doing
@@ -6817,27 +6815,48 @@ apply_scanjoin_target_to_paths(PlannerInfo *root,
 							   bool scanjoin_target_parallel_safe,
 							   bool tlist_same_exprs)
 {
-	ListCell   *lc;
+	bool		rel_is_partitioned = IS_PARTITIONED_REL(rel);
 	PathTarget *scanjoin_target;
-	bool		is_dummy_rel = IS_DUMMY_REL(rel);
+	ListCell   *lc;
 
+	/* This recurses, so be paranoid. */
 	check_stack_depth();
 
 	/*
+	 * If the rel is partitioned, we want to drop its existing paths and
+	 * generate new ones.  This function would still be correct if we kept the
+	 * existing paths: we'd modify them to generate the correct target above
+	 * the partitioning Append, and then they'd compete on cost with paths
+	 * generating the target below the Append.  However, in our current cost
+	 * model the latter way is always the same or cheaper cost, so modifying
+	 * the existing paths would just be useless work.  Moreover, when the cost
+	 * is the same, varying roundoff errors might sometimes allow an existing
+	 * path to be picked, resulting in undesirable cross-platform plan
+	 * variations.  So we drop old paths and thereby force the work to be done
+	 * below the Append, except in the case of a non-parallel-safe target.
+	 *
+	 * Some care is needed, because we have to allow generate_gather_paths to
+	 * see the old partial paths in the next stanza.  Hence, zap the main
+	 * pathlist here, then allow generate_gather_paths to add path(s) to the
+	 * main list, and finally zap the partial pathlist.
+	 */
+	if (rel_is_partitioned)
+		rel->pathlist = NIL;
+
+	/*
 	 * If the scan/join target is not parallel-safe, partial paths cannot
 	 * generate it.
 	 */
 	if (!scanjoin_target_parallel_safe)
 	{
 		/*
-		 * Since we can't generate the final scan/join target, this is our
-		 * last opportunity to use any partial paths that exist.  We don't do
-		 * this if the case where the target is parallel-safe, since we will
-		 * be able to generate superior paths by doing it after the final
-		 * scan/join target has been applied.
-		 *
-		 * Note that this may invalidate rel->cheapest_total_path, so we must
-		 * not rely on it after this point without first calling set_cheapest.
+		 * Since we can't generate the final scan/join target in parallel
+		 * workers, this is our last opportunity to use any partial paths that
+		 * exist; so build Gather path(s) that use them and emit whatever the
+		 * current reltarget is.  We don't do this in the case where the
+		 * target is parallel-safe, since we will be able to generate superior
+		 * paths by doing it after the final scan/join target has been
+		 * applied.
 		 */
 		generate_gather_paths(root, rel, false);
 
@@ -6846,61 +6865,27 @@ apply_scanjoin_target_to_paths(PlannerInfo *root,
 		rel->consider_parallel = false;
 	}
 
-	/*
-	 * Update the reltarget.  This may not be strictly necessary in all cases,
-	 * but it is at least necessary when create_append_path() gets called
-	 * below directly or indirectly, since that function uses the reltarget as
-	 * the pathtarget for the resulting path.  It seems like a good idea to do
-	 * it unconditionally.
-	 */
-	rel->reltarget = llast_node(PathTarget, scanjoin_targets);
-
-	/* Special case: handle dummy relations separately. */
-	if (is_dummy_rel)
-	{
-		/*
-		 * Since this is a dummy rel, it's got a single Append path with no
-		 * child paths.  Replace it with a new path having the final scan/join
-		 * target.  (Note that since Append is not projection-capable, it
-		 * would be bad to handle this using the general purpose code below;
-		 * we'd end up putting a ProjectionPath on top of the existing Append
-		 * node, which would cause this relation to stop appearing to be a
-		 * dummy rel.)
-		 */
-		rel->pathlist = list_make1(create_append_path(root, rel, NIL, NIL,
-													  NULL, 0, false, NIL,
-													  -1));
+	/* Finish dropping old paths for a partitioned rel, per comment above */
+	if (rel_is_partitioned)
 		rel->partial_pathlist = NIL;
-		set_cheapest(rel);
-		Assert(IS_DUMMY_REL(rel));
-
-		/*
-		 * Forget about any child relations.  There's no point in adjusting
-		 * them and no point in using them for later planning stages (in
-		 * particular, partitionwise aggregate).
-		 */
-		rel->nparts = 0;
-		rel->part_rels = NULL;
-		rel->boundinfo = NULL;
-
-		return;
-	}
 
 	/* Extract SRF-free scan/join target. */
 	scanjoin_target = linitial_node(PathTarget, scanjoin_targets);
 
 	/*
-	 * Adjust each input path.  If the tlist exprs are the same, we can just
-	 * inject the sortgroupref information into the existing pathtarget.
-	 * Otherwise, replace each path with a projection path that generates the
-	 * SRF-free scan/join target.  This can't change the ordering of paths
-	 * within rel->pathlist, so we just modify the list in place.
+	 * Apply the SRF-free scan/join target to each existing path.
+	 *
+	 * If the tlist exprs are the same, we can just inject the sortgroupref
+	 * information into the existing pathtargets.  Otherwise, replace each
+	 * path with a projection path that generates the SRF-free scan/join
+	 * target.  This can't change the ordering of paths within rel->pathlist,
+	 * so we just modify the list in place.
 	 */
 	foreach(lc, rel->pathlist)
 	{
 		Path	   *subpath = (Path *) lfirst(lc);
-		Path	   *newpath;
 
+		/* Shouldn't have any parameterized paths anymore */
 		Assert(subpath->param_info == NULL);
 
 		if (tlist_same_exprs)
@@ -6908,17 +6893,18 @@ apply_scanjoin_target_to_paths(PlannerInfo *root,
 				scanjoin_target->sortgrouprefs;
 		else
 		{
+			Path	   *newpath;
+
 			newpath = (Path *) create_projection_path(root, rel, subpath,
 													  scanjoin_target);
 			lfirst(lc) = newpath;
 		}
 	}
 
-	/* Same for partial paths. */
+	/* Likewise adjust the targets for any partial paths. */
 	foreach(lc, rel->partial_pathlist)
 	{
 		Path	   *subpath = (Path *) lfirst(lc);
-		Path	   *newpath;
 
 		/* Shouldn't have any parameterized paths anymore */
 		Assert(subpath->param_info == NULL);
@@ -6928,39 +6914,54 @@ apply_scanjoin_target_to_paths(PlannerInfo *root,
 				scanjoin_target->sortgrouprefs;
 		else
 		{
-			newpath = (Path *) create_projection_path(root,
-													  rel,
-													  subpath,
+			Path	   *newpath;
+
+			newpath = (Path *) create_projection_path(root, rel, subpath,
 													  scanjoin_target);
 			lfirst(lc) = newpath;
 		}
 	}
 
-	/* Now fix things up if scan/join target contains SRFs */
+	/*
+	 * Now, if final scan/join target contains SRFs, insert ProjectSetPath(s)
+	 * atop each existing path.  (Note that this function doesn't look at the
+	 * cheapest-path fields, which is a good thing because they're bogus right
+	 * now.)
+	 */
 	if (root->parse->hasTargetSRFs)
 		adjust_paths_for_srfs(root, rel,
 							  scanjoin_targets,
 							  scanjoin_targets_contain_srfs);
 
 	/*
-	 * If the relation is partitioned, recursively apply the same changes to
-	 * all partitions and generate new Append paths.  Since Append is not
-	 * projection-capable, that might save a separate Result node, and it also
-	 * is important for partitionwise aggregate.
+	 * Update the rel's target to be the final (with SRFs) scan/join target.
+	 * This now matches the actual output of all the paths, and we might get
+	 * confused in createplan.c if they don't agree.  We must do this now so
+	 * that any append paths made in the next part will use the correct
+	 * pathtarget (cf. create_append_path).
 	 */
-	if (rel->part_scheme && rel->part_rels)
+	rel->reltarget = llast_node(PathTarget, scanjoin_targets);
+
+	/*
+	 * If the relation is partitioned, recursively apply the scan/join target
+	 * to all partitions, and generate brand-new Append paths in which the
+	 * scan/join target is computed below the Append rather than above it.
+	 * Since Append is not projection-capable, that might save a separate
+	 * Result node, and it also is important for partitionwise aggregate.
+	 */
+	if (rel_is_partitioned)
 	{
-		int			partition_idx;
 		List	   *live_children = NIL;
+		int			partition_idx;
 
 		/* Adjust each partition. */
 		for (partition_idx = 0; partition_idx < rel->nparts; partition_idx++)
 		{
 			RelOptInfo *child_rel = rel->part_rels[partition_idx];
-			ListCell   *lc;
 			AppendRelInfo **appinfos;
 			int			nappinfos;
 			List	   *child_scanjoin_targets = NIL;
+			ListCell   *lc;
 
 			/* Translate scan/join targets for this child. */
 			appinfos = find_appinfos_by_relids(root, child_rel->relids,
@@ -6992,8 +6993,7 @@ apply_scanjoin_target_to_paths(PlannerInfo *root,
 		}
 
 		/* Build new paths for this relation by appending child paths. */
-		if (live_children != NIL)
-			add_paths_to_append_rel(root, rel, live_children);
+		add_paths_to_append_rel(root, rel, live_children);
 	}
 
 	/*