v10-0002-Use-a-list-of-subplan-names-instead-of-a-list-of.patch

From b6736b127a93b0203ad8a5ab9dd1514135606517 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Wed, 24 Sep 2025 17:30:22 -0400
Subject: [PATCH v10 2/2] Use a list of subplan names instead of a list of
 "allroots".

I don't like constructing a list of all PlannerInfo roots: it's
partially redundant with the subroots list, and it's far from clear
how to handle transient roots such as the one manufactured by
pull_up_simple_subquery.  This patch proposes that we should
instead just store a list of assigned subplan names.  One advantage
is that choose_plan_name can immediately enter the new name into
the list, instead of relying on the caller to get it right later.

Apropos to that point, I found that build_minmax_path was putting
the wrong root into the list, so that it was perfectly capable
of assigning "minmax_1" over and over, as indeed is visible in
a couple of incorrect regression test changes in v9-0001.

I remain unsure whether it's OK for pull_up_simple_subquery to just do
+	subroot->plan_name = root->plan_name;
but at least now we won't have two roots with the same plan_name
in the allroots list.  There's no way that that's helpful.

Also fix the missed use of choose_plan_name in SS_process_ctes:
without that, we can't really promise that we'll attach unique
names to CTE subplans.
---
 src/backend/optimizer/plan/planagg.c      |  3 --
 src/backend/optimizer/plan/planner.c      | 35 ++++++++++++++---------
 src/backend/optimizer/plan/subselect.c    |  5 ++--
 src/backend/optimizer/prep/prepjointree.c |  3 --
 src/include/nodes/pathnodes.h             |  6 ++--
 src/test/regress/expected/aggregates.out  |  6 ++--
 src/test/regress/expected/inherit.out     |  2 +-
 7 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/src/backend/optimizer/plan/planagg.c b/src/backend/optimizer/plan/planagg.c
