Re: O(N^3) planning time in choose_plan_name() since 8c49a484e8e

From: Ayoub Kazar <ayoub(dot)kazar(at)data-bene(dot)io>
To: Manu <manuelreyesbravo(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, Robert Haas <rhaas(at)postgresql(dot)org>
Subject: Re: O(N^3) planning time in choose_plan_name() since 8c49a484e8e
Date: 2026-10-30 01:57:21
Message-ID: 9f92e15c-6fdd-468d-a00f-0c68a6d8b9ee@data-bene.io
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On 28/10/2026 21:27, Manu wrote:
> So v2 keeps a counter per base name. Names are never released, so
> every smaller suffix is taken and the counter gives today's name. A
> suffixed name determines its base and suffix, split at the last
> underscore, so suffixed names can't collide with each other; only names
> used as given (CTE names, subquery aliases) can, and those are checked.
> No existing expected output changes, and a test-only build running the
> old algorithm alongside found no mismatch. check-world passes with
> cassert; REL_19_STABLE takes the patch as is.

One small suggestion: the suffix parsing in subplan_name_in_use() scans
the digits
multiple times across strspn, strlen, and strtoul. It might be a bit cleaner
to fold the validation and conversion together into a single pass.

Something like
-    if (suffix != NULL && suffix[1] >= '1' && suffix[1] <= '9' &&
-        strspn(suffix + 1, "0123456789") == strlen(suffix + 1) &&
-        strlen(suffix + 1) <= 10)
+    if (suffix != NULL && suffix[1] >= '1' && suffix[1] <= '9')
     {
-        char       *base = pnstrdup(name, suffix - name);
-        SubplanNameSuffix *entry = subplan_name_suffix(glob, base, false);
+        char       *endptr;
+        unsigned long val = strtoul(suffix + 1, &endptr, 10);

-        pfree(base);
-        if (entry != NULL && strtoul(suffix + 1, NULL, 10) <
entry->next_suffix)
-            return true;
+        if (*endptr == '\0')
+        {
+            char       *base = pnstrdup(name, suffix - name);
+            SubplanNameSuffix *entry = subplan_name_suffix(glob, base,
false);
+
+            pfree(base);
+            if (entry != NULL && val < entry->next_suffix)
+                return true;
+        }
     }

Not a big deal though...

Regards,
Ayoub

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Richard Guo 2026-10-30 02:02:47 Re: ERROR: failed to assign all NestLoopParams to plan nodes
Previous Message jian he 2026-10-30 01:54:11 Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions