On Fri, Sep 26, 2025 at 11:37 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Richard Guo <guofenglinux@gmail.com> writes:
> > FWIW, I'm a bit concerned about the double for loop inside
> > choose_plan_name(), especially since the outer loop runs with a true
> > condition. Maybe I'm just worrying over nothing, as we probably don't
> > expect a large number of subroots in practice, but the nested loops
> > still make me a little uneasy.
> I really doubt that a query could have enough subplans to make
> that a problem. But if I'm wrong, it's surely something we could
> improve in a localized way later.
I'm concerned not only about the potential for a large number of
subplans but also because if there happens to be a bug within the
nested loops, the always-true condition in the outer loop could cause
an infinite loop. However, if you're confident that the code inside
the loop is completely bug-free and will remain so through future
changes, then this shouldn't be an issue.
Looking at choose_plan_name(), IIUC, the nested loop is used to find
the next unused suffix number for a given name. I'm wondering why not
simply iterate through glob->subplanNames once, check the suffix
number for each name matching the given base name, determine the
current maximum suffix, and then use "max_suffix + 1" as the next
unused suffix. This approach requires only a single pass through the
list, and if there's a bug, the worst-case scenario would be a
duplicate name rather than an infinite loop. It seems to me that this
approach is both more efficient and less risky.
- Richard
