Remove [Merge]AppendPath.partitioned_rels.

It turns out that the calculation of [Merge]AppendPath.partitioned_rels in allpaths.c is faulty and sometimes omits relevant non-leaf partitions, allowing an assertion added by commit a929e17e5a to trigger. Rather than fix that, it seems better to get rid of those fields altogether. We don't really need the info until create_plan time, and calculating it once for the selected plan should be cheaper than calculating it for each append path we consider. The preceding two commits did away with all use of the partitioned_rels values; this commit just mechanically removes the fields and the code that calculated them. Discussion: https://postgr.es/m/87sg8tqhsl.fsf@aurora.ydns.eu Discussion: https://postgr.es/m/CAJKUy5gCXDSmFs2c=R+VGgn7FiYcLCsEFEuDNNLGfoha=pBE_g@mail.gmail.com
2025-08-28 18:48:04 +03:00 · 2021-02-01 14:43:54 -05:00
parent 5076f88bc9
commit f003a7522b
11 changed files with 31 additions and 187 deletions
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -99,18 +99,13 @@ static void set_append_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
 									Index rti, RangeTblEntry *rte);
 static void generate_orderedappend_paths(PlannerInfo *root, RelOptInfo *rel,
 										 List *live_childrels,
-										 List *all_child_pathkeys,
-										 List *partitioned_rels);
+										 List *all_child_pathkeys);
 static Path *get_cheapest_parameterized_child_path(PlannerInfo *root,
 												   RelOptInfo *rel,
 												   Relids required_outer);
-static List *accumulate_partitioned_rels(List *partitioned_rels,
-										 List *sub_partitioned_rels,
-										 bool flatten_partitioned_rels);
 static void accumulate_append_subpath(Path *path,
-									  List **subpaths, List **special_subpaths,
-									  List **partitioned_rels,
-									  bool flatten_partitioned_rels);
+									  List **subpaths,
+									  List **special_subpaths);
 static Path *get_singleton_append_subpath(Path *path);
 static void set_dummy_rel_pathlist(RelOptInfo *rel);
 static void set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
@@ -1299,38 +1294,11 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
 	List	   *all_child_pathkeys = NIL;
 	List	   *all_child_outers = NIL;
 	ListCell   *l;
-	List	   *partitioned_rels = NIL;
-	List	   *partial_partitioned_rels = NIL;
-	List	   *pa_partitioned_rels = NIL;
 	double		partial_rows = -1;
-	bool		flatten_partitioned_rels;

 	/* If appropriate, consider parallel append */
 	pa_subpaths_valid = enable_parallel_append && rel->consider_parallel;

-	/* What we do with the partitioned_rels list is different for UNION ALL */
-	flatten_partitioned_rels = (rel->rtekind != RTE_SUBQUERY);
-
-	/*
-	 * For partitioned tables, we accumulate a list of Relids of each
-	 * partitioned table which has at least one of its subpartitions directly
-	 * present as a subpath in this Append.  This is used later for run-time
-	 * partition pruning.  We must maintain separate lists for each Append
-	 * Path that we create as some paths that we create here can't flatten
-	 * sub-Appends and sub-MergeAppends into the top-level Append.  We needn't
-	 * bother doing this for join rels as no run-time pruning is done on
-	 * those.
-	 */
-	if (rel->reloptkind != RELOPT_JOINREL && rel->part_scheme != NULL)
-	{
-		partitioned_rels = list_make1(bms_make_singleton(rel->relid));
-		partial_partitioned_rels = list_make1(bms_make_singleton(rel->relid));
-
-		/* skip this one if we're not going to make a Parallel Append path */
-		if (pa_subpaths_valid)
-			pa_partitioned_rels = list_make1(bms_make_singleton(rel->relid));
-	}
-
 	/*
 	 * For every non-dummy child, remember the cheapest path.  Also, identify
 	 * all pathkeys (orderings) and parameterizations (required_outer sets)
@@ -1353,8 +1321,7 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
 		if (childrel->pathlist != NIL &&
 			childrel->cheapest_total_path->param_info == NULL)
 			accumulate_append_subpath(childrel->cheapest_total_path,
-									  &subpaths, NULL, &partitioned_rels,
-									  flatten_partitioned_rels);
+									  &subpaths, NULL);
 		else
 			subpaths_valid = false;

@@ -1363,9 +1330,7 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
 		{
 			cheapest_partial_path = linitial(childrel->partial_pathlist);
 			accumulate_append_subpath(cheapest_partial_path,
-									  &partial_subpaths, NULL,
-									  &partial_partitioned_rels,
-									  flatten_partitioned_rels);
+									  &partial_subpaths, NULL);
 		}
 		else
 			partial_subpaths_valid = false;
@@ -1394,10 +1359,7 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
 				Assert(cheapest_partial_path != NULL);
 				accumulate_append_subpath(cheapest_partial_path,
 										  &pa_partial_subpaths,
-										  &pa_nonpartial_subpaths,
-										  &pa_partitioned_rels,
-										  flatten_partitioned_rels);
-
+										  &pa_nonpartial_subpaths);
 			}
 			else
 			{
@@ -1416,9 +1378,7 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
 				 */
 				accumulate_append_subpath(nppath,
 										  &pa_nonpartial_subpaths,
-										  NULL,
-										  &pa_partitioned_rels,
-										  flatten_partitioned_rels);
+										  NULL);
 			}
 		}

