Re: oversight in EphemeralNamedRelation support

On Fri, Oct 13, 2017 at 8:50 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Julien Rouhaud <rjuju123(at)gmail(dot)com> writes:
>> On Mon, Oct 9, 2017 at 10:43 PM, Thomas Munro
>> <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
>>> I suppose we could consider moving the schemaname check into
>>> getRTEForSpecialRelationType(), since otherwise both callers need to
>>> do that (and as you discovered, one forgot).
>
>> Thanks for the feedback. That was my first idea, but I assumed there
>> could be future use for this function on qualified RangeVar if it
>> wasn't done this way.
>
>> I agree it'd be much safer, so v2 attached, check moved in
>> getRTEForSpecialRelationType().
>
> Hm. I actually think the bug here is that 18ce3a4ab introduced
> anything into setTargetTable at all. There was never previously
> any assumption that the target could be anything but a regular
> table, so we just ignored CTEs there, and I do not think the
> new behavior is an improvement.
>
> So my proposal is to rip out the getRTEForSpecialRelationTypes
> check there. I tend to agree that getRTEForSpecialRelationTypes
> should probably contain an explicit check for unqualified name
> rather than relying on its caller ... but that's a matter of
> future-proofing not a bug fix.

That check arrived in v11 revision of the patch:

https://www.postgresql.org/message-id/CACjxUsPfUUa813oDvJRx2wuiqHXO3VsCLQzcuy0r%3DUEyS-xOjQ%40mail.gmail.com

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);
^

... but ENRs used like that caused a crash. The change to
setTargetTable() went in to prevent that (and improved the CTE case's
error message semi-incidentally). To take out we'll need a new check
somewhere else to prevent that. Where?

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