Re: plan shape work

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