index 0ce35cabaf5..a2ac58d246e 100644
--- a/src/backend/optimizer/plan/planagg.c
+++ b/src/backend/optimizer/plan/planagg.c
@@ -362,9 +362,6 @@ build_minmax_path(PlannerInfo *root, MinMaxAggInfo *mminfo,
 	/* and we haven't created PlaceHolderInfos, either */
 	Assert(subroot->placeholder_list == NIL);
 
-	/* Add this to list of all PlannerInfo objects. */
-	root->glob->allroots = lappend(root->glob->allroots, root);
-
 	/*----------
 	 * Generate modified query of the form
 	 *		(SELECT col FROM tab
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index acd1356a721..3b130e724f7 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -631,6 +631,7 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
  *
  * glob is the global state for the current planner run.
  * parse is the querytree produced by the parser & rewriter.
+ * plan_name is the name to assign to this subplan (NULL at the top level).
  * parent_root is the immediate parent Query's info (NULL at the top level).
  * hasRecursion is true if this is a recursive WITH query.
  * tuple_fraction is the fraction of tuples we expect will be retrieved.
@@ -712,9 +713,6 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name,
 	root->non_recursive_path = NULL;
 	root->partColsUpdated = false;
 
-	/* Add this to list of all PlannerInfo objects. */
-	root->glob->allroots = lappend(root->glob->allroots, root);
-
 	/*
 	 * Create the top-level join domain.  This won't have valid contents until
 	 * deconstruct_jointree fills it in, but the node needs to exist before
@@ -8840,7 +8838,9 @@ create_partial_unique_paths(PlannerInfo *root, RelOptInfo *input_rel,
 }
 
 /*
- * Choose a unique plan name for subroot.
+ * Choose a unique name for some subroot.
+ *
+ * Modifies glob->subplanNames to track names already used.
  */
 char *
 choose_plan_name(PlannerGlobal *glob, const char *name, bool always_number)
@@ -8848,18 +8848,17 @@ choose_plan_name(PlannerGlobal *glob, const char *name, bool always_number)
 	unsigned	n;
 
 	/*
-	 * If a numeric suffix is not required, then search the list of roots for
-	 * a plan with the requested name. If none is found, then we can use the
-	 * provided name without modification.
+	 * If a numeric suffix is not required, then search the list of
+	 * previously-assigned names for a match. If none is found, then we can
+	 * use the provided name without modification.
 	 */
 	if (!always_number)
 	{
 		bool		found = false;
 
-		foreach_node(PlannerInfo, root, glob->allroots)
+		foreach_ptr(char, subplan_name, glob->subplanNames)
 		{
-			if (root->plan_name != NULL &&
-				strcmp(name, root->plan_name) == 0)
+			if (strcmp(subplan_name, name) == 0)
 			{
 				found = true;
 				break;
@@ -8867,7 +8866,13 @@ choose_plan_name(PlannerGlobal *glob, const char *name, bool always_number)
 		}
 
 		if (!found)
-			return pstrdup(name);
+		{
+			/* pstrdup here is just to avoid cast-away-const */
+			char	   *chosen_name = pstrdup(name);
+
+			glob->subplanNames = lappend(glob->subplanNames, chosen_name);
+			return chosen_name;
+		}
 	}
 
 	/*
@@ -8880,10 +8885,9 @@ choose_plan_name(PlannerGlobal *glob, const char *name, bool always_number)
 		char	   *proposed_name = psprintf("%s_%u", name, n);
 		bool		found = false;
 
-		foreach_node(PlannerInfo, root, glob->allroots)
+		foreach_ptr(char, subplan_name, glob->subplanNames)
 		{
-			if (root->plan_name != NULL &&
-				strcmp(proposed_name, root->plan_name) == 0)
+			if (strcmp(subplan_name, proposed_name) == 0)
 			{
 				found = true;
 				break;
@@ -8891,7 +8895,10 @@ choose_plan_name(PlannerGlobal *glob, const char *name, bool always_number)
 		}
 
 		if (!found)
+		{
+			glob->subplanNames = lappend(glob->subplanNames, proposed_name);
 			return proposed_name;
+		}
 
 		pfree(proposed_name);
 	}
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index 5f8306bc421..14192a13236 100644
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -967,8 +967,9 @@ SS_process_ctes(PlannerInfo *root)
 		 * Generate Paths for the CTE query.  Always plan for full retrieval
 		 * --- we don't have enough info to predict otherwise.
 		 */
-		subroot = subquery_planner(root->glob, subquery, cte->ctename, root,
-								   cte->cterecursive, 0.0, NULL);
+		subroot = subquery_planner(root->glob, subquery,
+								   choose_plan_name(root->glob, cte->ctename, false),
+								   root, cte->cterecursive, 0.0, NULL);
 
 		/*
 		 * Since the current query level doesn't yet contain any RTEs, it
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 2ec13637d16..563be151a4d 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -1388,9 +1388,6 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
 	subroot->non_recursive_path = NULL;
 	/* We don't currently need a top JoinDomain for the subroot */
 
-	/* Add new subroot to master list of PlannerInfo objects. */
-	root->glob->allroots = lappend(root->glob->allroots, subroot);
-
 	/* No CTEs to worry about */
 	Assert(subquery->cteList == NIL);
 
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index a341b01a1e1..7ee9a7a68d8 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -110,8 +110,8 @@ typedef struct PlannerGlobal
 	/* PlannerInfos for SubPlan nodes */
 	List	   *subroots pg_node_attr(read_write_ignore);
 
-	/* every PlannerInfo regardless of whether it's an InitPlan/SubPlan */
-	List	   *allroots pg_node_attr(read_write_ignore);
+	/* names already used for subplans (list of C strings) */
+	List	   *subplanNames pg_node_attr(read_write_ignore);
 
 	/* indices of subplans that require REWIND */
 	Bitmapset  *rewindPlanIDs;
@@ -231,7 +231,7 @@ struct PlannerInfo
 	/* NULL at outermost Query */
 	PlannerInfo *parent_root pg_node_attr(read_write_ignore);
 
-	/* Name for EXPLAIN and debugging purposes */
+	/* Subplan name for EXPLAIN and debugging purposes (NULL at top level) */
 	char	   *plan_name;
 
 	/*
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index a9503e810c5..1f84db2f361 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -1269,7 +1269,7 @@ explain (costs off)
                  ->  Index Only Scan Backward using minmaxtest2i on minmaxtest2 minmaxtest_3
                        Index Cond: (f1 IS NOT NULL)
                  ->  Index Only Scan using minmaxtest3i on minmaxtest3 minmaxtest_4
-   InitPlan minmax_1
+   InitPlan minmax_2
      ->  Limit
            ->  Merge Append
                  Sort Key: minmaxtest_5.f1 DESC
@@ -1305,7 +1305,7 @@ explain (costs off)
                  ->  Index Only Scan Backward using minmaxtest2i on minmaxtest2 minmaxtest_3
                        Index Cond: (f1 IS NOT NULL)
                  ->  Index Only Scan using minmaxtest3i on minmaxtest3 minmaxtest_4
-   InitPlan minmax_1
+   InitPlan minmax_2
      ->  Limit
            ->  Merge Append
                  Sort Key: minmaxtest_5.f1 DESC
@@ -1317,7 +1317,7 @@ explain (costs off)
                        Index Cond: (f1 IS NOT NULL)
                  ->  Index Only Scan Backward using minmaxtest3i on minmaxtest3 minmaxtest_9
    ->  Sort
-         Sort Key: ((InitPlan minmax_1).col1), ((InitPlan minmax_1).col1)
+         Sort Key: ((InitPlan minmax_1).col1), ((InitPlan minmax_2).col1)
          ->  Result
                Replaces: MinMaxAggregate
 (27 rows)
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 6dbbd26f56b..0490a746555 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -3264,7 +3264,7 @@ explain (costs off) select min(a), max(a) from parted_minmax where b = '12345';
      ->  Limit
            ->  Index Only Scan using parted_minmax1i on parted_minmax1 parted_minmax
                  Index Cond: ((a IS NOT NULL) AND (b = '12345'::text))
-   InitPlan minmax_1
+   InitPlan minmax_2
      ->  Limit
            ->  Index Only Scan Backward using parted_minmax1i on parted_minmax1 parted_minmax_1
                  Index Cond: ((a IS NOT NULL) AND (b = '12345'::text))
-- 
2.43.7