Re: Re: [BUGS] BUG #14153: Unrecognized node type error when upsert is present in recursive CTE

Peter Geoghegan <pg(at)heroku(dot)com> writes:
> What I meant is that I think naively adding the choke-point for a
> top-level SelectStmt would do the job (although perhaps we could do
> better). I wasn't suggesting that we'd avoid recursing from there;
> only that we could reasonably avoid recursing from someplace else
> (e.g. InsertStmt) in the hope of finding a SelectStmt to test.
> Offhand, I don't imagine that that would offer better coverage.

I think it would. The attached patch shows nearly 150 failures in
the current regression tests. If I remove "case T_InsertStmt" that
drops to two failures: there are two cases in with.sgml that almost
manage to exercise the bug, but not quite because they are not
WITH RECURSIVE. That doesn't leave me with any warm feeling about
being able to find other similar oversights if we apply this testing
only to top-level SelectStmts.

>> If that sounds like a plausible choke-point, the next question is what
>> to use to enable the debug test. I propose "#ifdef COPY_PARSE_PLAN_TREES"
>> since that enables similar sanity checking for other parts of
>> backend/nodes/, and we do have at least one buildfarm member using it.

> That's what I was thinking, too. No need to keep it separate.

After cogitating, I did it as attached just for readability's sake.

regards, tom lane