Обсуждение: insert on conflict on updatable views

Hi.

While working on updating the patch for the bug reported below:

https://www.postgresql.org/message-id/flat/CAFYwGJ0xfzy8jaK80hVN2eUWr6huce0RU8AgU04MGD00igqkTg%40mail.gmail.com

I noticed that the EXCLUDED pseudo-relation allows accessing columns that,
ISTM, should rather be inaccessible.

Example:

create table foo (a int unique, b text);
create view foo_view as select b, a+1 as c, a from foo;

explain insert into foo_view (a, b) select 1, 2 on conflict (a) do update
set b = excluded.b where excluded.c > 0;

The excluded.c above should result in an error, because it is impossible
for a user to specify a value for it, as shown below:

insert into foo_view (a, b, c) select 1, 2, 3 on conflict (a) do update
set b = excluded.b where excluded.c > 0;
ERROR:  cannot insert into column "c" of view "foo_view"
DETAIL:  View columns that are not columns of their base relation are not
updatable

IOW, the EXCLUDED pseudo-relation should only allow referencing the
columns present in the underlying physical relation.  Thoughts?

Thanks,
Amit

On Wed, Aug 1, 2018 at 7:15 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> create table foo (a int unique, b text);
> create view foo_view as select b, a+1 as c, a from foo;
>
> explain insert into foo_view (a, b) select 1, 2 on conflict (a) do update
> set b = excluded.b where excluded.c > 0;

> IOW, the EXCLUDED pseudo-relation should only allow referencing the
> columns present in the underlying physical relation.  Thoughts?

EXCLUDED represents the values that the user attempted to insert, so
it's arguably useful that EXCLUDED.c is accessible here, even though
the target rel doesn't have a "c" column.

Do you have some particular practical problem in mind here, or are you
just concerned about the semantics being exactly consistent?

-- 
Peter Geoghegan

On 2018/08/02 11:24, Peter Geoghegan wrote:
> On Wed, Aug 1, 2018 at 7:15 PM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> create table foo (a int unique, b text);
>> create view foo_view as select b, a+1 as c, a from foo;
>>
>> explain insert into foo_view (a, b) select 1, 2 on conflict (a) do update
>> set b = excluded.b where excluded.c > 0;
> 
>> IOW, the EXCLUDED pseudo-relation should only allow referencing the
>> columns present in the underlying physical relation.  Thoughts?
> 
> EXCLUDED represents the values that the user attempted to insert, so
> it's arguably useful that EXCLUDED.c is accessible here, even though
> the target rel doesn't have a "c" column.

Maybe it's OK while we're still in the parser, where as long as parser
transformation doesn't find that EXCLUDED.c is problematic, we won't emit
an error.  That is, as long as transformExpr() applied to OnConflictClause
can resolve "c", there is nothing to complain about.

But...

> Do you have some particular practical problem in mind here, or are you
> just concerned about the semantics being exactly consistent?

Once we get to the rewriter, where the INSERT on the view is transformed
into into one on the underlying physical relation, then it's clear which
columns we can go ahead with (and which ones we cannot).  In fact,
rewriteTargetView() even forbids inserting into columns that are not
present in the underlying relation.  So, from this point on, ON CONFLICT
handling code cannot rely on being able to compute the columns not present
in the underlying relation.

Thanks,
Amit