From b6c81dbdb80bb3522a7c30d4e83c56c3606dde8d Mon Sep 17 00:00:00 2001 From: Manuel Reyes Bravo Date: Wed, 4 Nov 2026 18:21:54 -0300 Subject: [PATCH v6] Avoid O(N^3) planning time in choose_plan_name() Since commit 8c49a484e8e, choose_plan_name() found the smallest free numeric suffix by trying 1, 2, 3, ... and scanning every name assigned so far for each candidate, so planning a statement with N subplans of one kind took O(N^3) time: about 17 s for a 4000-arm UNION, against 19 ms on v18. Different names given by the user, such as CTE names, still took O(N^2) time. Once a statement has 32 subplan names, also keep them in a hash table keyed by the name, and look names up there instead of in the list. Each entry records whether the name itself is in use and, for a base name, the suffix to start the search for a free "name_N" from: names are never released, so every smaller N is known to be taken. Below 32 names, the code runs as before. The names chosen do not change. Suggested-by: Robert Haas --- src/backend/optimizer/plan/planner.c | 149 +++++++++++++++++++----- src/include/nodes/pathnodes.h | 3 + src/test/regress/expected/subselect.out | 101 ++++++++++++++++ src/test/regress/sql/subselect.sql | 55 +++++++++ src/tools/pgindent/typedefs.list | 2 + 5 files changed, 281 insertions(+), 29 deletions(-) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index edaa42d..9148ee3 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -26,6 +26,7 @@ #include "catalog/pg_inherits.h" #include "catalog/pg_proc.h" #include "catalog/pg_type.h" +#include "common/hashfn.h" #include "executor/executor.h" #include "foreign/fdwapi.h" #include "jit/jit.h" @@ -9249,42 +9250,128 @@ create_partial_unique_paths(PlannerInfo *root, RelOptInfo *input_rel, } } +/* Hash table entry in glob->subplanNameHash. */ +typedef struct +{ + uint32 status; + const char *name; + bool used; /* is the name itself in use? */ + unsigned next_suffix; /* name_N is taken for N < next_suffix */ +} SubplanNameEntry; + +/* Threshold for switching from list to hash table */ +#define SUBPLAN_NAME_HASH_THRESHOLD 32 + +#define SH_PREFIX subplan_name +#define SH_ELEMENT_TYPE SubplanNameEntry +#define SH_KEY_TYPE const char * +#define SH_KEY name +#define SH_HASH_KEY(tb, key) \ + hash_bytes((const unsigned char *) (key), strlen(key)) +#define SH_EQUAL(tb, a, b) (strcmp(a, b) == 0) +#define SH_SCOPE static inline +#define SH_DECLARE +#define SH_DEFINE +#include "lib/simplehash.h" + +/* + * Enter "name", which is now in use, into the hash table. + */ +static void +enter_subplan_name(subplan_name_hash *tb, char *name) +{ + SubplanNameEntry *entry; + bool found; + + entry = subplan_name_insert(tb, name, &found); + if (!found) + entry->next_suffix = 1; + entry->used = true; +} + +/* + * Is "name" in glob->subplanNames? + */ +static bool +subplan_name_in_use(PlannerGlobal *glob, const char *name) +{ + if (glob->subplanNameHash) + { + SubplanNameEntry *entry; + + entry = subplan_name_lookup(glob->subplanNameHash, name); + return entry != NULL && entry->used; + } + + foreach_ptr(char, subplan_name, glob->subplanNames) + { + if (strcmp(subplan_name, name) == 0) + return true; + } + return false; +} + +/* + * Add "name" to glob->subplanNames, and to glob->subplanNameHash if built. + */ +static void +add_subplan_name(PlannerGlobal *glob, char *name) +{ + glob->subplanNames = lappend(glob->subplanNames, name); + if (glob->subplanNameHash) + enter_subplan_name(glob->subplanNameHash, name); +} + /* * Choose a unique name for some subroot. * - * Modifies glob->subplanNames to track names already used. + * Modifies glob->subplanNames to track names already used. Once there are + * many of them, they are also kept in glob->subplanNameHash, so that we + * don't have to search the list for each name we try. + * + * The hash table also remembers, for each base name, the smallest N + * that might give a free "name_N": since names are never released, + * every smaller N is known to be taken, and we start the search there. */ char * choose_plan_name(PlannerGlobal *glob, const char *name, bool always_number) { unsigned n; + /* Switch to using hash lookup when the list grows "too long". */ + if (!glob->subplanNameHash && + list_length(glob->subplanNames) >= SUBPLAN_NAME_HASH_THRESHOLD) + { + glob->subplanNameHash = + subplan_name_create(CurrentMemoryContext, + list_length(glob->subplanNames), NULL); + foreach_ptr(char, subplan_name, glob->subplanNames) + enter_subplan_name(glob->subplanNameHash, subplan_name); + } + /* * 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) + if (!always_number && !subplan_name_in_use(glob, name)) { - bool found = false; + /* pstrdup here is just to avoid cast-away-const */ + char *chosen_name = pstrdup(name); - foreach_ptr(char, subplan_name, glob->subplanNames) - { - if (strcmp(subplan_name, name) == 0) - { - found = true; - break; - } - } + add_subplan_name(glob, chosen_name); + return chosen_name; + } - if (!found) - { - /* pstrdup here is just to avoid cast-away-const */ - char *chosen_name = pstrdup(name); + /* With the hash table, start where the last search for this name ended. */ + n = 1; + if (glob->subplanNameHash) + { + SubplanNameEntry *entry; - glob->subplanNames = lappend(glob->subplanNames, chosen_name); - return chosen_name; - } + entry = subplan_name_lookup(glob->subplanNameHash, name); + if (entry) + n = entry->next_suffix; } /* @@ -9292,23 +9379,27 @@ choose_plan_name(PlannerGlobal *glob, const char *name, bool always_number) * in use, then loop until we find a positive integer that produces a * novel name. */ - for (n = 1; true; ++n) + for (; true; ++n) { char *proposed_name = psprintf("%s_%u", name, n); - bool found = false; - foreach_ptr(char, subplan_name, glob->subplanNames) + if (!subplan_name_in_use(glob, proposed_name)) { - if (strcmp(subplan_name, proposed_name) == 0) + add_subplan_name(glob, proposed_name); + if (glob->subplanNameHash) { - found = true; - break; + SubplanNameEntry *entry; + bool found; + + entry = subplan_name_insert(glob->subplanNameHash, + name, &found); + if (!found) + { + entry->name = pstrdup(name); + entry->used = false; + } + entry->next_suffix = n + 1; } - } - - if (!found) - { - glob->subplanNames = lappend(glob->subplanNames, proposed_name); return proposed_name; } diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index ff46a9f..9922d9b 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -186,6 +186,9 @@ typedef struct PlannerGlobal /* names already used for subplans (list of C strings) */ List *subplanNames pg_node_attr(read_write_ignore); + /* optional hash table for fast lookup; contains same names as list */ + struct subplan_name_hash *subplanNameHash pg_node_attr(read_write_ignore); + /* indices of subplans that require REWIND */ Bitmapset *rewindPlanIDs; diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out index c78267d..a37b76e 100644 --- a/src/test/regress/expected/subselect.out +++ b/src/test/regress/expected/subselect.out @@ -4126,3 +4126,104 @@ WHERE t NOT IN (SELECT id FROM not_null_tab); (4 rows) ROLLBACK; +-- +-- Numbered subplan names must not collide with names used as given, such as +-- CTE names, whichever comes first +-- +EXPLAIN (COSTS OFF) +WITH expr_1 AS MATERIALIZED (SELECT 1 AS x) +SELECT (SELECT x FROM expr_1), (SELECT 2 WHERE random() >= 0); + QUERY PLAN +---------------------------------------------------------------- + Result + CTE expr_1 + -> Result + InitPlan expr_2 + -> CTE Scan on expr_1 + InitPlan expr_3 + -> Result + One-Time Filter: (random() >= '0'::double precision) +(8 rows) + +-- expr_1 is generated first, so the CTE expr_1 that comes after is numbered +EXPLAIN (COSTS OFF) +SELECT (SELECT 1 WHERE random() >= 0), + (WITH expr_1 AS MATERIALIZED (SELECT 2 AS x) SELECT x FROM expr_1); + QUERY PLAN +---------------------------------------------------------------- + Result + InitPlan expr_1 + -> Result + One-Time Filter: (random() >= '0'::double precision) + InitPlan expr_2 + -> CTE Scan on expr_1 + CTE expr_1_1 + -> Result +(8 rows) + +-- expr_5 is used as given after expr_1 .. expr_4, so the next one is expr_6 +EXPLAIN (COSTS OFF) +SELECT (SELECT 1 WHERE random() >= 0), (SELECT 2 WHERE random() >= 0), + (SELECT 3 WHERE random() >= 0), + (WITH expr_5 AS MATERIALIZED (SELECT 4 AS x) SELECT x FROM expr_5), + (SELECT 5 WHERE random() >= 0); + QUERY PLAN +---------------------------------------------------------------- + Result + InitPlan expr_1 + -> Result + One-Time Filter: (random() >= '0'::double precision) + InitPlan expr_2 + -> Result + One-Time Filter: (random() >= '0'::double precision) + InitPlan expr_3 + -> Result + One-Time Filter: (random() >= '0'::double precision) + InitPlan expr_4 + -> CTE Scan on expr_5 + CTE expr_5 + -> Result + InitPlan expr_6 + -> Result + One-Time Filter: (random() >= '0'::double precision) +(17 rows) + +-- Past 32 names, choose_plan_name() looks them up in a hash table. The CTEs +-- c1 .. c40 keep their names, generated names still do not collide with +-- expr_2 and expr_1 given by the user, and a CTE called expr after expr_N have +-- been generated keeps its name (only the names other than cN are shown) +CREATE FUNCTION subplan_names(query text) RETURNS text +LANGUAGE plpgsql AS +$$ +DECLARE + ln text; + names text[] := '{}'; +BEGIN + FOR ln IN EXECUTE 'EXPLAIN (COSTS OFF) ' || query LOOP + IF ln ~ '^\s*(CTE|InitPlan|SubPlan) ' THEN + ln := regexp_replace(ln, '^\s*(CTE|InitPlan|SubPlan) ', ''); + IF ln !~ '^c[0-9]+$' THEN + names := names || ln; + END IF; + END IF; + END LOOP; + RETURN array_to_string(names, ' '); +END; +$$; +SELECT subplan_names(format( + 'WITH %s, expr_2 AS MATERIALIZED (SELECT 0 AS x) ' + 'SELECT (SELECT x FROM expr_2), (SELECT 1 WHERE random() >= 0), ' + '(WITH expr_1 AS MATERIALIZED (SELECT 2 AS x) SELECT x FROM expr_1), ' + '(WITH expr AS MATERIALIZED (SELECT 3 AS x) SELECT x FROM expr) ' + 'FROM (%s) s', + (SELECT string_agg(format('c%s AS MATERIALIZED (SELECT %s AS x)', i, i), + ', ') + FROM generate_series(1, 40) i), + (SELECT string_agg(format('SELECT x FROM c%s', i), ' UNION ALL ') + FROM generate_series(1, 40) i))); + subplan_names +-------------------------------------------------- + expr_2 expr_1 expr_3 expr_4 expr_1_1 expr_5 expr +(1 row) + +DROP FUNCTION subplan_names(text); diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql index c046b0e..538b1aa 100644 --- a/src/test/regress/sql/subselect.sql +++ b/src/test/regress/sql/subselect.sql @@ -1815,3 +1815,58 @@ SELECT * FROM not_null_text_tab WHERE t NOT IN (SELECT id FROM not_null_tab); ROLLBACK; + +-- +-- Numbered subplan names must not collide with names used as given, such as +-- CTE names, whichever comes first +-- +EXPLAIN (COSTS OFF) +WITH expr_1 AS MATERIALIZED (SELECT 1 AS x) +SELECT (SELECT x FROM expr_1), (SELECT 2 WHERE random() >= 0); + +-- expr_1 is generated first, so the CTE expr_1 that comes after is numbered +EXPLAIN (COSTS OFF) +SELECT (SELECT 1 WHERE random() >= 0), + (WITH expr_1 AS MATERIALIZED (SELECT 2 AS x) SELECT x FROM expr_1); + +-- expr_5 is used as given after expr_1 .. expr_4, so the next one is expr_6 +EXPLAIN (COSTS OFF) +SELECT (SELECT 1 WHERE random() >= 0), (SELECT 2 WHERE random() >= 0), + (SELECT 3 WHERE random() >= 0), + (WITH expr_5 AS MATERIALIZED (SELECT 4 AS x) SELECT x FROM expr_5), + (SELECT 5 WHERE random() >= 0); + +-- Past 32 names, choose_plan_name() looks them up in a hash table. The CTEs +-- c1 .. c40 keep their names, generated names still do not collide with +-- expr_2 and expr_1 given by the user, and a CTE called expr after expr_N have +-- been generated keeps its name (only the names other than cN are shown) +CREATE FUNCTION subplan_names(query text) RETURNS text +LANGUAGE plpgsql AS +$$ +DECLARE + ln text; + names text[] := '{}'; +BEGIN + FOR ln IN EXECUTE 'EXPLAIN (COSTS OFF) ' || query LOOP + IF ln ~ '^\s*(CTE|InitPlan|SubPlan) ' THEN + ln := regexp_replace(ln, '^\s*(CTE|InitPlan|SubPlan) ', ''); + IF ln !~ '^c[0-9]+$' THEN + names := names || ln; + END IF; + END IF; + END LOOP; + RETURN array_to_string(names, ' '); +END; +$$; +SELECT subplan_names(format( + 'WITH %s, expr_2 AS MATERIALIZED (SELECT 0 AS x) ' + 'SELECT (SELECT x FROM expr_2), (SELECT 1 WHERE random() >= 0), ' + '(WITH expr_1 AS MATERIALIZED (SELECT 2 AS x) SELECT x FROM expr_1), ' + '(WITH expr AS MATERIALIZED (SELECT 3 AS x) SELECT x FROM expr) ' + 'FROM (%s) s', + (SELECT string_agg(format('c%s AS MATERIALIZED (SELECT %s AS x)', i, i), + ', ') + FROM generate_series(1, 40) i), + (SELECT string_agg(format('SELECT x FROM c%s', i), ' UNION ALL ') + FROM generate_series(1, 40) i))); +DROP FUNCTION subplan_names(text); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 553162c..6055f03 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3021,6 +3021,7 @@ SubXactCallback SubXactCallbackItem SubXactEvent SubXactInfo +SubplanNameEntry SubqueryScan SubqueryScanPath SubqueryScanState @@ -4331,6 +4332,7 @@ storeInfo storeRes_func stream_stop_callback string +subplan_name_hash substitute_actual_parameters_context substitute_actual_parameters_in_from_context substitute_grouped_columns_context -- 2.56.0