Re: plan shape work

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Richard Guo <guofenglinux(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, 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-29 14:12:31
Message-ID: CA+TgmoY7875Kss5BVDUjJsOX0QkBtL-BYk+Es2oMp=wH9VNvyw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Sep 29, 2025 at 4:52 AM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> 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 think that if there are subqueries named expr_1 and expr_3, this
will assign the name expr_4 next, whereas the previous patch assigns
the name expr_2. That might not be a bug, but it's different and I
don't like it as well.

I also think that if there are subqueries named expr_1a and expr_2a,
this will assign the name expr_3 next, whereas the previous patch
assigns the name expr_1. I would consider that a clear bug.

Your code will also see expr_01 and decide that the next name should
be expr_2 rather than expr_1. I don't think that's right, either.

I also think that it's a bug that your function sometimes returns a
value different from the one it appends to canon_names.

I don't really understand why you're so fixed on this point. I think
that the code as I wrote it is quite a normal way to write code for
that kind of thing. Sure, there are other things that we could do, but
I wrote the code that way I did precisely in order to avoid behaviors
like the ones I mention above. It would be possible to rearrange the
code so that the termination condition for the loop was something like
"!found", or to rewrite the loop as a do { ... } while (!found)
construct as we do in set_rtable_names() for a very similar problem to
the code that this is solving, but I think the generated machine code
would be exactly the same and the code would not look as intuitive for
a human to read. Somebody else might have had a different stylistic
preference, but if you are going to object every time I write for (;;)
or while (1), you're going to hate an awful lot of my code for, IMHO,
very little reason. It's reasonable to be concerned about whether a
loop will ever terminate, but the mere fact of putting the loop exit
someplace other than the top of the loop isn't enough to say that
there's a problem.

--
Robert Haas
EDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2025-09-29 14:17:05 Re: [PATCH] Fix pg_rewind false positives caused by shutdown-only WAL
Previous Message Ivan Kovmir 2025-09-29 14:07:28 Mutable listen_addresses GUC