@@ -1495,7 +1455,7 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
 	if (subpaths_valid)
 		add_path(rel, (Path *) create_append_path(root, rel, subpaths, NIL,
 												  NIL, NULL, 0, false,
-												  partitioned_rels, -1));
+												  -1));

 	/*
 	 * Consider an append of unordered, unparameterized partial paths.  Make
@@ -1538,7 +1498,7 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
 		appendpath = create_append_path(root, rel, NIL, partial_subpaths,
 										NIL, NULL, parallel_workers,
 										enable_parallel_append,
-										partial_partitioned_rels, -1);
+										-1);

 		/*
 		 * Make sure any subsequent partial paths use the same row count
@@ -1587,7 +1547,7 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
 		appendpath = create_append_path(root, rel, pa_nonpartial_subpaths,
 										pa_partial_subpaths,
 										NIL, NULL, parallel_workers, true,
-										pa_partitioned_rels, partial_rows);
+										partial_rows);
 		add_partial_path(rel, (Path *) appendpath);
 	}

@@ -1597,8 +1557,7 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
 	 */
 	if (subpaths_valid)
 		generate_orderedappend_paths(root, rel, live_childrels,
-									 all_child_pathkeys,
-									 partitioned_rels);
+									 all_child_pathkeys);

 	/*
 	 * Build Append paths for each parameterization seen among the child rels.
@@ -1617,10 +1576,6 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
 	{
 		Relids		required_outer = (Relids) lfirst(l);
 		ListCell   *lcr;
-		List	   *part_rels = NIL;
-
-		if (rel->reloptkind != RELOPT_JOINREL && rel->part_scheme != NULL)
-			part_rels = list_make1(bms_make_singleton(rel->relid));

 		/* Select the child paths for an Append with this parameterization */
 		subpaths = NIL;
@@ -1646,15 +1601,14 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
 				subpaths_valid = false;
 				break;
 			}
