Re: plan shape work

On Mon, Sep 29, 2025 at 11:41 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Richard Guo <guofenglinux@gmail.com> writes:
> > On Fri, Sep 26, 2025 at 11:23 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> 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.

> > I'd argue that this hypothetical edge case can be resolved with a bit
> > of canonicalization in how subplan names are represented internally.

> [ raised eyebrow... ]  How did you get to that from the complaint
> that Robert's patch was not obviously bug-free?  (A complaint I
> thought was unmerited, but nevermind.)

I'm not sure I fully understand your point here.  Apologies if I got
it wrong.

Firstly, my intention in the previous email was merely to propose a
solution for my approach to address the edge case you raised.  I don't
see how this relates to my so-called "complaint" about Robert's patch
not being obviously bug-free.  You raised a case where my approach
won't work, and I provided a solution to address it.  That's all.

Secondly, I don't think it's fair to characterize my concern as a
complaint when I expressed that the nested loop with an always-true
condition is vulnerable to bugs and could potentially cause an
infinite loop if such a bug exists.

In a nearby thread, I was asked whether I can guarantee my code is
100% bug-free.  After some consideration, I think I cannot make such a
guarantee, and I doubt that anyone realistically can.  Given this, I
think it's important that we try our best to write code that minimizes
the potential bad-effect should a bug occur.

Therefore, upon observing a nested loop with an always-true condition
in the patch, I expressed my concern and suggested a possible
improvement.  However, I did not expect that concern to be treated as
an unmerited complaint.

> This proposal is neither
> simple, nor obviously bug-free.  Moreover, in view of comments
> upthread, I think we should look with great suspicion on any
> proposal that involves changing user-supplied subquery aliases
> unnecessarily.

It seems no one has attempted to code up the approach I suggested, so
I went ahead and did it; please see the attached PoC patch.  It's just
a proof of concept to show what I have in mind, so please excuse the
lack of comments and necessary assertions for now.

I agree that this implementation cannot be guaranteed to be bug-free,
but I'm not sure I agree that it's not simple.  I'm also not convinced
that it would be slower in typical cases.

BTW, a small nit I just noticed: I suggest explicitly initializing
glob->subplanNames in standard_planner().  It may be argued that this
is pointless, as makeNode() zeroes all fields by default.  But AFAICS
subplanNames is the only field in PlannerGlobal that is not explicitly
initialized.

- Richard