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

2016-05-23 Thread Peter Geoghegan
On Mon, May 23, 2016 at 4:27 PM, Tom Lane wrote: >> You also have to be aware of the new thing, which a lot of hackers >> will not be, if awareness of COPY_PARSE_PLAN_TREES is anything to go >> by. > > I doubt that anybody ever enables that in manual builds anyway. > The important thing is to have

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

2016-05-23 Thread Tom Lane
Peter Geoghegan writes: > On Mon, May 23, 2016 at 4:11 PM, Tom Lane wrote: >> And after further thought, I decided that that was penny-wise and >> pound-foolish; it's more readable if the #define is just an independent >> pg_config_manual.h entry. The only work it'd save is the need to update >>

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

2016-05-23 Thread Peter Geoghegan
On Mon, May 23, 2016 at 4:11 PM, Tom Lane wrote: > And after further thought, I decided that that was penny-wise and > pound-foolish; it's more readable if the #define is just an independent > pg_config_manual.h entry. The only work it'd save is the need to update > a buildfarm animal or two to a

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

2016-05-23 Thread Tom Lane
I wrote: > Peter Geoghegan writes: >>> 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

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

2016-05-23 Thread Tom Lane
Peter Geoghegan 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

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

2016-05-23 Thread Peter Geoghegan
On Mon, May 23, 2016 at 1:55 PM, Tom Lane wrote: > Um, I think not --- you need the case cited by the OP, namely an INSERT > ON CONFLICT in a CTE in a SelectStmt. If we'd had any of those in the > regression tests, we'd have found the bug, but we don't; and it's not > an obvious combination to tr

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

2016-05-23 Thread Tom Lane
Peter Geoghegan writes: > On Mon, May 23, 2016 at 12:48 PM, Tom Lane wrote: >> Also, related to this complaint though not this patch, it's disturbing >> that this oversight wasn't detected long ago. My first thought was to add >> some conditionally-compiled debugging code that just performs a no

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

2016-05-23 Thread Peter Geoghegan
On Mon, May 23, 2016 at 12:48 PM, Tom Lane wrote: >> I saw no reason to avoid the extra cycles. A noticeable omission has a >> cost: it gets noticed. Given this code path is likely to hardly ever >> be hit, this mechanical approach seemed best. That's all. > > I agree that performance isn't much o

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

2016-05-23 Thread Tom Lane
Peter Geoghegan writes: > On Mon, May 23, 2016 at 12:22 PM, Tom Lane wrote: >> It seems unlikely to me that recursing into the name lists is helpful >> here: those are not going to contain any data that is interpretable >> without context. Did you have a reason to do that? > I saw no reason to

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

2016-05-23 Thread Peter Geoghegan
On Mon, May 23, 2016 at 12:22 PM, Tom Lane wrote: > It seems unlikely to me that recursing into the name lists is helpful > here: those are not going to contain any data that is interpretable > without context. Did you have a reason to do that? I saw no reason to avoid the extra cycles. A notice

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

2016-05-23 Thread Tom Lane
Peter Geoghegan writes: > Attached patch fixes this issue by adding the appropriate > raw_expression_tree_walker handler. This isn't the first quasi-utility > node to need such a handler, so the fix is simple. It seems unlikely to me that recursing into the name lists is helpful here: those are n

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

2016-05-23 Thread Peter Geoghegan
On Sat, May 21, 2016 at 5:03 PM, Peter Geoghegan wrote: > On Sat, May 21, 2016 at 4:28 PM, wrote: >> ERROR: XX000: unrecognized node type: 920 >> LOCATION: raw_expression_tree_walker, nodeFuncs.c:3410 >> >> I expected the query run successfully and return one row with 'a'. > > This is clearly