-			accumulate_append_subpath(subpath, &subpaths, NULL, &part_rels,
-									  flatten_partitioned_rels);
+			accumulate_append_subpath(subpath, &subpaths, NULL);
 		}

 		if (subpaths_valid)
 			add_path(rel, (Path *)
 					 create_append_path(root, rel, subpaths, NIL,
 										NIL, required_outer, 0, false,
-										part_rels, -1));
+										-1));
 	}

 	/*
@@ -1681,7 +1635,7 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
 			appendpath = create_append_path(root, rel, NIL, list_make1(path),
 											NIL, NULL,
 											path->parallel_workers, true,
-											partitioned_rels, partial_rows);
+											partial_rows);
 			add_partial_path(rel, (Path *) appendpath);
 		}
 	}
@@ -1717,26 +1671,13 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
 static void
 generate_orderedappend_paths(PlannerInfo *root, RelOptInfo *rel,
 							 List *live_childrels,
-							 List *all_child_pathkeys,
-							 List *partitioned_rels)
+							 List *all_child_pathkeys)
 {
 	ListCell   *lcp;
 	List	   *partition_pathkeys = NIL;
 	List	   *partition_pathkeys_desc = NIL;
 	bool		partition_pathkeys_partial = true;
 	bool		partition_pathkeys_desc_partial = true;
-	List	   *startup_partitioned_rels = NIL;
-	List	   *total_partitioned_rels = NIL;
-	bool		flatten_partitioned_rels;
-
-	/* Set up the method for building the partitioned rels lists */
-	flatten_partitioned_rels = (rel->rtekind != RTE_SUBQUERY);
-
-	if (rel->reloptkind != RELOPT_JOINREL && rel->part_scheme != NULL)
-	{
-		startup_partitioned_rels = list_make1(bms_make_singleton(rel->relid));
-		total_partitioned_rels = list_make1(bms_make_singleton(rel->relid));
-	}

 	/*
 	 * Some partitioned table setups may allow us to use an Append node
@@ -1878,13 +1819,9 @@ generate_orderedappend_paths(PlannerInfo *root, RelOptInfo *rel,
 				 * child paths for the MergeAppend.
 				 */
 				accumulate_append_subpath(cheapest_startup,
-										  &startup_subpaths, NULL,
-										  &startup_partitioned_rels,
-										  flatten_partitioned_rels);
+										  &startup_subpaths, NULL);
 				accumulate_append_subpath(cheapest_total,
-										  &total_subpaths, NULL,
-										  &total_partitioned_rels,
-										  flatten_partitioned_rels);
+										  &total_subpaths, NULL);
 			}
 		}

@@ -1900,7 +1837,6 @@ generate_orderedappend_paths(PlannerInfo *root, RelOptInfo *rel,
 													  NULL,
 													  0,
 													  false,
-													  startup_partitioned_rels,
 													  -1));
 			if (startup_neq_total)
 				add_path(rel, (Path *) create_append_path(root,
@@ -1911,7 +1847,6 @@ generate_orderedappend_paths(PlannerInfo *root, RelOptInfo *rel,
 														  NULL,
 														  0,
 														  false,
-														  total_partitioned_rels,
 														  -1));
 		}
 		else
@@ -1921,15 +1856,13 @@ generate_orderedappend_paths(PlannerInfo *root, RelOptInfo *rel,
 															rel,
 															startup_subpaths,
 															pathkeys,
-															NULL,
-															startup_partitioned_rels));
+															NULL));
 			if (startup_neq_total)
 				add_path(rel, (Path *) create_merge_append_path(root,
 																rel,
 																total_subpaths,
 																pathkeys,
-																NULL,
-																total_partitioned_rels));
+																NULL));
 		}
 	}
 }
@@ -2008,54 +1941,6 @@ get_cheapest_parameterized_child_path(PlannerInfo *root, RelOptInfo *rel,
 	return cheapest;
 }

-/*
- * accumulate_partitioned_rels
- *		Record 'sub_partitioned_rels' in the 'partitioned_rels' list,
- *		flattening as appropriate.
- */
-static List *
-accumulate_partitioned_rels(List *partitioned_rels,
-							List *sub_partitioned_rels,
-							bool flatten)
-{
-	if (flatten)
-	{
-		/*
-		 * We're only called with flatten == true when the partitioned_rels
-		 * list has at most 1 element.  So we can just add the members from
-		 * sub list's first element onto the first element of
-		 * partitioned_rels.  Only later in planning when doing UNION ALL
-		 * Append processing will we see flatten == false. partitioned_rels
-		 * may end up with more than 1 element then, but we never expect to be
-		 * called with flatten == true again after that, so we needn't bother
-		 * doing anything here for anything but the initial element.
-		 */
-		if (partitioned_rels != NIL && sub_partitioned_rels != NIL)
-		{
-			Relids		partrels = (Relids) linitial(partitioned_rels);
-			Relids		subpartrels = (Relids) linitial(sub_partitioned_rels);
-
-			/* Ensure the above comment holds true */
-			Assert(list_length(partitioned_rels) == 1);
-			Assert(list_length(sub_partitioned_rels) == 1);
-
-			linitial(partitioned_rels) = bms_add_members(partrels, subpartrels);
-		}
-	}
-	else
-	{
-		/*
-		 * Handle UNION ALL to partitioned tables.  This always occurs after
-		 * we've done the accumulation for sub-partitioned tables, so there's
-		 * no need to consider how adding multiple elements to the top level
-		 * list affects the flatten == true case above.
-		 */
-		partitioned_rels = list_concat(partitioned_rels, sub_partitioned_rels);
-	}
-
-	return partitioned_rels;
-}
-
 /*
 * accumulate_append_subpath
 *		Add a subpath to the list being built for an Append or MergeAppend.
@@ -2076,24 +1961,9 @@ accumulate_partitioned_rels(List *partitioned_rels,
 * children to subpaths and the rest to special_subpaths.  If the latter is
 * NULL, we don't flatten the path at all (unless it contains only partial
 * paths).
- *
- * When pulling up sub-Appends and sub-Merge Appends, we also gather the
- * path's list of partitioned tables and store in 'partitioned_rels'.  The
- * exact behavior here depends on the value of 'flatten_partitioned_rels'.
- *
- * When 'flatten_partitioned_rels' is true, 'partitioned_rels' will contain at
- * most one element which is a Relids of the partitioned relations which there
- * are subpaths for.  In this case, we just add the RT indexes for the
- * partitioned tables for the subpath we're pulling up to the single entry in
- * 'partitioned_rels'.  When 'flatten_partitioned_rels' is false we
- * concatenate the path's partitioned rel list onto the top-level list.  This
- * done for UNION ALLs which could have a partitioned table in each union
- * branch.
 */
 static void
