Re: BUG #18077: PostgreSQL server subprocess crashed by a SELECT statement with WITH clause

On Sat, Sep 16, 2023 at 2:38 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Richard Guo <guofenglinux(at)gmail(dot)com> writes:
> > Here is v3 patch which is v2 + fix for this issue.
>
> This seems not quite right yet: we need to pass the correct
> parent-namespaces list to set_deparse_for_query, else set_rtable_names
> might make unexpected choices. I think the net effect of what you
> have would only be to make generated table-alias names more unique
> than necessary (i.e, avoiding collisions with names that are not
> really in scope), but still this could be confusingly inconsistent.
> So we should do more like the attached.

Yes, you're right. And we need to do the same for the RTE_CTE case.

> I set about back-patching this, and discovered that your deparse
> test case exposes additional problems in the back branches. We
> get "record type has not been registered" failures in deparsing,
> or even in trying to parse the view to begin with, unless we
> back-patch d57534740 into pre-v14 branches and also 8b7a0f1d1
> into pre-v13 branches. At the time I'd thought d57534740's bug
> could not be exposed without SEARCH BREADTH FIRST, but that was
> clearly a failure of imagination. As for 8b7a0f1d1, I'd judged
> it too narrow of a corner case to be worth back-patching, and
> maybe it still is: I don't think it's reachable without attempting
> to fetch a ".fN" field out of an anonymous record type. Still,
> we do document that ".fN" is what the generated names are, so
> it seems like people ought to be able to use them. On balance,
> therefore, I'm inclined to back-patch both of those.

Agreed. Thanks for pushing and back-patching this.

Thanks
Richard