Re: oversight in EphemeralNamedRelation support

On Fri, Oct 13, 2017 at 10:01 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> writes:
>> Before that, CTE used as modify targets produced a different error message:
>
>> postgres=# WITH d AS (SELECT 42) INSERT INTO d VALUES (1);
>> ERROR: relation "d" does not exist
>> LINE 1: WITH d AS (SELECT 42) INSERT INTO d VALUES (1);
>> ^
>
> Well, I think that is a poorly chosen example. Consider this instead:
> pre-v10, you could do this:
>
> regression=# create table mytable (f1 int);
> CREATE TABLE
> regression=# with mytable as (select 1 as x) insert into mytable values(1);
> INSERT 0 1
> regression=# select * from mytable;
> f1
> ----
> 1
> (1 row)
>
> The CTE was simply not part of the available namespace for the INSERT's
> target, so it found the regular table instead. v10 has thus broken
> cases that used to work. I think that's a bug.

Hmm. Yeah. I have to say it's a bit surprising that the following
refers to two different objects:

with mytable as (select 1 as x) insert into mytable select * from mytable

Obviously the spec is useless here since this is non-standard (at a
guess they'd probably require a qualifier there to avoid parsing as a
<query name> if they allowed DML after <with clause>). As you said
it's worked like that for several releases, so whatever I might think
about someone who deliberately writes such queries, the precedent
probably trumps naive notions about WITH creating a single consistent
lexical scope.

> There may or may not be a case for allowing ENRs to capture names that
> would otherwise refer to ordinary tables; I'm not sure. But I see very
> little case for allowing CTEs to capture such references, because surely
> we are never going to allow that to do anything useful, and we have
> several years of precedent now that they don't capture.
>
> I think we need to either remove that call from setTargetTable entirely,
> or maybe adjust it so it can only find ENRs not CTEs.

I think it'd be better to find and reject ENRs only. The alternative
would be to make ENRs invisible to DML, which seems arbitrary and
weird (even though it might be more consistent with our traditional
treatment of CTEs). One handwavy reason I'd like them to remain
visible to DML (and be rejected) is that I think they're a bit like
table variables (see SQL Server), and someone might eventually want to
teach them, or something like them, how to be writable.

--
Thomas Munro
http://www.enterprisedb.com