-accumulate_append_subpath(Path *path, List **subpaths, List **special_subpaths,
-						  List **partitioned_rels,
-						  bool flatten_partitioned_rels)
+accumulate_append_subpath(Path *path, List **subpaths, List **special_subpaths)
 {
 	if (IsA(path, AppendPath))
 	{
@@ -2102,9 +1972,6 @@ accumulate_append_subpath(Path *path, List **subpaths, List **special_subpaths,
 		if (!apath->path.parallel_aware || apath->first_partial_path == 0)
 		{
 			*subpaths = list_concat(*subpaths, apath->subpaths);
-			*partitioned_rels = accumulate_partitioned_rels(*partitioned_rels,
-															apath->partitioned_rels,
-															flatten_partitioned_rels);
 			return;
 		}
 		else if (special_subpaths != NULL)
@@ -2120,9 +1987,6 @@ accumulate_append_subpath(Path *path, List **subpaths, List **special_subpaths,
 							  apath->first_partial_path);
 			*special_subpaths = list_concat(*special_subpaths,
 											new_special_subpaths);
-			*partitioned_rels = accumulate_partitioned_rels(*partitioned_rels,
-															apath->partitioned_rels,
-															flatten_partitioned_rels);
 			return;
 		}
 	}
@@ -2131,9 +1995,6 @@ accumulate_append_subpath(Path *path, List **subpaths, List **special_subpaths,
 		MergeAppendPath *mpath = (MergeAppendPath *) path;

 		*subpaths = list_concat(*subpaths, mpath->subpaths);
-		*partitioned_rels = accumulate_partitioned_rels(*partitioned_rels,
-														mpath->partitioned_rels,
-														flatten_partitioned_rels);
 		return;
 	}

@@ -2195,7 +2056,7 @@ set_dummy_rel_pathlist(RelOptInfo *rel)
 	/* Set up the dummy path */
 	add_path(rel, (Path *) create_append_path(NULL, rel, NIL, NIL,
 											  NIL, rel->lateral_relids,
-											  0, false, NIL, -1));
+											  0, false, -1));

 	/*
 	 * We set the cheapest-path fields immediately, just in case they were
--- a/src/backend/optimizer/path/joinrels.c
+++ b/src/backend/optimizer/path/joinrels.c
@@ -1279,7 +1279,7 @@ mark_dummy_rel(RelOptInfo *rel)
 	/* Set up the dummy path */
 	add_path(rel, (Path *) create_append_path(NULL, rel, NIL, NIL,
 											  NIL, rel->lateral_relids,
-											  0, false, NIL, -1));
+											  0, false, -1));

 	/* Set or update cheapest_total_path and related fields */
 	set_cheapest(rel);
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -1248,7 +1248,6 @@ create_append_plan(PlannerInfo *root, AppendPath *best_path, int flags)
 			partpruneinfo =
 				make_partition_pruneinfo(root, rel,
 										 best_path->subpaths,
-										 best_path->partitioned_rels,
 										 prunequal);
 	}

