Re: [HACKERS] oversight in EphemeralNamedRelation support
От | Thomas Munro |
---|---|
Тема | Re: [HACKERS] oversight in EphemeralNamedRelation support |
Дата | |
Msg-id | CAEepm=01HRS0TZQmXbdBogfimi7E8LH_z70COPYoubKPG7HO3A@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: [HACKERS] oversight in EphemeralNamedRelation support (Tom Lane <tgl@sss.pgh.pa.us>) |
Ответы |
Re: [HACKERS] oversight in EphemeralNamedRelation support
(Tom Lane <tgl@sss.pgh.pa.us>)
|
Список | pgsql-hackers |
On Fri, Oct 13, 2017 at 12:46 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Thomas Munro <thomas.munro@enterprisedb.com> writes: >> On Fri, Oct 13, 2017 at 10:01 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> 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 > > Yeah, I agree --- personally I'd never write a query like that. But > the fact that somebody ran into it when v10 has been out for barely > a week suggests that people are doing it. Not exactly -- Julien's bug report was about a *qualified* reference being incorrectly rejected. >>> 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. > > Yeah, that would be the argument for making them visible. I'm not > sure how likely it is that we'll ever actually have writable ENRs, > but I won't stand in the way if you want to do it like that. I hope so :-) I might be a nice way to get cheap locally scoped temporary tables, among other things. Okay, here's Julien's patch tweaked to reject just the ENR case. This takes us back to the 9.6 behaviour where CTEs don't hide tables in this context. I also removed the schema qualification in his regression test so we don't break that again. This way, his query from the first message in the thread works with the schema qualification (the bug he reported) and without it (the bug or at least incompatible change from 9.6 you discovered). I considered testing for a NULL return from parserOpenTable() instead of the way the patch has it, since parserOpenTable() already had an explicit test for ENRs, but its coding would give preference to an unqualified table of the same name. I considered moving the test for an ENR match higher up in parserOpenTable(), and that might have been OK, but then I realised no code in the tree actually tests for its undocumented NULL return value anyway. I think that NULL-returning branch is dead code, and all tests pass without it. Shouldn't we just remove it, as in the attached? I renamed the ENR used in plpgsql.sql's transition_table_level2_bad_usage_func() and with.sql's "sane response" test, because they both confused matters by using an ENR with the name "d" which is also the name of an existing table. For example, if you start with unpatched master, rename transition_table_level2_bad_usage_func()'s ENR to "dx" and simply remove the check for ENRs from setTargetTable() as you originally suggested, you'll get a segfault because the NULL return from parserOpenTable() wasn't checked. If you leave transition_table_level2_bad_usage_func()'s ENR name as "d" it'll quietly access the wrong table instead, which is misleading. -- Thomas Munro http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Вложения
В списке pgsql-hackers по дате отправления: