From: | Peter Geoghegan <pg(at)heroku(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Thomas Alton <thomas(dot)alton(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Re: [BUGS] BUG #14153: Unrecognized node type error when upsert is present in recursive CTE |
Date: | 2016-05-23 20:43:49 |
Message-ID: | CAM3SWZTwROYTfeJGA0W9=_3=hX_CP2U7fdpe_KnaJGRoxe3vVA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs pgsql-hackers |
On Mon, May 23, 2016 at 12:48 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> 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 of a concern, but code bloat and
> inconsistency with other cases are valid concerns. That function does
> not recurse into name lists in, for example, the A_Expr and FuncCall
> cases.
Okay. Noted.
> 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-op
> traverse of every raw parse tree produced by the grammar. However that
> doesn't work because raw_expression_tree_walker doesn't promise to support
> everything, only DML statements. Thoughts?
Why couldn't the debug code be executed conditionally, too? Since
raw_expression_tree_walker() promises to work for "SelectStmt and
everything that could appear under it", I don't think it would be
difficult to find a choke point for that. Perhaps there is some
subtlety I've missed, since what I propose seems too obvious. FWIW, I
don't think it would much matter if this debug code was not reliably
executed for every possible SelectStmt. Just limiting it to top-level
SelectStmts would have easily caught this bug.
--
Peter Geoghegan
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2016-05-23 20:55:49 | Re: Re: [BUGS] BUG #14153: Unrecognized node type error when upsert is present in recursive CTE |
Previous Message | Tom Lane | 2016-05-23 19:48:38 | Re: Re: [BUGS] BUG #14153: Unrecognized node type error when upsert is present in recursive CTE |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2016-05-23 20:55:49 | Re: Re: [BUGS] BUG #14153: Unrecognized node type error when upsert is present in recursive CTE |
Previous Message | Tom Lane | 2016-05-23 20:42:46 | Re: Changed SRF in targetlist handling |