@@ -1412,7 +1411,6 @@ create_merge_append_plan(PlannerInfo *root, MergeAppendPath *best_path,
 		if (prunequal != NIL)
 			partpruneinfo = make_partition_pruneinfo(root, rel,
 													 best_path->subpaths,
-													 best_path->partitioned_rels,
 													 prunequal);
 	}

--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -1726,7 +1726,7 @@ inheritance_planner(PlannerInfo *root)
 		/* Make a dummy path, cf set_dummy_rel_pathlist() */
 		dummy_path = (Path *) create_append_path(NULL, final_rel, NIL, NIL,
 												 NIL, NULL, 0, false,
-												 NIL, -1);
+												 -1);

 		/* These lists must be nonempty to make a valid ModifyTable node */
 		subpaths = list_make1(dummy_path);
@@ -4006,7 +4006,6 @@ create_degenerate_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel,
 							   NULL,
 							   0,
 							   false,
-							   NIL,
 							   -1);
 	}
 	else
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -620,7 +620,7 @@ generate_union_paths(SetOperationStmt *op, PlannerInfo *root,
 	 * Append the child results together.
 	 */
 	path = (Path *) create_append_path(root, result_rel, pathlist, NIL,
-									   NIL, NULL, 0, false, NIL, -1);
+									   NIL, NULL, 0, false, -1);

 	/*
 	 * For UNION ALL, we just need the Append path.  For UNION, need to add
@@ -677,7 +677,7 @@ generate_union_paths(SetOperationStmt *op, PlannerInfo *root,
 			create_append_path(root, result_rel, NIL, partial_pathlist,
 							   NIL, NULL,
 							   parallel_workers, enable_parallel_append,
-							   NIL, -1);
+							   -1);
 		ppath = (Path *)
 			create_gather_path(root, result_rel, ppath,
 							   result_rel->reltarget, NULL, NULL);
@@ -787,7 +787,7 @@ generate_nonunion_paths(SetOperationStmt *op, PlannerInfo *root,
 	 * Append the child results together.
 	 */
 	path = (Path *) create_append_path(root, result_rel, pathlist, NIL,
-									   NIL, NULL, 0, false, NIL, -1);
+									   NIL, NULL, 0, false, -1);

 	/* Identify the grouping semantics */
 	groupList = generate_setop_grouplist(op, tlist);
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -1217,7 +1217,7 @@ create_append_path(PlannerInfo *root,
 				   List *subpaths, List *partial_subpaths,
 				   List *pathkeys, Relids required_outer,
 				   int parallel_workers, bool parallel_aware,
-				   List *partitioned_rels, double rows)
+				   double rows)
 {
 	AppendPath *pathnode = makeNode(AppendPath);
 	ListCell   *l;
@@ -1249,7 +1249,6 @@ create_append_path(PlannerInfo *root,
 	pathnode->path.parallel_safe = rel->consider_parallel;
 	pathnode->path.parallel_workers = parallel_workers;
 	pathnode->path.pathkeys = pathkeys;
-	pathnode->partitioned_rels = list_copy(partitioned_rels);

 	/*
 	 * For parallel append, non-partial paths are sorted by descending total
@@ -1377,8 +1376,7 @@ create_merge_append_path(PlannerInfo *root,
 						 RelOptInfo *rel,
 						 List *subpaths,
 						 List *pathkeys,
-						 Relids required_outer,
-						 List *partitioned_rels)
+						 Relids required_outer)
 {
 	MergeAppendPath *pathnode = makeNode(MergeAppendPath);
 	Cost		input_startup_cost;
@@ -1394,7 +1392,6 @@ create_merge_append_path(PlannerInfo *root,
 	pathnode->path.parallel_safe = rel->consider_parallel;
 	pathnode->path.parallel_workers = 0;
 	pathnode->path.pathkeys = pathkeys;
-	pathnode->partitioned_rels = list_copy(partitioned_rels);
 	pathnode->subpaths = subpaths;

 	/*
@@ -3847,7 +3844,6 @@ reparameterize_path(PlannerInfo *root, Path *path,
 									   apath->path.pathkeys, required_outer,
 									   apath->path.parallel_workers,
 									   apath->path.parallel_aware,
-									   apath->partitioned_rels,
 									   -1);
 			}
 		default: