Re: plan shape work

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Richard Guo <guofenglinux(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Alexandra Wang <alexandra(dot)wang(dot)oss(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "bruce(at)momjian(dot)us" <bruce(at)momjian(dot)us>, lepihov(at)gmail(dot)com
Subject: Re: plan shape work
Date: 2025-09-26 14:23:21
Message-ID: 4055880.1758896601@sss.pgh.pa.us
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Richard Guo <guofenglinux(at)gmail(dot)com> writes:
> 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.

"simply" is perhaps not the right adjective there. My guess is that
this approach nets out to more code, more possibilities for bugs
(especially in cases where one name is a prefix of another), and
will be slower in typical cases with just a few subplan names.

As an example of edge cases that your idea introduces, what happens
if a user-written subquery name is "expr_999999999999999999999999"
and then we need to generate a unique name based on "expr"? Now
we have an integer-overflow situation to worry about, with possibly
platform-dependent results.

But it's Robert's patch, so he gets to make the call.

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2025-09-26 14:23:29 Re: plan shape work
Previous Message Nathan Bossart 2025-09-26 14:15:25 Re: splitting src/bin/scripts/vacuumdb.c