On Mon, Jun 09, 2025 at 07:07:42PM +0900, Fujii Masao wrote:
> Shouldn't this part be skipped in cases where the SELECT with parens is
> followed by a clause? At least in those cases, this logic doesn't seem
> appropriate.
Touché. That means messing with any parent Node that includes or
touches select_with_parens, select_no_parens (JSON_ARRAY has one) or
SelectStmt (few of these, like in CTAS). That's not cool in the long
term because we would expect any new node that englobes a SelectStmt
to be able to set up their inner length and location as they should,
as far as I get it. I was wondering first if we could have done
something with the top-level statement depending on the nesting level
of PGSS, but that's not going to fly high, for example with cases like
this one (any case that has an inner SELECT):
copy ((select 1) union ((select 1)) fetch first 1 row only) to stdout
Even with that in mind, a second thing I was wondering is to compile
the length within select_with_parens, still there seems to be an extra
parenthesis pending at the end of some of the strings. Perhaps I'm
missing a simpler concept, of course.
Anyway, this is the second bug that we have in this area, and this one
is worse. Now that we are in the middle of beta, this is gathering
the signs that we should revert the change from the tree for the
moment and potentially revisit the whole in v19 with these edge cases
handled. So attached is a patch doing a revert of:
499edb09741b, nested statement tracking.
06450c7b8c70, follow-up fix for 499edb09741b.
There was also 6b652e6ce85a for the addition of query ID for CREATE
TABLE AS and DECLARE, and this is still OK as a standalone change.
Its tests are updated with an extra entry added in the non-top-level
case, with the query string matching the top-level string as a result
of the attached.
If you have comments, feel free.
--
Michael
