From 31f4e33142227bd31355aa2042ae0823937a36bd Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Mon, 8 Dec 2025 13:46:43 -0500 Subject: [PATCH v1 1/2] Add a list of all PlannerInfo "roots" to PlannerGlobal. This can serve the same purpose as glob->subplanNames, namely, to make sure that we don't assign the same name to more than one subplan, but it also can be used to locate a given subplan name, which is something that glob->subplanNames cannot do. This commit does not remove glob->subplanNames, but it ensures that glob->allroots and glob->subplanNames have equivalent contents at the key point in the code, namely, the start of choose_plan_name. --- src/backend/optimizer/plan/planagg.c | 1 + src/backend/optimizer/plan/planner.c | 50 +++++++++++++++++++++-- src/backend/optimizer/prep/prepjointree.c | 7 ++++ src/include/nodes/pathnodes.h | 3 ++ src/include/optimizer/planner.h | 1 + 5 files changed, 58 insertions(+), 4 deletions(-) diff --git a/src/backend/optimizer/plan/planagg.c b/src/backend/optimizer/plan/planagg.c index a2ac58d246e..f138f372e7d 100644 --- a/src/backend/optimizer/plan/planagg.c +++ b/src/backend/optimizer/plan/planagg.c @@ -341,6 +341,7 @@ build_minmax_path(PlannerInfo *root, MinMaxAggInfo *mminfo, subroot->query_level++; subroot->parent_root = root; subroot->plan_name = choose_plan_name(root->glob, "minmax", true); + remember_plannerinfo(subroot); /* reset subplan-related stuff */ subroot->plan_params = NIL; diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index fd77334e5fd..47b32b20e1a 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -745,6 +745,9 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name, root->wt_param_id = -1; root->non_recursive_path = NULL; + /* Add this PlanenrInfo to the PlannerGlobal's list */ + remember_plannerinfo(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 @@ -8968,6 +8971,18 @@ choose_plan_name(PlannerGlobal *glob, const char *name, bool always_number) { unsigned n; +#ifdef USE_ASSERT_CHECKING + Assert(list_length(glob->allroots) == list_length(glob->subplanNames) + 1); + foreach_ptr(char, subplan_name, glob->subplanNames) + { + PlannerInfo *subroot; + + subroot = list_nth(glob->allroots, + foreach_current_index(subplan_name) + 1); + Assert(strcmp(subroot->plan_name, subplan_name) == 0); + } +#endif + /* * 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 @@ -8977,9 +8992,10 @@ choose_plan_name(PlannerGlobal *glob, const char *name, bool always_number) { bool found = false; - foreach_ptr(char, subplan_name, glob->subplanNames) + foreach_ptr(PlannerInfo, subroot, glob->allroots) { - if (strcmp(subplan_name, name) == 0) + if (subroot->plan_name != NULL && + strcmp(subroot->plan_name, name) == 0) { found = true; break; @@ -9006,9 +9022,10 @@ choose_plan_name(PlannerGlobal *glob, const char *name, bool always_number) char *proposed_name = psprintf("%s_%u", name, n); bool found = false; - foreach_ptr(char, subplan_name, glob->subplanNames) + foreach_ptr(PlannerInfo, subroot, glob->allroots) { - if (strcmp(subplan_name, proposed_name) == 0) + if (subroot->plan_name != NULL && + strcmp(subroot->plan_name, proposed_name) == 0) { found = true; break; @@ -9024,3 +9041,28 @@ choose_plan_name(PlannerGlobal *glob, const char *name, bool always_number) pfree(proposed_name); } } + +/* + * Add a new PlannerInfo to the PlannerGlobal's list of all subroots. + */ +void +remember_plannerinfo(PlannerInfo *newroot) +{ + PlannerGlobal *glob = newroot->glob; + +#ifdef USE_ASSERT_CHECKING + /* Only the first PlannerInfo should be nameless. */ + Assert(newroot->plan_name != NULL || glob->allroots == NIL); + + /* PlannerInfo names should not be duplicated. */ + foreach_node(PlannerInfo, root, glob->allroots) + { + if (root->plan_name == NULL) + continue; + Assert(strcmp(root->plan_name, newroot->plan_name) != 0); + } +#endif + + /* Add new PlannerInfo to list. */ + glob->allroots = lappend(glob->allroots, newroot); +} diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index 7581695647d..86e1abe8524 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -1350,6 +1350,13 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte, /* * Create a PlannerInfo data structure for this subquery. * + * Unlike subquery_planner, the subroot we create here is only transient. + * It inherits properties such as the query_level, plan_name, and + * parent_root from the supplied root, rather than becoming a new query + * level. Whether we succeed or fail in pulling up the subquery, this + * subroot won't survive long-term and shouldn't be linked into any + * long-lived planner data structures. + * * NOTE: the next few steps should match the first processing in * subquery_planner(). Can we refactor to avoid code duplication, or * would that just make things uglier? diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 46a8655621d..2707ca50019 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -110,6 +110,9 @@ typedef struct PlannerGlobal /* PlannerInfos for SubPlan nodes */ List *subroots pg_node_attr(read_write_ignore); + /* every non-transient PlannerInfo (superset of subroots) */ + 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); diff --git a/src/include/optimizer/planner.h b/src/include/optimizer/planner.h index 55d9b7940aa..76a7b7e9a2a 100644 --- a/src/include/optimizer/planner.h +++ b/src/include/optimizer/planner.h @@ -82,5 +82,6 @@ extern RelOptInfo *create_unique_paths(PlannerInfo *root, RelOptInfo *rel, extern char *choose_plan_name(PlannerGlobal *glob, const char *name, bool always_number); +extern void remember_plannerinfo(PlannerInfo *newroot); #endif /* PLANNER_H */ -- 2.51.0