pg.ddx.io pgsql-admin@postgresql.org mailing list archive
help / color / mirror / Atom feedProblem with a "complex" upsert
30+ messages / 8 participants
[nested] [flat]
Commits from this thread:
2018-08-04 b8a1247a34 e7154b6acf b484bffe7d f6a124d019 5ad143cda0 Fix INSERT ON CONFLICT UPDATE through a view that isn't just SELECT *.
* Problem with a "complex" upsert
@ 2018-06-21 14:07 Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:40 ` Re: Problem with a "complex" upsert Geoff Winkless <pgsqladmin@geoff.dj>
0 siblings, 2 replies; 30+ messages in thread
From: Mario De Frutos Dieguez @ 2018-06-21 14:07 UTC (permalink / raw)
To: pgsql-admin@lists.postgresql.org
I'm trying to do an upsert to an updatable view with the following SQL
query:
INSERT INTO "acs2014_5yr"."b01003" (geoid,
b01003001)
SELECT (left(acs.geoid, 7) || bi.blockid) geoid, (b01003001 *
(percentage*100.0)) b01003001
FROM "tiger2015".blocks_interpolation bi
INNER JOIN "acs2014_5yr"."b01003" acs ON bi.blockgroupid =
substr(acs.geoid,8)
WHERE acs.geoid = '15000US020200001013' AND char_length(substr(acs.geoid,
8)) = 12
ON CONFLICT (geoid) DO UPDATE SET (b01003001) = ROW(EXCLUDED.b01003001);
The View is:
View "acs2014_5yr.b01003"
Column | Type | Collation | Nullable | Default |
Storage | Description
-----------+-----------------------+-----------+----------+---------+----------+-------------
geoid | character varying(40) | | | |
extended |
b01003001 | double precision | | | |
plain |
View definition:
SELECT seq0003.geoid,
seq0003.b01003001
FROM acs2014_5yr.seq0003;
If I don't get any conflict everything works as intended but if we hit a
conflict then I get the following error message:
ERROR: attribute 2 of type record has the wrong type
DETAIL: Table has type character varying, but query expects double
precision.
Looks like it's trying to use the geoid value in the b01003001 field.
I've tried using the source insert table data but the server crashes:
INSERT INTO "acs2014_5yr"."b01003" (geoid,
b01003001)
SELECT (left(acs.geoid, 7) || bi.blockid) geoid, (b01003001 *
(percentage*100.0))::float b01003001
FROM "tiger2015".blocks_interpolation bi
INNER JOIN "acs2014_5yr"."b01003" acs ON bi.blockgroupid =
substr(acs.geoid,8)
WHERE acs.geoid = '15000US020200001013' AND char_length(substr(acs.geoid,
8)) = 12
ON CONFLICT (geoid) DO UPDATE SET (b01003001) =
ROW("acs2014_5yr"."b01003".b01003001);
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
Any clues? Could be a bug? I see something similar here
https://www.postgresql.org/message-id/CAEzk6fdzJ3xYQZGbcuYM2rBd2BuDkUksmK=mY9UYYDugg_GgZg@mail.gmail...
and it was a bug
^ permalink raw reply [nested|flat] 30+ messages in thread
* Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
@ 2018-06-21 14:11 ` Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 16:55 ` Re: Problem with a "complex" upsert Peter Geoghegan <pg@bowt.ie>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 2 replies; 30+ messages in thread
From: Mario De Frutos Dieguez @ 2018-06-21 14:11 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org
I'm trying to do an upsert to an updatable view with the following SQL
query:
INSERT INTO "acs2014_5yr"."b01003" (geoid, b01003001)
SELECT (left(acs.geoid, 7) || bi.blockid) geoid, (b01003001 *
(percentage*100.0)) b01003001
FROM "tiger2015".blocks_interpolation bi
INNER JOIN "acs2014_5yr"."b01003" acs ON bi.blockgroupid =
substr(acs.geoid,8)
WHERE acs.geoid = '15000US020200001013' AND char_length(substr(acs.geoid,
8)) = 12
ON CONFLICT (geoid) DO UPDATE SET (b01003001) = ROW(EXCLUDED.b01003001);
The View is:
View "acs2014_5yr.b01003"
Column | Type | Collation | Nullable | Default |
Storage | Description
-----------+-----------------------+-----------+----------+-
--------+----------+-------------
geoid | character varying(40) | | | |
extended |
b01003001 | double precision | | | |
plain |
View definition:
SELECT seq0003.geoid,
seq0003.b01003001
FROM acs2014_5yr.seq0003;
If I don't get any conflict everything works as intended but if we hit a
conflict then I get the following error message:
ERROR: attribute 2 of type record has the wrong type
DETAIL: Table has type character varying, but query expects double
precision.
Looks like it's trying to use the geoid value in the b01003001 field.
I've tried using the source insert table data but the server crashes:
INSERT INTO "acs2014_5yr"."b01003" (geoid, b01003001)
SELECT (left(acs.geoid, 7) || bi.blockid) geoid, (b01003001 *
(percentage*100.0))::float b01003001
FROM "tiger2015".blocks_interpolation bi
INNER JOIN "acs2014_5yr"."b01003" acs ON bi.blockgroupid =
substr(acs.geoid,8)
WHERE acs.geoid = '15000US020200001013' AND char_length(substr(acs.geoid,
8)) = 12
ON CONFLICT (geoid) DO UPDATE SET (b01003001) = ROW("acs2014_5yr"."b01003".
b01003001);
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
Any clues? Could be a bug? I see something similar here
https://www.postgresql.org/message-id/CAEzk6fdzJ3xYQZGbcuYM2rBd2BuDk
UksmK=mY9UYYDugg_GgZg@mail.gmail.com and it was a bug
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
@ 2018-06-21 16:55 ` Peter Geoghegan <pg@bowt.ie>
1 sibling, 0 replies; 30+ messages in thread
From: Peter Geoghegan @ 2018-06-21 16:55 UTC (permalink / raw)
To: Mario De Frutos Dieguez <mariodefrutos@gmail.com>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Thu, Jun 21, 2018 at 7:11 AM, Mario De Frutos Dieguez
<mariodefrutos@gmail.com> wrote:
> I've tried using the source insert table data but the server crashes:
>
> INSERT INTO "acs2014_5yr"."b01003" (geoid, b01003001)
> SELECT (left(acs.geoid, 7) || bi.blockid) geoid, (b01003001 *
> (percentage*100.0))::float b01003001
> FROM "tiger2015".blocks_interpolation bi
> INNER JOIN "acs2014_5yr"."b01003" acs ON bi.blockgroupid =
> substr(acs.geoid,8)
> WHERE acs.geoid = '15000US020200001013' AND char_length(substr(acs.geoid,
> 8)) = 12
> ON CONFLICT (geoid) DO UPDATE SET (b01003001) =
> ROW("acs2014_5yr"."b01003".b01003001);
>
> server closed the connection unexpectedly
> This probably means the server terminated abnormally
> before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
>
> Any clues? Could be a bug? I see something similar here
It would be very helpful if you could get a stack trace of the crashing backend:
https://wiki.postgresql.org/wiki/Getting_a_stack_trace_of_a_running_PostgreSQL_backend_on_Linux/BSD#...
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
@ 2018-06-21 17:05 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
1 sibling, 1 reply; 30+ messages in thread
From: Tom Lane @ 2018-06-21 17:05 UTC (permalink / raw)
To: Mario De Frutos Dieguez <mariodefrutos@gmail.com>; +Cc: pgsql-bugs@lists.postgresql.org
Mario De Frutos Dieguez <mariodefrutos@gmail.com> writes:
> I'm trying to do an upsert to an updatable view with the following SQL
> query:
> ...
> If I don't get any conflict everything works as intended but if we hit a
> conflict then I get the following error message:
> ERROR: attribute 2 of type record has the wrong type
> DETAIL: Table has type character varying, but query expects double
> precision.
When filing a bug report, it's a good idea to provide both a self-
contained test case and a mention of what PG version you're using.
I guess from the ROW() syntax you used here, which isn't accepted pre-v10,
that you're using 10.0 or later, but that's not specific enough.
I tried to duplicate this problem using the attached script, but it
works for me.
FWIW, that error message definitely looks like a bug, but I can't
tell whether it's an already-fixed bug or there's some triggering
detail you didn't mention.
regards, tom lane
Attachments:
this-test-case-doesnt-fail.sql (text/plain, 1.0K) download
drop schema if exists tiger2015 cascade;
drop schema if exists acs2014_5yr cascade;
create schema tiger2015;
create table tiger2015.blocks_interpolation (
blockid text,
blockgroupid text,
percentage float8
);
create schema acs2014_5yr;
create table acs2014_5yr.seq0003 (
geoid character varying(40) primary key,
b01003001 double precision
);
insert into acs2014_5yr.seq0003 values ('15000US020200001013', 42);
insert into tiger2015.blocks_interpolation values
('020200001013', '020200001013', 99);
create view acs2014_5yr.b01003 as
SELECT seq0003.geoid,
seq0003.b01003001
FROM acs2014_5yr.seq0003;
INSERT INTO "acs2014_5yr"."b01003" (geoid, b01003001)
SELECT (left(acs.geoid, 7) || bi.blockid) geoid, (b01003001 *
(percentage*100.0)) b01003001
FROM "tiger2015".blocks_interpolation bi
INNER JOIN "acs2014_5yr"."b01003" acs ON bi.blockgroupid =
substr(acs.geoid,8)
WHERE acs.geoid = '15000US020200001013' AND char_length(substr(acs.geoid,
8)) = 12
ON CONFLICT (geoid) DO UPDATE SET (b01003001) = ROW(EXCLUDED.b01003001);
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
@ 2018-06-22 08:11 ` Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-06-22 09:15 ` Re: Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 30+ messages in thread
From: Amit Langote @ 2018-06-22 08:11 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; Mario De Frutos Dieguez <mariodefrutos@gmail.com>; +Cc: pgsql-bugs@lists.postgresql.org
On 2018/06/22 2:05, Tom Lane wrote:
> Mario De Frutos Dieguez <mariodefrutos@gmail.com> writes:
>> I'm trying to do an upsert to an updatable view with the following SQL
>> query:
>> ...
>> If I don't get any conflict everything works as intended but if we hit a
>> conflict then I get the following error message:
>> ERROR: attribute 2 of type record has the wrong type
>> DETAIL: Table has type character varying, but query expects double
>> precision.
>
> When filing a bug report, it's a good idea to provide both a self-
> contained test case and a mention of what PG version you're using.
>
> I guess from the ROW() syntax you used here, which isn't accepted pre-v10,
> that you're using 10.0 or later, but that's not specific enough.
>
> I tried to duplicate this problem using the attached script, but it
> works for me.
>
> FWIW, that error message definitely looks like a bug, but I can't
> tell whether it's an already-fixed bug or there's some triggering
> detail you didn't mention.
Having worked a little bit on the ON CONFLICT code recently, I was able to
guess at the triggering detail. At least, I was able to reproduce the
error and crash seen in the OP's report. Here's a minimal example:
create table foo (a text unique, b float);
insert into foo values ('xyxyxy', 1);
-- note the different order of columns in the view
create view foo_view as select b, a from foo;
insert into foo_view values (1, 'xyxyxy')
on conflict (a) do update set b = excluded.b;
ERROR: attribute 1 of type record has wrong type
DETAIL: Table has type text, but query expects double precision.
-- crash occurs if, like OP, I change EXCLUDED reference to target table
insert into foo_view values (1, 'xyxyxy') on conflict (a) do update set b
= foo_view.b;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
I tried debugging why that happens and concluded that rewriteTargetView
fails to *completely* account for the fact that the view's column's may
have different attribute numbers than the underlying table that the DO
UPDATE action will be applied to. Especially, even if the view's Vars are
replaced with those corresponding underlying base table's columns, the
TargetEntry's into which those Vars are contained are not refreshed, that
is, their resnos don't match varattnos.
I created a patch that seems to fix the issue, which also adds a
representative test in updatable_view.sql.
Thanks,
Amit
Attachments:
view-insert-on-conflict-bug-1.patch (text/plain, 3.9K) download
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 5b87c554f5..f951cb33a9 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -3032,6 +3032,52 @@ rewriteTargetView(Query *parsetree, Relation view)
elog(ERROR, "attribute number %d not found in view targetlist",
tle->resno);
}
+
+ /*
+ * Do the same for targetlists related to handling ON CONFLICT
+ * DO UPDATE.
+ */
+ if (parsetree->onConflict &&
+ parsetree->onConflict->action == ONCONFLICT_UPDATE)
+ {
+ foreach(lc, parsetree->onConflict->onConflictSet)
+ {
+ TargetEntry *tle = (TargetEntry *) lfirst(lc);
+ TargetEntry *view_tle;
+
+ if (tle->resjunk)
+ continue;
+
+ view_tle = get_tle_by_resno(view_targetlist, tle->resno);
+ if (view_tle != NULL && !view_tle->resjunk && IsA(view_tle->expr, Var))
+ tle->resno = ((Var *) view_tle->expr)->varattno;
+ else
+ elog(ERROR, "attribute number %d not found in view targetlist",
+ tle->resno);
+ }
+
+ /*
+ * exclRelTlist contains Var nodes corresponding to all attributes
+ * of the target (view) relation, although the corresponding
+ * entries in view_targetlist may not actually be Vars. Modify
+ * resnos of only those for which we found a Var node in
+ * view_targetlist.
+ */
+ foreach(lc, parsetree->onConflict->exclRelTlist)
+ {
+ TargetEntry *tle = (TargetEntry *) lfirst(lc);
+ TargetEntry *view_tle;
+
+ if (tle->resjunk)
+ continue;
+
+ view_tle = get_tle_by_resno(view_targetlist, tle->resno);
+
+ if (view_tle != NULL && !view_tle->resjunk &&
+ IsA(view_tle->expr, Var))
+ tle->resno = ((Var *) view_tle->expr)->varattno;
+ }
+ }
}
/*
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index b34bab4b29..cf5bd9584e 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -2578,3 +2578,20 @@ ERROR: new row violates check option for view "wcowrtest_v2"
DETAIL: Failing row contains (2, no such row in sometable).
drop view wcowrtest_v, wcowrtest_v2;
drop table wcowrtest, sometable;
+-- Check INSERT .. ON CONFLICT DO UPDATE works correctly when the view's
+-- columns may be ordered differently than the underlying table's.
+create table uv_iocu_tab (a text unique, b float);
+insert into uv_iocu_tab values ('xyxyxy', 1);
+-- note the different order of columns in the view
+create view uv_iocu_view as select b, a from uv_iocu_tab;
+insert into uv_iocu_view (a, b) values ('xyxyxy', 2) on conflict (a) do update set b = uv_iocu_view.b;
+insert into uv_iocu_view (a, b) values ('xyxyxy', 3) on conflict (a) do update set b = excluded.b;
+-- should display 'xyxyxy, 3'
+select * from uv_iocu_view;
+ b | a
+---+--------
+ 3 | xyxyxy
+(1 row)
+
+drop view uv_iocu_view;
+drop table uv_iocu_tab;
diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql
index a7786b26e9..cd3f7a9f33 100644
--- a/src/test/regress/sql/updatable_views.sql
+++ b/src/test/regress/sql/updatable_views.sql
@@ -1244,3 +1244,16 @@ insert into wcowrtest_v2 values (2, 'no such row in sometable');
drop view wcowrtest_v, wcowrtest_v2;
drop table wcowrtest, sometable;
+
+-- Check INSERT .. ON CONFLICT DO UPDATE works correctly when the view's
+-- columns may be ordered differently than the underlying table's.
+create table uv_iocu_tab (a text unique, b float);
+insert into uv_iocu_tab values ('xyxyxy', 1);
+-- note the different order of columns in the view
+create view uv_iocu_view as select b, a from uv_iocu_tab;
+insert into uv_iocu_view (a, b) values ('xyxyxy', 2) on conflict (a) do update set b = uv_iocu_view.b;
+insert into uv_iocu_view (a, b) values ('xyxyxy', 3) on conflict (a) do update set b = excluded.b;
+-- should display 'xyxyxy, 3'
+select * from uv_iocu_view;
+drop view uv_iocu_view;
+drop table uv_iocu_tab;
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
@ 2018-06-22 09:15 ` Mario De Frutos Dieguez <mariodefrutos@gmail.com>
1 sibling, 0 replies; 30+ messages in thread
From: Mario De Frutos Dieguez @ 2018-06-22 09:15 UTC (permalink / raw)
To: ; +Cc: pgsql-bugs@lists.postgresql.org
Wow, that's amazing news. Sorry for not being doing this in a proper way,
it was my first time guessing if I'm confronting a bug or not. For the next
time, I'll provide a more prepared answer :)
Thank you very much to all :)
2018-06-22 10:11 GMT+02:00 Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>:
> On 2018/06/22 2:05, Tom Lane wrote:
> > Mario De Frutos Dieguez <mariodefrutos@gmail.com> writes:
> >> I'm trying to do an upsert to an updatable view with the following SQL
> >> query:
> >> ...
> >> If I don't get any conflict everything works as intended but if we hit a
> >> conflict then I get the following error message:
> >> ERROR: attribute 2 of type record has the wrong type
> >> DETAIL: Table has type character varying, but query expects double
> >> precision.
> >
> > When filing a bug report, it's a good idea to provide both a self-
> > contained test case and a mention of what PG version you're using.
> >
> > I guess from the ROW() syntax you used here, which isn't accepted
> pre-v10,
> > that you're using 10.0 or later, but that's not specific enough.
> >
> > I tried to duplicate this problem using the attached script, but it
> > works for me.
> >
> > FWIW, that error message definitely looks like a bug, but I can't
> > tell whether it's an already-fixed bug or there's some triggering
> > detail you didn't mention.
>
> Having worked a little bit on the ON CONFLICT code recently, I was able to
> guess at the triggering detail. At least, I was able to reproduce the
> error and crash seen in the OP's report. Here's a minimal example:
>
> create table foo (a text unique, b float);
> insert into foo values ('xyxyxy', 1);
>
> -- note the different order of columns in the view
> create view foo_view as select b, a from foo;
>
> insert into foo_view values (1, 'xyxyxy')
> on conflict (a) do update set b = excluded.b;
> ERROR: attribute 1 of type record has wrong type
> DETAIL: Table has type text, but query expects double precision.
>
> -- crash occurs if, like OP, I change EXCLUDED reference to target table
> insert into foo_view values (1, 'xyxyxy') on conflict (a) do update set b
> = foo_view.b;
> server closed the connection unexpectedly
> This probably means the server terminated abnormally
> before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
>
> I tried debugging why that happens and concluded that rewriteTargetView
> fails to *completely* account for the fact that the view's column's may
> have different attribute numbers than the underlying table that the DO
> UPDATE action will be applied to. Especially, even if the view's Vars are
> replaced with those corresponding underlying base table's columns, the
> TargetEntry's into which those Vars are contained are not refreshed, that
> is, their resnos don't match varattnos.
>
> I created a patch that seems to fix the issue, which also adds a
> representative test in updatable_view.sql.
>
> Thanks,
> Amit
>
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
@ 2018-07-10 20:59 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-07-30 22:53 ` Re: Fwd: Problem with a "complex" upsert Peter Geoghegan <pg@bowt.ie>
2018-08-02 10:38 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
1 sibling, 2 replies; 30+ messages in thread
From: Tom Lane @ 2018-07-10 20:59 UTC (permalink / raw)
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; +Cc: Mario De Frutos Dieguez <mariodefrutos@gmail.com>; pgsql-bugs@lists.postgresql.org
Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
> Having worked a little bit on the ON CONFLICT code recently, I was able to
> guess at the triggering detail. At least, I was able to reproduce the
> error and crash seen in the OP's report. Here's a minimal example:
> create table foo (a text unique, b float);
> insert into foo values ('xyxyxy', 1);
> -- note the different order of columns in the view
> create view foo_view as select b, a from foo;
Ah-hah.
> I tried debugging why that happens and concluded that rewriteTargetView
> fails to *completely* account for the fact that the view's column's may
> have different attribute numbers than the underlying table that the DO
> UPDATE action will be applied to. Especially, even if the view's Vars are
> replaced with those corresponding underlying base table's columns, the
> TargetEntry's into which those Vars are contained are not refreshed, that
> is, their resnos don't match varattnos.
> I created a patch that seems to fix the issue, which also adds a
> representative test in updatable_view.sql.
Hm. I looked at this patch a bit. While the onConflictSet change looks
reasonable, I find the exclRelTlist change fishy. Shouldn't those resnos
correspond to the exclRelTlist's *own* vars, independently of what is or
isn't in the view_targetlist? And why is it OK to ignore failure to find
a match?
The provided test case doesn't seem to me to prove that that code is OK.
AFAICS, exclRelTlist only gets used by EXPLAIN, and there's no EXPLAIN
output in the test case.
regards, tom lane
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
@ 2018-07-30 22:53 ` Peter Geoghegan <pg@bowt.ie>
2018-07-31 01:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
1 sibling, 1 reply; 30+ messages in thread
From: Peter Geoghegan @ 2018-07-30 22:53 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; Mario De Frutos Dieguez <mariodefrutos@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Tue, Jul 10, 2018 at 1:59 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I tried debugging why that happens and concluded that rewriteTargetView
>> fails to *completely* account for the fact that the view's column's may
>> have different attribute numbers than the underlying table that the DO
>> UPDATE action will be applied to. Especially, even if the view's Vars are
>> replaced with those corresponding underlying base table's columns, the
>> TargetEntry's into which those Vars are contained are not refreshed, that
>> is, their resnos don't match varattnos.
>
>> I created a patch that seems to fix the issue, which also adds a
>> representative test in updatable_view.sql.
>
> Hm. I looked at this patch a bit. While the onConflictSet change looks
> reasonable, I find the exclRelTlist change fishy. Shouldn't those resnos
> correspond to the exclRelTlist's *own* vars, independently of what is or
> isn't in the view_targetlist? And why is it OK to ignore failure to find
> a match?
Any update on this, Amit? I would like to get this one out of the way soon.
Thanks
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-07-30 22:53 ` Re: Fwd: Problem with a "complex" upsert Peter Geoghegan <pg@bowt.ie>
@ 2018-07-31 01:11 ` Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
0 siblings, 0 replies; 30+ messages in thread
From: Amit Langote @ 2018-07-31 01:11 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Mario De Frutos Dieguez <mariodefrutos@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On 2018/07/31 7:53, Peter Geoghegan wrote:
> On Tue, Jul 10, 2018 at 1:59 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> I tried debugging why that happens and concluded that rewriteTargetView
>>> fails to *completely* account for the fact that the view's column's may
>>> have different attribute numbers than the underlying table that the DO
>>> UPDATE action will be applied to. Especially, even if the view's Vars are
>>> replaced with those corresponding underlying base table's columns, the
>>> TargetEntry's into which those Vars are contained are not refreshed, that
>>> is, their resnos don't match varattnos.
>>
>>> I created a patch that seems to fix the issue, which also adds a
>>> representative test in updatable_view.sql.
>>
>> Hm. I looked at this patch a bit. While the onConflictSet change looks
>> reasonable, I find the exclRelTlist change fishy. Shouldn't those resnos
>> correspond to the exclRelTlist's *own* vars, independently of what is or
>> isn't in the view_targetlist? And why is it OK to ignore failure to find
>> a match?
>
> Any update on this, Amit? I would like to get this one out of the way soon.
This has slipped my mind. I will look into updating the patch today.
Thanks,
Amit
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
@ 2018-08-02 10:38 ` Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 06:52 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
1 sibling, 1 reply; 30+ messages in thread
From: Amit Langote @ 2018-08-02 10:38 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Mario De Frutos Dieguez <mariodefrutos@gmail.com>; pgsql-bugs@lists.postgresql.org
Thanks for looking at the patch and sorry I couldn't reply sooner.
On 2018/07/11 5:59, Tom Lane wrote:
> Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
>> Having worked a little bit on the ON CONFLICT code recently, I was able to
>> guess at the triggering detail. At least, I was able to reproduce the
>> error and crash seen in the OP's report. Here's a minimal example:
>
>> create table foo (a text unique, b float);
>> insert into foo values ('xyxyxy', 1);
>
>> -- note the different order of columns in the view
>> create view foo_view as select b, a from foo;
>
> Ah-hah.
>
>> I tried debugging why that happens and concluded that rewriteTargetView
>> fails to *completely* account for the fact that the view's column's may
>> have different attribute numbers than the underlying table that the DO
>> UPDATE action will be applied to. Especially, even if the view's Vars are
>> replaced with those corresponding underlying base table's columns, the
>> TargetEntry's into which those Vars are contained are not refreshed, that
>> is, their resnos don't match varattnos.
>
>> I created a patch that seems to fix the issue, which also adds a
>> representative test in updatable_view.sql.
>
> Hm. I looked at this patch a bit. While the onConflictSet change looks
> reasonable, I find the exclRelTlist change fishy. Shouldn't those resnos
> correspond to the exclRelTlist's *own* vars, independently of what is or
> isn't in the view_targetlist? And why is it OK to ignore failure to find
> a match?
>
> The provided test case doesn't seem to me to prove that that code is OK.
> AFAICS, exclRelTlist only gets used by EXPLAIN, and there's no EXPLAIN
> output in the test case.
On further study, I have concluded that EXCLUDED pseudo-relation and any
references to it in the sub-expressions of OnConflictExpr need to be
revised after the rewriter has substituted target view relation with its
underlying base relation.
As things stand today, transformOnConflictClause creates the EXCLUDED
pseudo-relation based on the original target relation of the query, which
in this case is the view relation. rewriteTargetView replaces the view
relation with its underlying base relation, subject to various
restrictions on what the query can then do, such as not being able to
update columns that are not present in the underlying base relation.
On the same lines, I think OnConflictExpr shouldn't be allowed to contain
references to view's own columns via the EXCLUDED pseudo-relation. That's
because ON CONFLICT's execution machinery would be able to access only
those columns of the EXCLUDED tuple that are present in the underlying
physical relation. Hence, to account for the view relation's substitution
with its underlying physical relation, we should discard the original
EXCLUDED range table entry and target list (exclRelTlist) that parser
created based on the view relation and recreate both based on the
substituted base rel. Furthermore, any Vars contained in OnConflictExpr's
sub-expressions that reference original EXCLUDED rte will need to be
substituted with Vars based on the revised rte.
Attaching the updated patch, which is quite heavily revised from the
earlier patch, given the above findings.
Thanks,
Amit
Attachments:
v2-0001-Fix-INSERT-ON-CONFLICT-DO-UPDATE-on-updatable-vie.patch (text/plain, 21.8K) download
From 3359921fa13977226f8d1a51645181d3ff1f277a Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Fri, 22 Jun 2018 17:03:16 +0900
Subject: [PATCH v2] Fix INSERT ON CONFLICT DO UPDATE on updatable views
Rewriter forgot to change resnos of the entries in ON CONFLICT related
target lists to refer to the attribute numbers of a view's underlying
base relation instead of the view's own. Fix that because that's the
relation that the executor will try to apply DO UPDATE actions to.
---
src/backend/access/common/tupconvert.c | 6 +-
src/backend/catalog/partition.c | 3 +-
src/backend/catalog/pg_constraint.c | 3 +-
src/backend/commands/indexcmds.c | 3 +-
src/backend/commands/tablecmds.c | 12 ++-
src/backend/executor/execPartition.c | 9 +-
src/backend/parser/analyze.c | 121 +++++++++++++----------
src/backend/rewrite/rewriteHandler.c | 136 +++++++++++++++++++++++++-
src/include/access/tupconvert.h | 2 +-
src/include/parser/analyze.h | 2 +
src/test/regress/expected/updatable_views.out | 32 ++++++
src/test/regress/sql/updatable_views.sql | 20 ++++
12 files changed, 282 insertions(+), 67 deletions(-)
diff --git a/src/backend/access/common/tupconvert.c b/src/backend/access/common/tupconvert.c
index 3bc67b846d..4498810d2a 100644
--- a/src/backend/access/common/tupconvert.c
+++ b/src/backend/access/common/tupconvert.c
@@ -218,7 +218,7 @@ convert_tuples_by_name(TupleDesc indesc,
bool same;
/* Verify compatibility and prepare attribute-number map */
- attrMap = convert_tuples_by_name_map(indesc, outdesc, msg);
+ attrMap = convert_tuples_by_name_map(indesc, outdesc, msg, false);
/*
* Check to see if the map is one-to-one, in which case we need not do a
@@ -292,7 +292,7 @@ convert_tuples_by_name(TupleDesc indesc,
AttrNumber *
convert_tuples_by_name_map(TupleDesc indesc,
TupleDesc outdesc,
- const char *msg)
+ const char *msg, bool missing_ok)
{
AttrNumber *attrMap;
int outnatts;
@@ -355,7 +355,7 @@ convert_tuples_by_name_map(TupleDesc indesc,
break;
}
}
- if (attrMap[i] == 0)
+ if (attrMap[i] == 0 && !missing_ok)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg_internal("%s", _(msg)),
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 558022647c..a8b65bdab9 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -177,7 +177,8 @@ map_partition_varattnos(List *expr, int fromrel_varno,
part_attnos = convert_tuples_by_name_map(RelationGetDescr(to_rel),
RelationGetDescr(from_rel),
- gettext_noop("could not convert row type"));
+ gettext_noop("could not convert row type"),
+ false);
expr = (List *) map_variable_attnos((Node *) expr,
fromrel_varno, 0,
part_attnos,
diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 7a6d158f89..d9c4d6a656 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -438,7 +438,8 @@ CloneForeignKeyConstraints(Oid parentId, Oid relationId, List **cloned)
*/
attmap = convert_tuples_by_name_map(RelationGetDescr(rel),
RelationGetDescr(parentRel),
- gettext_noop("could not convert row type"));
+ gettext_noop("could not convert row type"),
+ false);
ScanKeyInit(&key,
Anum_pg_constraint_conrelid, BTEqualStrategyNumber,
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index b9dad9672e..b1d0a62cb8 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -914,7 +914,8 @@ DefineIndex(Oid relationId,
attmap =
convert_tuples_by_name_map(RelationGetDescr(childrel),
parentDesc,
- gettext_noop("could not convert row type"));
+ gettext_noop("could not convert row type"),
+ false);
maplen = parentDesc->natts;
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index eb2d33dd86..b169e37e8b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -938,7 +938,8 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
attmap = convert_tuples_by_name_map(RelationGetDescr(rel),
RelationGetDescr(parent),
- gettext_noop("could not convert row type"));
+ gettext_noop("could not convert row type"),
+ false);
idxstmt =
generateClonedIndexStmt(NULL, RelationGetRelid(rel), idxRel,
attmap, RelationGetDescr(rel)->natts,
@@ -9263,7 +9264,8 @@ ATPrepAlterColumnType(List **wqueue,
attmap = convert_tuples_by_name_map(RelationGetDescr(childrel),
RelationGetDescr(rel),
- gettext_noop("could not convert row type"));
+ gettext_noop("could not convert row type"),
+ false);
((ColumnDef *) cmd->def)->cooked_default =
map_variable_attnos(def->cooked_default,
1, 0,
@@ -14413,7 +14415,8 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel)
info = BuildIndexInfo(idxRel);
attmap = convert_tuples_by_name_map(RelationGetDescr(attachrel),
RelationGetDescr(rel),
- gettext_noop("could not convert row type"));
+ gettext_noop("could not convert row type"),
+ false);
constraintOid = get_relation_idx_constraint_oid(RelationGetRelid(rel), idx);
/*
@@ -14890,7 +14893,8 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name)
parentInfo = BuildIndexInfo(parentIdx);
attmap = convert_tuples_by_name_map(RelationGetDescr(partTbl),
RelationGetDescr(parentTbl),
- gettext_noop("could not convert row type"));
+ gettext_noop("could not convert row type"),
+ false);
if (!CompareIndexInfo(childInfo, parentInfo,
partIdx->rd_indcollation,
parentIdx->rd_indcollation,
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index d13be4145f..e875163bac 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -427,7 +427,8 @@ ExecInitPartitionInfo(ModifyTableState *mtstate,
part_attnos =
convert_tuples_by_name_map(RelationGetDescr(partrel),
RelationGetDescr(firstResultRel),
- gettext_noop("could not convert row type"));
+ gettext_noop("could not convert row type"),
+ false);
wcoList = (List *)
map_variable_attnos((Node *) wcoList,
firstVarno, 0,
@@ -487,7 +488,8 @@ ExecInitPartitionInfo(ModifyTableState *mtstate,
part_attnos =
convert_tuples_by_name_map(RelationGetDescr(partrel),
RelationGetDescr(firstResultRel),
- gettext_noop("could not convert row type"));
+ gettext_noop("could not convert row type"),
+ false);
returningList = (List *)
map_variable_attnos((Node *) returningList,
firstVarno, 0,
@@ -603,7 +605,8 @@ ExecInitPartitionInfo(ModifyTableState *mtstate,
part_attnos =
convert_tuples_by_name_map(RelationGetDescr(partrel),
RelationGetDescr(firstResultRel),
- gettext_noop("could not convert row type"));
+ gettext_noop("could not convert row type"),
+ false);
onconflset = (List *)
map_variable_attnos((Node *) onconflset,
INNER_VAR, 0,
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 05f57591e4..8e3ea6ef88 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -1022,9 +1022,6 @@ transformOnConflictClause(ParseState *pstate,
if (onConflictClause->action == ONCONFLICT_UPDATE)
{
Relation targetrel = pstate->p_target_relation;
- Var *var;
- TargetEntry *te;
- int attno;
/*
* All INSERT expressions have been parsed, get ready for potentially
@@ -1044,55 +1041,8 @@ transformOnConflictClause(ParseState *pstate,
exclRte->relkind = RELKIND_COMPOSITE_TYPE;
exclRelIndex = list_length(pstate->p_rtable);
- /*
- * Build a targetlist representing the columns of the EXCLUDED pseudo
- * relation. Have to be careful to use resnos that correspond to
- * attnos of the underlying relation.
- */
- for (attno = 0; attno < RelationGetNumberOfAttributes(targetrel); attno++)
- {
- Form_pg_attribute attr = TupleDescAttr(targetrel->rd_att, attno);
- char *name;
-
- if (attr->attisdropped)
- {
- /*
- * can't use atttypid here, but it doesn't really matter what
- * type the Const claims to be.
- */
- var = (Var *) makeNullConst(INT4OID, -1, InvalidOid);
- name = "";
- }
- else
- {
- var = makeVar(exclRelIndex, attno + 1,
- attr->atttypid, attr->atttypmod,
- attr->attcollation,
- 0);
- name = pstrdup(NameStr(attr->attname));
- }
-
- te = makeTargetEntry((Expr *) var,
- attno + 1,
- name,
- false);
-
- /* don't require select access yet */
- exclRelTlist = lappend(exclRelTlist, te);
- }
-
- /*
- * Add a whole-row-Var entry to support references to "EXCLUDED.*".
- * Like the other entries in exclRelTlist, its resno must match the
- * Var's varattno, else the wrong things happen while resolving
- * references in setrefs.c. This is against normal conventions for
- * targetlists, but it's okay since we don't use this as a real tlist.
- */
- var = makeVar(exclRelIndex, InvalidAttrNumber,
- targetrel->rd_rel->reltype,
- -1, InvalidOid, 0);
- te = makeTargetEntry((Expr *) var, InvalidAttrNumber, NULL, true);
- exclRelTlist = lappend(exclRelTlist, te);
+ exclRelTlist = BuildOnConflictExcludedTargetlist(targetrel,
+ exclRelIndex);
/*
* Add EXCLUDED and the target RTE to the namespace, so that they can
@@ -1125,6 +1075,73 @@ transformOnConflictClause(ParseState *pstate,
return result;
}
+/*
+ * CreateOnConflictExcludedTargetlist
+ * Create the target list of EXCLUDED pseudo-relation of ON CONFLICT
+ *
+ * Note: Exported for use in the rewriter.
+ */
+List *
+BuildOnConflictExcludedTargetlist(Relation targetrel,
+ Index exclRelIndex)
+{
+ List *result = NIL;
+ int attno;
+ Var *var;
+ TargetEntry *te;
+
+ /*
+ * Build a targetlist representing the columns of the EXCLUDED pseudo
+ * relation. Have to be careful to use resnos that correspond to
+ * attnos of the underlying relation.
+ */
+ for (attno = 0; attno < RelationGetNumberOfAttributes(targetrel); attno++)
+ {
+ Form_pg_attribute attr = TupleDescAttr(targetrel->rd_att, attno);
+ char *name;
+
+ if (attr->attisdropped)
+ {
+ /*
+ * can't use atttypid here, but it doesn't really matter what
+ * type the Const claims to be.
+ */
+ var = (Var *) makeNullConst(INT4OID, -1, InvalidOid);
+ name = "";
+ }
+ else
+ {
+ var = makeVar(exclRelIndex, attno + 1,
+ attr->atttypid, attr->atttypmod,
+ attr->attcollation,
+ 0);
+ name = pstrdup(NameStr(attr->attname));
+ }
+
+ te = makeTargetEntry((Expr *) var,
+ attno + 1,
+ name,
+ false);
+
+ /* don't require select access yet */
+ result = lappend(result, te);
+ }
+
+ /*
+ * Add a whole-row-Var entry to support references to "EXCLUDED.*".
+ * Like the other entries in exclRelTlist, its resno must match the
+ * Var's varattno, else the wrong things happen while resolving
+ * references in setrefs.c. This is against normal conventions for
+ * targetlists, but it's okay since we don't use this as a real tlist.
+ */
+ var = makeVar(exclRelIndex, InvalidAttrNumber,
+ targetrel->rd_rel->reltype,
+ -1, InvalidOid, 0);
+ te = makeTargetEntry((Expr *) var, InvalidAttrNumber, NULL, true);
+ result = lappend(result, te);
+
+ return result;
+}
/*
* count_rowexpr_columns -
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 5b87c554f5..06db9003cd 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -29,6 +29,7 @@
#include "nodes/nodeFuncs.h"
#include "parser/analyze.h"
#include "parser/parse_coerce.h"
+#include "parser/parse_relation.h"
#include "parser/parsetree.h"
#include "rewrite/rewriteDefine.h"
#include "rewrite/rewriteHandler.h"
@@ -80,7 +81,9 @@ static List *matchLocks(CmdType event, RuleLock *rulelocks,
static Query *fireRIRrules(Query *parsetree, List *activeRIRs);
static bool view_has_instead_trigger(Relation view, CmdType event);
static Bitmapset *adjust_view_column_set(Bitmapset *cols, List *targetlist);
-
+static void CheckOnConflictVars(OnConflictExpr *onConflict,
+ Bitmapset *orig_varnos,
+ Relation view, Relation base_rel);
/*
* AcquireRewriteLocks -
@@ -3035,6 +3038,86 @@ rewriteTargetView(Query *parsetree, Relation view)
}
/*
+ * Modify expressions in OnConflictExpr that would be affected by
+ * target rel substitution.
+ */
+ if (parsetree->onConflict &&
+ parsetree->onConflict->action == ONCONFLICT_UPDATE)
+ {
+ Index old_exclRelIndex,
+ new_exclRelIndex;
+ RangeTblEntry *new_exclRte;
+ AttrNumber *attrMap;
+ bool found_whole_row;
+
+ /*
+ * First modify the DO UPDATE SET targetlist like we did above for the
+ * main targetlist.
+ */
+ foreach(lc, parsetree->onConflict->onConflictSet)
+ {
+ TargetEntry *tle = (TargetEntry *) lfirst(lc);
+ TargetEntry *view_tle;
+
+ if (tle->resjunk)
+ continue;
+
+ view_tle = get_tle_by_resno(view_targetlist, tle->resno);
+ if (view_tle != NULL && !view_tle->resjunk && IsA(view_tle->expr, Var))
+ tle->resno = ((Var *) view_tle->expr)->varattno;
+ else
+ elog(ERROR, "attribute number %d not found in view targetlist",
+ tle->resno);
+ }
+
+ /*
+ * Modify EXCLUDED pseudo-relation to be based on the base relation
+ * instead of the view relation as it originally would be.
+ */
+ old_exclRelIndex = parsetree->onConflict->exclRelIndex;
+
+ /*
+ * Create a new RTE for EXCLUDED pseudo-relation based on base_rel.
+ * This closely mimics the code in transformOnConflictClause.
+ */
+ new_exclRte = addRangeTableEntryForRelation(make_parsestate(NULL),
+ base_rel,
+ makeAlias("excluded",
+ NIL),
+ false, false);
+ new_exclRte->relkind = RELKIND_COMPOSITE_TYPE;
+ parsetree->rtable = lappend(parsetree->rtable, new_exclRte);
+ new_exclRelIndex = parsetree->onConflict->exclRelIndex =
+ list_length(parsetree->rtable);
+
+ /* Change exclRelTlist into one containing base_rel's attrs. */
+ list_free(parsetree->onConflict->exclRelTlist);
+ parsetree->onConflict->exclRelTlist =
+ BuildOnConflictExcludedTargetlist(base_rel, new_exclRelIndex);
+
+ /* Check that none of EXCLUDED references contains view's columns. */
+ CheckOnConflictVars(parsetree->onConflict,
+ bms_make_singleton(old_exclRelIndex),
+ view, base_rel);
+
+ /* Convert Vars to use the new EXCLUDED pseudo relation. */
+ attrMap = convert_tuples_by_name_map(RelationGetDescr(base_rel),
+ RelationGetDescr(view),
+ gettext_noop("could not convert row type"),
+ true);
+ parsetree->onConflict = (OnConflictExpr *)
+ map_variable_attnos((Node *) parsetree->onConflict,
+ old_exclRelIndex, 0,
+ attrMap, RelationGetDescr(view)->natts,
+ RelationGetForm(view)->reltype,
+ &found_whole_row);
+ ChangeVarNodes((Node *) parsetree->onConflict,
+ old_exclRelIndex,
+ new_exclRelIndex,
+ 0);
+ }
+
+ /*
* For UPDATE/DELETE, pull up any WHERE quals from the view. We know that
* any Vars in the quals must reference the one base relation, so we need
* only adjust their varnos to reference the new target (just the same as
@@ -3164,6 +3247,57 @@ rewriteTargetView(Query *parsetree, Relation view)
return parsetree;
}
+typedef struct
+{
+ Bitmapset *orig_varnos;
+ Relation view;
+ Relation base_rel;
+} CheckOnConflictVars_context;
+
+static bool
+CheckOnConflictVars_walker(Node *node, CheckOnConflictVars_context *context)
+{
+ if (node == NULL)
+ return false;
+
+ if (IsA(node, Var) &&
+ bms_is_member(((Var *) node)->varno, context->orig_varnos))
+ {
+ AttrNumber attno = ((Var *) node)->varattno;
+ TupleDesc orig_tupdesc = RelationGetDescr(context->view);
+ TupleDesc new_tupdesc = RelationGetDescr(context->base_rel);
+ char *attname = orig_tupdesc->attrs[attno - 1].attname.data;
+
+ for (attno = 1; attno <= new_tupdesc->natts; attno++)
+ {
+ Form_pg_attribute att = &new_tupdesc->attrs[attno - 1];
+
+ if (strcmp(attname, att->attname.data) == 0)
+ return false;
+ }
+
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot access column \"%s\" of view \"%s\" in this part of query",
+ attname, RelationGetRelationName(context->view)),
+ errdetail("Only columns present in base relation can be used in ON CONFLICT specification")));
+ }
+
+ return expression_tree_walker(node, CheckOnConflictVars_walker, context);
+}
+
+static void
+CheckOnConflictVars(OnConflictExpr *onConflict, Bitmapset *orig_varnos,
+ Relation view, Relation base_rel)
+{
+ CheckOnConflictVars_context context;
+
+ context.orig_varnos = orig_varnos;
+ context.view = view;
+ context.base_rel = base_rel;
+
+ CheckOnConflictVars_walker((Node *) onConflict, &context);
+}
/*
* RewriteQuery -
diff --git a/src/include/access/tupconvert.h b/src/include/access/tupconvert.h
index 66c0ed0882..a382a23adc 100644
--- a/src/include/access/tupconvert.h
+++ b/src/include/access/tupconvert.h
@@ -40,7 +40,7 @@ extern TupleConversionMap *convert_tuples_by_name(TupleDesc indesc,
extern AttrNumber *convert_tuples_by_name_map(TupleDesc indesc,
TupleDesc outdesc,
- const char *msg);
+ const char *msg, bool missing_ok);
extern HeapTuple do_convert_tuple(HeapTuple tuple, TupleConversionMap *map);
diff --git a/src/include/parser/analyze.h b/src/include/parser/analyze.h
index 687ae1b5b7..31b6cd4db6 100644
--- a/src/include/parser/analyze.h
+++ b/src/include/parser/analyze.h
@@ -42,5 +42,7 @@ extern void CheckSelectLocking(Query *qry, LockClauseStrength strength);
extern void applyLockingClause(Query *qry, Index rtindex,
LockClauseStrength strength,
LockWaitPolicy waitPolicy, bool pushedDown);
+extern List *BuildOnConflictExcludedTargetlist(Relation targetrel,
+ Index exclRelIndex);
#endif /* ANALYZE_H */
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index b34bab4b29..d128b45e68 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -2578,3 +2578,35 @@ ERROR: new row violates check option for view "wcowrtest_v2"
DETAIL: Failing row contains (2, no such row in sometable).
drop view wcowrtest_v, wcowrtest_v2;
drop table wcowrtest, sometable;
+-- Check INSERT .. ON CONFLICT DO UPDATE works correctly when the view's
+-- columns may be ordered differently than the underlying table's.
+create table uv_iocu_tab (a text unique, b float);
+insert into uv_iocu_tab values ('xyxyxy', 1);
+-- note the different order of columns in the view
+create view uv_iocu_view as select b, b+1 as c, a from uv_iocu_tab;
+insert into uv_iocu_view (a, b) values ('xyxyxy', 2) on conflict (a) do update set b = uv_iocu_view.b;
+-- error on trying to access view's column that's not present in underlying
+-- base relation in the ON CONFLICT portion of the query
+insert into uv_iocu_view (a, b) values ('xyxyxy', 3) on conflict (a) do update set b = excluded.b where excluded.c > 0;
+ERROR: cannot access column "c" of view "uv_iocu_view" in this part of query
+DETAIL: Only columns present in base relation can be used in ON CONFLICT specification
+explain (costs off) insert into uv_iocu_view (a, b) values ('xyxyxy', 3) on conflict (a) do update set b = excluded.b where excluded.b > 0;
+ QUERY PLAN
+---------------------------------------------------------
+ Insert on uv_iocu_tab
+ Conflict Resolution: UPDATE
+ Conflict Arbiter Indexes: uv_iocu_tab_a_key
+ Conflict Filter: (excluded.b > '0'::double precision)
+ -> Result
+(5 rows)
+
+insert into uv_iocu_view (a, b) values ('xyxyxy', 3) on conflict (a) do update set b = excluded.b where excluded.b > 0;
+-- should display 'xyxyxy, 3'
+select * from uv_iocu_tab;
+ a | b
+--------+---
+ xyxyxy | 3
+(1 row)
+
+drop view uv_iocu_view;
+drop table uv_iocu_tab;
diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql
index a7786b26e9..8dc0c6a8cc 100644
--- a/src/test/regress/sql/updatable_views.sql
+++ b/src/test/regress/sql/updatable_views.sql
@@ -1244,3 +1244,23 @@ insert into wcowrtest_v2 values (2, 'no such row in sometable');
drop view wcowrtest_v, wcowrtest_v2;
drop table wcowrtest, sometable;
+
+-- Check INSERT .. ON CONFLICT DO UPDATE works correctly when the view's
+-- columns may be ordered differently than the underlying table's.
+create table uv_iocu_tab (a text unique, b float);
+insert into uv_iocu_tab values ('xyxyxy', 1);
+-- note the different order of columns in the view
+create view uv_iocu_view as select b, b+1 as c, a from uv_iocu_tab;
+insert into uv_iocu_view (a, b) values ('xyxyxy', 2) on conflict (a) do update set b = uv_iocu_view.b;
+
+-- error on trying to access view's column that's not present in underlying
+-- base relation in the ON CONFLICT portion of the query
+insert into uv_iocu_view (a, b) values ('xyxyxy', 3) on conflict (a) do update set b = excluded.b where excluded.c > 0;
+
+explain (costs off) insert into uv_iocu_view (a, b) values ('xyxyxy', 3) on conflict (a) do update set b = excluded.b where excluded.b > 0;
+
+insert into uv_iocu_view (a, b) values ('xyxyxy', 3) on conflict (a) do update set b = excluded.b where excluded.b > 0;
+-- should display 'xyxyxy, 3'
+select * from uv_iocu_tab;
+drop view uv_iocu_view;
+drop table uv_iocu_tab;
--
2.11.0
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-02 10:38 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
@ 2018-08-03 06:52 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 09:40 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
0 siblings, 1 reply; 30+ messages in thread
From: Dean Rasheed @ 2018-08-03 06:52 UTC (permalink / raw)
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Mario De Frutos Dieguez <mariodefrutos@gmail.com>; pgsql-bugs@lists.postgresql.org
On 2 August 2018 at 11:38, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> Attaching the updated patch, which is quite heavily revised from the
> earlier patch, given the above findings.
>
This doesn't look right to me. It breaks the following case which
currently works in HEAD:
--
drop table if exists foo cascade;
create table foo (a int unique, b text);
create view foo_view as select a as aa, b as bb from foo;
insert into foo_view (aa,bb) values (1,'x');
insert into foo_view (aa,bb) values (1,'y')
on conflict (aa) do update set bb = excluded.bb;
select * from foo_view;
--
I also don't see why it should reject columns from the view that
aren't in the base relation. Such columns need to remain unchanged in
the UPDATE because they're non-updatable, but they're still logically
part of the new excluded view tuple, and so it may still be useful to
refer to them in other parts of the auxiliary UPDATE.
Regards,
Dean
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-02 10:38 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 06:52 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
@ 2018-08-03 09:40 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 10:17 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 18:46 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
2018-08-03 23:34 ` Re: Fwd: Problem with a "complex" upsert Peter Geoghegan <pg@bowt.ie>
2018-08-03 23:51 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
0 siblings, 4 replies; 30+ messages in thread
From: Dean Rasheed @ 2018-08-03 09:40 UTC (permalink / raw)
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Mario De Frutos Dieguez <mariodefrutos@gmail.com>; pgsql-bugs@lists.postgresql.org
On 3 August 2018 at 07:52, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> This doesn't look right to me. It breaks the following case ...
Here's an updated patch that fixes this.
> I also don't see why it should reject columns from the view that
> aren't in the base relation.
This patch also allows access to view columns that aren't in the
underlying base relation. The rationale for the result in the new test
case where it attempts to insert (1,'y') into columns (aa,bb) of the
view is that the new view row that would have resulted if the insert
had succeeded is ('y',1,(1,'y')), hence that's what excluded.* should
be for the view in the "on conflict" action, and there should be no
problem referring to any part of that excluded view tuple.
Regards,
Dean
Attachments:
insert-on-conflict-on-updatable-view-v3.patch (text/x-patch, 14.7K) download
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
new file mode 100644
index 05f5759..87e084b
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -1022,9 +1022,6 @@ transformOnConflictClause(ParseState *ps
if (onConflictClause->action == ONCONFLICT_UPDATE)
{
Relation targetrel = pstate->p_target_relation;
- Var *var;
- TargetEntry *te;
- int attno;
/*
* All INSERT expressions have been parsed, get ready for potentially
@@ -1043,56 +1040,8 @@ transformOnConflictClause(ParseState *ps
false, false);
exclRte->relkind = RELKIND_COMPOSITE_TYPE;
exclRelIndex = list_length(pstate->p_rtable);
-
- /*
- * Build a targetlist representing the columns of the EXCLUDED pseudo
- * relation. Have to be careful to use resnos that correspond to
- * attnos of the underlying relation.
- */
- for (attno = 0; attno < RelationGetNumberOfAttributes(targetrel); attno++)
- {
- Form_pg_attribute attr = TupleDescAttr(targetrel->rd_att, attno);
- char *name;
-
- if (attr->attisdropped)
- {
- /*
- * can't use atttypid here, but it doesn't really matter what
- * type the Const claims to be.
- */
- var = (Var *) makeNullConst(INT4OID, -1, InvalidOid);
- name = "";
- }
- else
- {
- var = makeVar(exclRelIndex, attno + 1,
- attr->atttypid, attr->atttypmod,
- attr->attcollation,
- 0);
- name = pstrdup(NameStr(attr->attname));
- }
-
- te = makeTargetEntry((Expr *) var,
- attno + 1,
- name,
- false);
-
- /* don't require select access yet */
- exclRelTlist = lappend(exclRelTlist, te);
- }
-
- /*
- * Add a whole-row-Var entry to support references to "EXCLUDED.*".
- * Like the other entries in exclRelTlist, its resno must match the
- * Var's varattno, else the wrong things happen while resolving
- * references in setrefs.c. This is against normal conventions for
- * targetlists, but it's okay since we don't use this as a real tlist.
- */
- var = makeVar(exclRelIndex, InvalidAttrNumber,
- targetrel->rd_rel->reltype,
- -1, InvalidOid, 0);
- te = makeTargetEntry((Expr *) var, InvalidAttrNumber, NULL, true);
- exclRelTlist = lappend(exclRelTlist, te);
+ exclRelTlist = BuildOnConflictExcludedTargetlist(targetrel,
+ exclRelIndex);
/*
* Add EXCLUDED and the target RTE to the namespace, so that they can
@@ -1124,6 +1073,75 @@ transformOnConflictClause(ParseState *ps
return result;
}
+
+
+/*
+ * BuildOnConflictExcludedTargetlist
+ * Create the target list of EXCLUDED pseudo-relation of ON CONFLICT
+ *
+ * Note: Exported for use in the rewriter.
+ */
+List *
+BuildOnConflictExcludedTargetlist(Relation targetrel,
+ Index exclRelIndex)
+{
+ List *result = NIL;
+ int attno;
+ Var *var;
+ TargetEntry *te;
+
+ /*
+ * Build a targetlist representing the columns of the EXCLUDED pseudo
+ * relation. Have to be careful to use resnos that correspond to attnos
+ * of the underlying relation.
+ */
+ for (attno = 0; attno < RelationGetNumberOfAttributes(targetrel); attno++)
+ {
+ Form_pg_attribute attr = TupleDescAttr(targetrel->rd_att, attno);
+ char *name;
+
+ if (attr->attisdropped)
+ {
+ /*
+ * can't use atttypid here, but it doesn't really matter what type
+ * the Const claims to be.
+ */
+ var = (Var *) makeNullConst(INT4OID, -1, InvalidOid);
+ name = "";
+ }
+ else
+ {
+ var = makeVar(exclRelIndex, attno + 1,
+ attr->atttypid, attr->atttypmod,
+ attr->attcollation,
+ 0);
+ name = pstrdup(NameStr(attr->attname));
+ }
+
+ te = makeTargetEntry((Expr *) var,
+ attno + 1,
+ name,
+ false);
+
+ /* don't require select access yet */
+ result = lappend(result, te);
+ }
+
+ /*
+ * Add a whole-row-Var entry to support references to "EXCLUDED.*". Like
+ * the other entries in exclRelTlist, its resno must match the Var's
+ * varattno, else the wrong things happen while resolving references in
+ * setrefs.c. This is against normal conventions for targetlists, but
+ * it's okay since we don't use this as a real tlist.
+ */
+ var = makeVar(exclRelIndex, InvalidAttrNumber,
+ targetrel->rd_rel->reltype,
+ -1, InvalidOid, 0);
+ te = makeTargetEntry((Expr *) var, InvalidAttrNumber, NULL, true);
+ result = lappend(result, te);
+
+ return result;
+}
/*
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
new file mode 100644
index 5b87c55..345d69b
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -29,6 +29,7 @@
#include "nodes/nodeFuncs.h"
#include "parser/analyze.h"
#include "parser/parse_coerce.h"
+#include "parser/parse_relation.h"
#include "parser/parsetree.h"
#include "rewrite/rewriteDefine.h"
#include "rewrite/rewriteHandler.h"
@@ -2875,8 +2876,6 @@ rewriteTargetView(Query *parsetree, Rela
*/
base_rte->relkind = base_rel->rd_rel->relkind;
- heap_close(base_rel, NoLock);
-
/*
* If the view query contains any sublink subqueries then we need to also
* acquire locks on any relations they refer to. We know that there won't
@@ -3035,6 +3034,81 @@ rewriteTargetView(Query *parsetree, Rela
}
/*
+ * Similarly for INSERT .. ON CONFLICT .. DO UPDATE, update the resnos in
+ * the auxiliary UPDATE's targetlist to refer to columns of the base
+ * relation.
+ */
+ if (parsetree->onConflict &&
+ parsetree->onConflict->action == ONCONFLICT_UPDATE)
+ {
+ Index old_exclRelIndex,
+ new_exclRelIndex;
+ RangeTblEntry *new_exclRte;
+ List *tmp_tlist;
+
+ foreach(lc, parsetree->onConflict->onConflictSet)
+ {
+ TargetEntry *tle = (TargetEntry *) lfirst(lc);
+ TargetEntry *view_tle;
+
+ if (tle->resjunk)
+ continue;
+
+ view_tle = get_tle_by_resno(view_targetlist, tle->resno);
+ if (view_tle != NULL && !view_tle->resjunk && IsA(view_tle->expr, Var))
+ tle->resno = ((Var *) view_tle->expr)->varattno;
+ else
+ elog(ERROR, "attribute number %d not found in view targetlist",
+ tle->resno);
+ }
+
+ /*
+ * Create a new RTE for the EXCLUDED pseudo-relation using base_rel.
+ * This closely mimics the code in transformOnConflictClause.
+ */
+ old_exclRelIndex = parsetree->onConflict->exclRelIndex;
+
+ new_exclRte = addRangeTableEntryForRelation(make_parsestate(NULL),
+ base_rel,
+ makeAlias("excluded",
+ NIL),
+ false, false);
+ new_exclRte->relkind = RELKIND_COMPOSITE_TYPE;
+ parsetree->rtable = lappend(parsetree->rtable, new_exclRte);
+ new_exclRelIndex = parsetree->onConflict->exclRelIndex =
+ list_length(parsetree->rtable);
+
+ /*
+ * Replace the targetlist for the old EXCLUDED pseudo-relation with a
+ * new one using the columns from the base relation.
+ */
+ parsetree->onConflict->exclRelTlist =
+ BuildOnConflictExcludedTargetlist(base_rel, new_exclRelIndex);
+
+ /*
+ * Update all Vars in the ON CONFLICT clause that refer to the old
+ * EXCLUDED pseudo-relation using the column mappings defined in the
+ * view targetlist. We do this using a modified copy of the view
+ * targetlist, that refers to the new EXCLUDED pseudo-relation rather
+ * than the new target RTE.
+ */
+ tmp_tlist = copyObject(view_targetlist);
+
+ ChangeVarNodes((Node *) tmp_tlist, new_rt_index,
+ new_exclRelIndex, 0);
+
+ parsetree->onConflict = (OnConflictExpr *)
+ ReplaceVarsFromTargetList((Node *) parsetree->onConflict,
+ old_exclRelIndex,
+ 0,
+ view_rte,
+ tmp_tlist,
+ REPLACEVARS_REPORT_ERROR,
+ 0,
+ &parsetree->hasSubLinks);
+ }
+
+ /*
* For UPDATE/DELETE, pull up any WHERE quals from the view. We know that
* any Vars in the quals must reference the one base relation, so we need
* only adjust their varnos to reference the new target (just the same as
@@ -3161,6 +3235,8 @@ rewriteTargetView(Query *parsetree, Rela
}
}
+ heap_close(base_rel, NoLock);
+
return parsetree;
}
diff --git a/src/include/parser/analyze.h b/src/include/parser/analyze.h
new file mode 100644
index 687ae1b..7b5b90c
--- a/src/include/parser/analyze.h
+++ b/src/include/parser/analyze.h
@@ -43,4 +43,7 @@ extern void applyLockingClause(Query *qr
LockClauseStrength strength,
LockWaitPolicy waitPolicy, bool pushedDown);
+extern List *BuildOnConflictExcludedTargetlist(Relation targetrel,
+ Index exclRelIndex);
+
#endif /* ANALYZE_H */
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
new file mode 100644
index b34bab4..4726c67
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -2578,3 +2578,84 @@ ERROR: new row violates check option fo
DETAIL: Failing row contains (2, no such row in sometable).
drop view wcowrtest_v, wcowrtest_v2;
drop table wcowrtest, sometable;
+-- Check INSERT .. ON CONFLICT DO UPDATE works correctly when the view's
+-- columns are named and ordered differently than the underlying table's.
+create table uv_iocu_tab (a text unique, b float);
+insert into uv_iocu_tab values ('xyxyxy', 1);
+create view uv_iocu_view as select b, b+1 as c, a from uv_iocu_tab;
+insert into uv_iocu_view (a, b) values ('xyxyxy', 2)
+ on conflict (a) do update set b = uv_iocu_view.b;
+-- OK to access view columns that are not present in underlying base
+-- relation in the ON CONFLICT portion of the query
+explain (costs off)
+insert into uv_iocu_view (a, b) values ('xyxyxy', 3)
+ on conflict (a) do update set b = excluded.b where excluded.c > 0;
+ QUERY PLAN
+-----------------------------------------------------------------------------------
+ Insert on uv_iocu_tab
+ Conflict Resolution: UPDATE
+ Conflict Arbiter Indexes: uv_iocu_tab_a_key
+ Conflict Filter: ((excluded.b + '1'::double precision) > '0'::double precision)
+ -> Result
+(5 rows)
+
+insert into uv_iocu_view (a, b) values ('xyxyxy', 3)
+ on conflict (a) do update set b = excluded.b where excluded.c > 0;
+-- should display 'xyxyxy, 3'
+select * from uv_iocu_tab;
+ a | b
+--------+---
+ xyxyxy | 3
+(1 row)
+
+drop view uv_iocu_view;
+drop table uv_iocu_tab;
+-- Example with whole-row references to the view
+create table uv_iocu_tab (a int unique, b text);
+create view uv_iocu_view as
+ select b as bb, a as aa, uv_iocu_tab::text as cc from uv_iocu_tab;
+insert into uv_iocu_view (aa,bb) values (1,'x');
+explain (costs off)
+insert into uv_iocu_view (aa,bb) values (1,'y')
+ on conflict (aa) do update set bb = 'Rejected: '||excluded.*
+ where excluded.aa > 0
+ and excluded.bb != ''
+ and excluded.cc is not null;
+ QUERY PLAN
+---------------------------------------------------------------------------------------------------------
+ Insert on uv_iocu_tab
+ Conflict Resolution: UPDATE
+ Conflict Arbiter Indexes: uv_iocu_tab_a_key
+ Conflict Filter: ((excluded.a > 0) AND (excluded.b <> ''::text) AND ((excluded.*)::text IS NOT NULL))
+ -> Result
+(5 rows)
+
+insert into uv_iocu_view (aa,bb) values (1,'y')
+ on conflict (aa) do update set bb = 'Rejected: '||excluded.*
+ where excluded.aa > 0
+ and excluded.bb != ''
+ and excluded.cc is not null;
+select * from uv_iocu_view;
+ bb | aa | cc
+-------------------------+----+---------------------------------
+ Rejected: (y,1,"(1,y)") | 1 | (1,"Rejected: (y,1,""(1,y)"")")
+(1 row)
+
+-- Test omiting a column of the base relation
+delete from uv_iocu_view;
+insert into uv_iocu_view (aa,bb) values (1,'x');
+insert into uv_iocu_view (aa) values (1)
+ on conflict (aa) do update set bb = 'Rejected: '||excluded.*;
+select * from uv_iocu_view;
+ bb | aa | cc
+-----------------------+----+-------------------------------
+ Rejected: (,1,"(1,)") | 1 | (1,"Rejected: (,1,""(1,)"")")
+(1 row)
+
+-- Should fail to update non-updatable columns
+insert into uv_iocu_view (aa) values (1)
+ on conflict (aa) do update set cc = 'XXX';
+ERROR: cannot insert into column "cc" of view "uv_iocu_view"
+DETAIL: View columns that are not columns of their base relation are not updatable.
+drop view uv_iocu_view;
+drop table uv_iocu_tab;
diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql
new file mode 100644
index a7786b2..6e92f33
--- a/src/test/regress/sql/updatable_views.sql
+++ b/src/test/regress/sql/updatable_views.sql
@@ -1244,3 +1244,57 @@ insert into wcowrtest_v2 values (2, 'no
drop view wcowrtest_v, wcowrtest_v2;
drop table wcowrtest, sometable;
+
+-- Check INSERT .. ON CONFLICT DO UPDATE works correctly when the view's
+-- columns are named and ordered differently than the underlying table's.
+create table uv_iocu_tab (a text unique, b float);
+insert into uv_iocu_tab values ('xyxyxy', 1);
+create view uv_iocu_view as select b, b+1 as c, a from uv_iocu_tab;
+insert into uv_iocu_view (a, b) values ('xyxyxy', 2)
+ on conflict (a) do update set b = uv_iocu_view.b;
+
+-- OK to access view columns that are not present in underlying base
+-- relation in the ON CONFLICT portion of the query
+explain (costs off)
+insert into uv_iocu_view (a, b) values ('xyxyxy', 3)
+ on conflict (a) do update set b = excluded.b where excluded.c > 0;
+
+insert into uv_iocu_view (a, b) values ('xyxyxy', 3)
+ on conflict (a) do update set b = excluded.b where excluded.c > 0;
+-- should display 'xyxyxy, 3'
+select * from uv_iocu_tab;
+drop view uv_iocu_view;
+drop table uv_iocu_tab;
+
+-- Example with whole-row references to the view
+create table uv_iocu_tab (a int unique, b text);
+create view uv_iocu_view as
+ select b as bb, a as aa, uv_iocu_tab::text as cc from uv_iocu_tab;
+
+insert into uv_iocu_view (aa,bb) values (1,'x');
+explain (costs off)
+insert into uv_iocu_view (aa,bb) values (1,'y')
+ on conflict (aa) do update set bb = 'Rejected: '||excluded.*
+ where excluded.aa > 0
+ and excluded.bb != ''
+ and excluded.cc is not null;
+insert into uv_iocu_view (aa,bb) values (1,'y')
+ on conflict (aa) do update set bb = 'Rejected: '||excluded.*
+ where excluded.aa > 0
+ and excluded.bb != ''
+ and excluded.cc is not null;
+select * from uv_iocu_view;
+
+-- Test omiting a column of the base relation
+delete from uv_iocu_view;
+insert into uv_iocu_view (aa,bb) values (1,'x');
+insert into uv_iocu_view (aa) values (1)
+ on conflict (aa) do update set bb = 'Rejected: '||excluded.*;
+select * from uv_iocu_view;
+
+-- Should fail to update non-updatable columns
+insert into uv_iocu_view (aa) values (1)
+ on conflict (aa) do update set cc = 'XXX';
+
+drop view uv_iocu_view;
+drop table uv_iocu_tab;
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-02 10:38 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 06:52 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 09:40 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
@ 2018-08-03 10:17 ` Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
3 siblings, 0 replies; 30+ messages in thread
From: Amit Langote @ 2018-08-03 10:17 UTC (permalink / raw)
To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Mario De Frutos Dieguez <mariodefrutos@gmail.com>; pgsql-bugs@lists.postgresql.org
Thanks Dean for taking a look.
On 2018/08/03 18:40, Dean Rasheed wrote:
> On 3 August 2018 at 07:52, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>> This doesn't look right to me. It breaks the following case ...
Hmm yeah, matching view and base relation names like that was silly.
> Here's an updated patch that fixes this.
>
>> I also don't see why it should reject columns from the view that
>> aren't in the base relation.
>
> This patch also allows access to view columns that aren't in the
> underlying base relation. The rationale for the result in the new test
> case where it attempts to insert (1,'y') into columns (aa,bb) of the
> view is that the new view row that would have resulted if the insert
> had succeeded is ('y',1,(1,'y')), hence that's what excluded.* should
> be for the view in the "on conflict" action, and there should be no
> problem referring to any part of that excluded view tuple.
Ah, I see what you did there with converting EXCLUDED column references.
I had tried to do the coversion from view attnos to base rel attnos using
tupconvert.c. When fiddling with that, I had to install that restriction
of not allowing accessing view's own columns via EXCLUDED, *because of*
trying to convert using tupconvert.c. Somehow, I had also became
convinced that restricting it like that made sense semantically, which as
you've shown, it doesn't.
After seeing your first email, I had started replacing the tupconvert.c
based conversion (which wouldn't even be readily backpatchable to 9.5) to
ReplaceVarsFromTargetList one, but you beat me to it.
Your updated version looks good and also nice that it has more tests. One
thing that stood out to me was how column references via EXCLUDED now
refer to base rel column names, but I guess that's fine.
create table foo (a int unique, b int);
create view foo_view as select b as bb, a + 1 as cc, a as aa from foo;
explain insert into foo_view (aa, bb) select 1, 1 on conflict (aa) do
update set bb = excluded.bb where excluded.cc > 1;
QUERY PLAN
─────────────────────────────────────────────────
Insert on foo (cost=0.00..0.01 rows=1 width=8)
Conflict Resolution: UPDATE
Conflict Arbiter Indexes: foo_a_key
Conflict Filter: ((excluded.a + 1) > 1)
-> Result (cost=0.00..0.01 rows=1 width=8)
(5 rows)
explain insert into foo_view (aa, bb) select 1, 1 on conflict (aa) do
update set bb = excluded.bb where excluded.aa > 1;
QUERY PLAN
─────────────────────────────────────────────────
Insert on foo (cost=0.00..0.01 rows=1 width=8)
Conflict Resolution: UPDATE
Conflict Arbiter Indexes: foo_a_key
Conflict Filter: (excluded.a > 1)
-> Result (cost=0.00..0.01 rows=1 width=8)
(5 rows)
Thanks again.
Regards,
Amit
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-02 10:38 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 06:52 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 09:40 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
@ 2018-08-03 18:46 ` Andres Freund <andres@anarazel.de>
2018-08-03 21:26 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
3 siblings, 1 reply; 30+ messages in thread
From: Andres Freund @ 2018-08-03 18:46 UTC (permalink / raw)
To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; Tom Lane <tgl@sss.pgh.pa.us>; Mario De Frutos Dieguez <mariodefrutos@gmail.com>; pgsql-bugs@lists.postgresql.org
Hi Dean,
On 2018-08-03 10:40:50 +0100, Dean Rasheed wrote:
> On 3 August 2018 at 07:52, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> > This doesn't look right to me. It breaks the following case ...
>
> Here's an updated patch that fixes this.
Are you planning to push a version of this soon? It'd be good to
get this included in the next set of releases...
- Andres
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-02 10:38 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 06:52 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 09:40 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 18:46 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
@ 2018-08-03 21:26 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 22:32 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 30+ messages in thread
From: Dean Rasheed @ 2018-08-03 21:26 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; Tom Lane <tgl@sss.pgh.pa.us>; Mario De Frutos Dieguez <mariodefrutos@gmail.com>; pgsql-bugs@lists.postgresql.org
On 3 August 2018 at 19:46, Andres Freund <andres@anarazel.de> wrote:
> Are you planning to push a version of this soon? It'd be good to
> get this included in the next set of releases...
>
Yes, agreed. I'll try to do it this weekend.
Regards,
Dean
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-02 10:38 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 06:52 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 09:40 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 18:46 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
2018-08-03 21:26 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
@ 2018-08-03 22:32 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-08-03 22:44 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 30+ messages in thread
From: Tom Lane @ 2018-08-03 22:32 UTC (permalink / raw)
To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; Mario De Frutos Dieguez <mariodefrutos@gmail.com>; pgsql-bugs@lists.postgresql.org
Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> On 3 August 2018 at 19:46, Andres Freund <andres@anarazel.de> wrote:
>> Are you planning to push a version of this soon? It'd be good to
>> get this included in the next set of releases...
> Yes, agreed. I'll try to do it this weekend.
Keep in mind that we are hard up against the release deadline.
This is a bad weekend to be pushing anything you are not 100.00%
sure of; the later, the more so, as by Sunday you will probably not
get a complete set of buildfarm reports before the wrap happens.
Balance the risks of shipping a broken release vs. waiting one
more quarter to ship the fix. After a quick look at the size
of the patch, my own inclination if I were the committer would
be to wait till after the releases are out. Or you might
consider pushing only to 11+HEAD, with the expectation of
back-patching later after we've gotten some beta results.
regards, tom lane
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-02 10:38 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 06:52 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 09:40 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 18:46 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
2018-08-03 21:26 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 22:32 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
@ 2018-08-03 22:44 ` Andres Freund <andres@anarazel.de>
2018-08-03 22:58 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-03 23:14 ` Re: Fwd: Problem with a "complex" upsert Peter Geoghegan <pg@bowt.ie>
0 siblings, 2 replies; 30+ messages in thread
From: Andres Freund @ 2018-08-03 22:44 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Dean Rasheed <dean.a.rasheed@gmail.com>; Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; Mario De Frutos Dieguez <mariodefrutos@gmail.com>; pgsql-bugs@lists.postgresql.org
On 2018-08-03 18:32:57 -0400, Tom Lane wrote:
> Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> > On 3 August 2018 at 19:46, Andres Freund <andres@anarazel.de> wrote:
> >> Are you planning to push a version of this soon? It'd be good to
> >> get this included in the next set of releases...
>
> > Yes, agreed. I'll try to do it this weekend.
>
> Keep in mind that we are hard up against the release deadline.
Right.
> This is a bad weekend to be pushing anything you are not 100.00%
> sure of; the later, the more so, as by Sunday you will probably not
> get a complete set of buildfarm reports before the wrap happens.
>
> Balance the risks of shipping a broken release vs. waiting one
> more quarter to ship the fix. After a quick look at the size
> of the patch, my own inclination if I were the committer would
> be to wait till after the releases are out. Or you might
> consider pushing only to 11+HEAD, with the expectation of
> back-patching later after we've gotten some beta results.
This results in clearly inserting wrong data and/or crashing the
server. And it's not a huge effect outside of already broken scenarios.
I think we definitely should try to get this in.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-02 10:38 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 06:52 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 09:40 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 18:46 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
2018-08-03 21:26 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 22:32 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-03 22:44 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
@ 2018-08-03 22:58 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-08-04 22:37 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 30+ messages in thread
From: Tom Lane @ 2018-08-03 22:58 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Dean Rasheed <dean.a.rasheed@gmail.com>; Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; Mario De Frutos Dieguez <mariodefrutos@gmail.com>; pgsql-bugs@lists.postgresql.org
Andres Freund <andres@anarazel.de> writes:
> On 2018-08-03 18:32:57 -0400, Tom Lane wrote:
>> Balance the risks of shipping a broken release vs. waiting one
>> more quarter to ship the fix. After a quick look at the size
>> of the patch, my own inclination if I were the committer would
>> be to wait till after the releases are out. Or you might
>> consider pushing only to 11+HEAD, with the expectation of
>> back-patching later after we've gotten some beta results.
> This results in clearly inserting wrong data and/or crashing the
> server. And it's not a huge effect outside of already broken scenarios.
> I think we definitely should try to get this in.
Well, if you're excited about it, help Dean review it. My own feeling
is that this case has been broken for several years with no one noticing,
so it can't be all that critical.
regards, tom lane
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-02 10:38 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 06:52 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 09:40 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 18:46 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
2018-08-03 21:26 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 22:32 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-03 22:44 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
2018-08-03 22:58 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
@ 2018-08-04 22:37 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-08-06 16:41 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-07 00:50 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
0 siblings, 2 replies; 30+ messages in thread
From: Tom Lane @ 2018-08-04 22:37 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Dean Rasheed <dean.a.rasheed@gmail.com>; Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; Mario De Frutos Dieguez <mariodefrutos@gmail.com>; pgsql-bugs@lists.postgresql.org
I wrote:
> Andres Freund <andres@anarazel.de> writes:
>> I think we definitely should try to get this in.
> Well, if you're excited about it, help Dean review it.
So, in the spirit of "put your money where your mouth is", I've been
working off-list with Dean today to review this patch. We found a couple
additional minor issues:
* We noticed that the rewriter was expanding the EXCLUDED
pseudo-relation's RTE into an RTE_SUBQUERY RTE, rather than leaving it
in the intended form as an RTE_RELATION RTE with a nonstandard rtekind.
(This happens basically always with a view target rel in the existing
code, but only in corner cases after Dean's patch.) While this doesn't
seem to have obvious bad effects, it's both unintended and a waste of
cycles. Also, the fact that this area seems rather undertested leaves
me not wanting to have weird data structure differences that happen
only in corner cases. So we added a check to fireRIRrules to prevent
that from happening.
* We happened across some unexpected permissions failures while checking
the patch in cases where the calling user is not the view owner. This
turned out to be because of a pre-existing oversight: the replacement
EXCLUDED RTE is initially manufactured with requiredPerms = ACL_SELECT,
and nothing was getting done to change that, leading to failure if the
calling user doesn't have SELECT on the underlying table. (The desired
behavior is that the view owner needs permissions on the underlying table,
but the calling user only needs permissions on the view.) So that's
easily fixed by zeroing out requiredPerms in the EXCLUDED RTE; nothing is
lost because other code was already adding the required permission check
flags to the query's actual target relation. But out of paranoia we added
a bunch of permissions-checking test cases to updatable_views.sql.
Attached is our finished patch against HEAD. This is pretty much all
Dean's work, but I'm posting it on his behalf because it's late in the UK
and he's gone offline for the day. In the interests of getting a
full set of buildfarm testing on the patch before Monday's wrap deadline,
I'm going to finish up back-porting the patch and push it tonight.
regards, tom lane
Attachments:
insert-on-conflict-on-updatable-view-v4.patch (text/x-diff, 26.1K) download
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 05f5759..c601b6d 100644
*** a/src/backend/parser/analyze.c
--- b/src/backend/parser/analyze.c
*************** transformOnConflictClause(ParseState *ps
*** 1022,1030 ****
if (onConflictClause->action == ONCONFLICT_UPDATE)
{
Relation targetrel = pstate->p_target_relation;
- Var *var;
- TargetEntry *te;
- int attno;
/*
* All INSERT expressions have been parsed, get ready for potentially
--- 1022,1027 ----
*************** transformOnConflictClause(ParseState *ps
*** 1033,1107 ****
pstate->p_is_insert = false;
/*
! * Add range table entry for the EXCLUDED pseudo relation; relkind is
* set to composite to signal that we're not dealing with an actual
! * relation.
*/
exclRte = addRangeTableEntryForRelation(pstate,
targetrel,
makeAlias("excluded", NIL),
false, false);
exclRte->relkind = RELKIND_COMPOSITE_TYPE;
! exclRelIndex = list_length(pstate->p_rtable);
!
! /*
! * Build a targetlist representing the columns of the EXCLUDED pseudo
! * relation. Have to be careful to use resnos that correspond to
! * attnos of the underlying relation.
! */
! for (attno = 0; attno < RelationGetNumberOfAttributes(targetrel); attno++)
! {
! Form_pg_attribute attr = TupleDescAttr(targetrel->rd_att, attno);
! char *name;
!
! if (attr->attisdropped)
! {
! /*
! * can't use atttypid here, but it doesn't really matter what
! * type the Const claims to be.
! */
! var = (Var *) makeNullConst(INT4OID, -1, InvalidOid);
! name = "";
! }
! else
! {
! var = makeVar(exclRelIndex, attno + 1,
! attr->atttypid, attr->atttypmod,
! attr->attcollation,
! 0);
! name = pstrdup(NameStr(attr->attname));
! }
!
! te = makeTargetEntry((Expr *) var,
! attno + 1,
! name,
! false);
! /* don't require select access yet */
! exclRelTlist = lappend(exclRelTlist, te);
! }
! /*
! * Add a whole-row-Var entry to support references to "EXCLUDED.*".
! * Like the other entries in exclRelTlist, its resno must match the
! * Var's varattno, else the wrong things happen while resolving
! * references in setrefs.c. This is against normal conventions for
! * targetlists, but it's okay since we don't use this as a real tlist.
! */
! var = makeVar(exclRelIndex, InvalidAttrNumber,
! targetrel->rd_rel->reltype,
! -1, InvalidOid, 0);
! te = makeTargetEntry((Expr *) var, InvalidAttrNumber, NULL, true);
! exclRelTlist = lappend(exclRelTlist, te);
/*
* Add EXCLUDED and the target RTE to the namespace, so that they can
! * be used in the UPDATE statement.
*/
addRTEtoQuery(pstate, exclRte, false, true, true);
addRTEtoQuery(pstate, pstate->p_target_rangetblentry,
false, true, true);
onConflictSet =
transformUpdateTargetList(pstate, onConflictClause->targetList);
--- 1030,1065 ----
pstate->p_is_insert = false;
/*
! * Add range table entry for the EXCLUDED pseudo relation. relkind is
* set to composite to signal that we're not dealing with an actual
! * relation, and no permission checks are required on it. (We'll
! * check the actual target relation, instead.)
*/
exclRte = addRangeTableEntryForRelation(pstate,
targetrel,
makeAlias("excluded", NIL),
false, false);
exclRte->relkind = RELKIND_COMPOSITE_TYPE;
! exclRte->requiredPerms = 0;
! /* other permissions fields in exclRte are already empty */
! exclRelIndex = list_length(pstate->p_rtable);
! /* Create EXCLUDED rel's targetlist for use by EXPLAIN */
! exclRelTlist = BuildOnConflictExcludedTargetlist(targetrel,
! exclRelIndex);
/*
* Add EXCLUDED and the target RTE to the namespace, so that they can
! * be used in the UPDATE subexpressions.
*/
addRTEtoQuery(pstate, exclRte, false, true, true);
addRTEtoQuery(pstate, pstate->p_target_rangetblentry,
false, true, true);
+ /*
+ * Now transform the UPDATE subexpressions.
+ */
onConflictSet =
transformUpdateTargetList(pstate, onConflictClause->targetList);
*************** transformOnConflictClause(ParseState *ps
*** 1127,1132 ****
--- 1085,1158 ----
/*
+ * BuildOnConflictExcludedTargetlist
+ * Create target list for the EXCLUDED pseudo-relation of ON CONFLICT,
+ * representing the columns of targetrel with varno exclRelIndex.
+ *
+ * Note: Exported for use in the rewriter.
+ */
+ List *
+ BuildOnConflictExcludedTargetlist(Relation targetrel,
+ Index exclRelIndex)
+ {
+ List *result = NIL;
+ int attno;
+ Var *var;
+ TargetEntry *te;
+
+ /*
+ * Note that resnos of the tlist must correspond to attnos of the
+ * underlying relation, hence we need entries for dropped columns too.
+ */
+ for (attno = 0; attno < RelationGetNumberOfAttributes(targetrel); attno++)
+ {
+ Form_pg_attribute attr = TupleDescAttr(targetrel->rd_att, attno);
+ char *name;
+
+ if (attr->attisdropped)
+ {
+ /*
+ * can't use atttypid here, but it doesn't really matter what type
+ * the Const claims to be.
+ */
+ var = (Var *) makeNullConst(INT4OID, -1, InvalidOid);
+ name = "";
+ }
+ else
+ {
+ var = makeVar(exclRelIndex, attno + 1,
+ attr->atttypid, attr->atttypmod,
+ attr->attcollation,
+ 0);
+ name = pstrdup(NameStr(attr->attname));
+ }
+
+ te = makeTargetEntry((Expr *) var,
+ attno + 1,
+ name,
+ false);
+
+ result = lappend(result, te);
+ }
+
+ /*
+ * Add a whole-row-Var entry to support references to "EXCLUDED.*". Like
+ * the other entries in the EXCLUDED tlist, its resno must match the Var's
+ * varattno, else the wrong things happen while resolving references in
+ * setrefs.c. This is against normal conventions for targetlists, but
+ * it's okay since we don't use this as a real tlist.
+ */
+ var = makeVar(exclRelIndex, InvalidAttrNumber,
+ targetrel->rd_rel->reltype,
+ -1, InvalidOid, 0);
+ te = makeTargetEntry((Expr *) var, InvalidAttrNumber, NULL, true);
+ result = lappend(result, te);
+
+ return result;
+ }
+
+
+ /*
* count_rowexpr_columns -
* get number of columns contained in a ROW() expression;
* return -1 if expression isn't a RowExpr or a Var referencing one.
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 5b87c55..3123ee2 100644
*** a/src/backend/rewrite/rewriteHandler.c
--- b/src/backend/rewrite/rewriteHandler.c
***************
*** 29,34 ****
--- 29,35 ----
#include "nodes/nodeFuncs.h"
#include "parser/analyze.h"
#include "parser/parse_coerce.h"
+ #include "parser/parse_relation.h"
#include "parser/parsetree.h"
#include "rewrite/rewriteDefine.h"
#include "rewrite/rewriteHandler.h"
*************** fireRIRrules(Query *parsetree, List *act
*** 1771,1776 ****
--- 1772,1788 ----
continue;
/*
+ * In INSERT ... ON CONFLICT, ignore the EXCLUDED pseudo-relation;
+ * even if it points to a view, we needn't expand it, and should not
+ * because we want the RTE to remain of RTE_RELATION type. Otherwise,
+ * it would get changed to RTE_SUBQUERY type, which is an
+ * untested/unsupported situation.
+ */
+ if (parsetree->onConflict &&
+ rt_index == parsetree->onConflict->exclRelIndex)
+ continue;
+
+ /*
* If the table is not referenced in the query, then we ignore it.
* This prevents infinite expansion loop due to new rtable entries
* inserted by expansion of a rule. A table is referenced if it is
*************** rewriteTargetView(Query *parsetree, Rela
*** 2875,2882 ****
*/
base_rte->relkind = base_rel->rd_rel->relkind;
- heap_close(base_rel, NoLock);
-
/*
* If the view query contains any sublink subqueries then we need to also
* acquire locks on any relations they refer to. We know that there won't
--- 2887,2892 ----
*************** rewriteTargetView(Query *parsetree, Rela
*** 3035,3040 ****
--- 3045,3137 ----
}
/*
+ * For INSERT .. ON CONFLICT .. DO UPDATE, we must also update assorted
+ * stuff in the onConflict data structure.
+ */
+ if (parsetree->onConflict &&
+ parsetree->onConflict->action == ONCONFLICT_UPDATE)
+ {
+ Index old_exclRelIndex,
+ new_exclRelIndex;
+ RangeTblEntry *new_exclRte;
+ List *tmp_tlist;
+
+ /*
+ * Like the INSERT/UPDATE code above, update the resnos in the
+ * auxiliary UPDATE targetlist to refer to columns of the base
+ * relation.
+ */
+ foreach(lc, parsetree->onConflict->onConflictSet)
+ {
+ TargetEntry *tle = (TargetEntry *) lfirst(lc);
+ TargetEntry *view_tle;
+
+ if (tle->resjunk)
+ continue;
+
+ view_tle = get_tle_by_resno(view_targetlist, tle->resno);
+ if (view_tle != NULL && !view_tle->resjunk && IsA(view_tle->expr, Var))
+ tle->resno = ((Var *) view_tle->expr)->varattno;
+ else
+ elog(ERROR, "attribute number %d not found in view targetlist",
+ tle->resno);
+ }
+
+ /*
+ * Also, create a new RTE for the EXCLUDED pseudo-relation, using the
+ * query's new base rel (which may well have a different column list
+ * from the view, hence we need a new column alias list). This should
+ * match transformOnConflictClause. In particular, note that the
+ * relkind is set to composite to signal that we're not dealing with
+ * an actual relation, and no permissions checks are wanted.
+ */
+ old_exclRelIndex = parsetree->onConflict->exclRelIndex;
+
+ new_exclRte = addRangeTableEntryForRelation(make_parsestate(NULL),
+ base_rel,
+ makeAlias("excluded",
+ NIL),
+ false, false);
+ new_exclRte->relkind = RELKIND_COMPOSITE_TYPE;
+ new_exclRte->requiredPerms = 0;
+ /* other permissions fields in new_exclRte are already empty */
+
+ parsetree->rtable = lappend(parsetree->rtable, new_exclRte);
+ new_exclRelIndex = parsetree->onConflict->exclRelIndex =
+ list_length(parsetree->rtable);
+
+ /*
+ * Replace the targetlist for the EXCLUDED pseudo-relation with a new
+ * one, representing the columns from the new base relation.
+ */
+ parsetree->onConflict->exclRelTlist =
+ BuildOnConflictExcludedTargetlist(base_rel, new_exclRelIndex);
+
+ /*
+ * Update all Vars in the ON CONFLICT clause that refer to the old
+ * EXCLUDED pseudo-relation. We want to use the column mappings
+ * defined in the view targetlist, but we need the outputs to refer to
+ * the new EXCLUDED pseudo-relation rather than the new target RTE.
+ * Also notice that "EXCLUDED.*" will be expanded using the view's
+ * rowtype, which seems correct.
+ */
+ tmp_tlist = copyObject(view_targetlist);
+
+ ChangeVarNodes((Node *) tmp_tlist, new_rt_index,
+ new_exclRelIndex, 0);
+
+ parsetree->onConflict = (OnConflictExpr *)
+ ReplaceVarsFromTargetList((Node *) parsetree->onConflict,
+ old_exclRelIndex,
+ 0,
+ view_rte,
+ tmp_tlist,
+ REPLACEVARS_REPORT_ERROR,
+ 0,
+ &parsetree->hasSubLinks);
+ }
+
+ /*
* For UPDATE/DELETE, pull up any WHERE quals from the view. We know that
* any Vars in the quals must reference the one base relation, so we need
* only adjust their varnos to reference the new target (just the same as
*************** rewriteTargetView(Query *parsetree, Rela
*** 3161,3166 ****
--- 3258,3265 ----
}
}
+ heap_close(base_rel, NoLock);
+
return parsetree;
}
diff --git a/src/include/parser/analyze.h b/src/include/parser/analyze.h
index 687ae1b..7b5b90c 100644
*** a/src/include/parser/analyze.h
--- b/src/include/parser/analyze.h
*************** extern void applyLockingClause(Query *qr
*** 43,46 ****
--- 43,49 ----
LockClauseStrength strength,
LockWaitPolicy waitPolicy, bool pushedDown);
+ extern List *BuildOnConflictExcludedTargetlist(Relation targetrel,
+ Index exclRelIndex);
+
#endif /* ANALYZE_H */
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index b34bab4..e64d693 100644
*** a/src/test/regress/expected/updatable_views.out
--- b/src/test/regress/expected/updatable_views.out
*************** ERROR: new row violates check option fo
*** 2578,2580 ****
--- 2578,2774 ----
DETAIL: Failing row contains (2, no such row in sometable).
drop view wcowrtest_v, wcowrtest_v2;
drop table wcowrtest, sometable;
+ -- Check INSERT .. ON CONFLICT DO UPDATE works correctly when the view's
+ -- columns are named and ordered differently than the underlying table's.
+ create table uv_iocu_tab (a text unique, b float);
+ insert into uv_iocu_tab values ('xyxyxy', 0);
+ create view uv_iocu_view as
+ select b, b+1 as c, a, '2.0'::text as two from uv_iocu_tab;
+ insert into uv_iocu_view (a, b) values ('xyxyxy', 1)
+ on conflict (a) do update set b = uv_iocu_view.b;
+ select * from uv_iocu_tab;
+ a | b
+ --------+---
+ xyxyxy | 0
+ (1 row)
+
+ insert into uv_iocu_view (a, b) values ('xyxyxy', 1)
+ on conflict (a) do update set b = excluded.b;
+ select * from uv_iocu_tab;
+ a | b
+ --------+---
+ xyxyxy | 1
+ (1 row)
+
+ -- OK to access view columns that are not present in underlying base
+ -- relation in the ON CONFLICT portion of the query
+ insert into uv_iocu_view (a, b) values ('xyxyxy', 3)
+ on conflict (a) do update set b = cast(excluded.two as float);
+ select * from uv_iocu_tab;
+ a | b
+ --------+---
+ xyxyxy | 2
+ (1 row)
+
+ explain (costs off)
+ insert into uv_iocu_view (a, b) values ('xyxyxy', 3)
+ on conflict (a) do update set b = excluded.b where excluded.c > 0;
+ QUERY PLAN
+ -----------------------------------------------------------------------------------
+ Insert on uv_iocu_tab
+ Conflict Resolution: UPDATE
+ Conflict Arbiter Indexes: uv_iocu_tab_a_key
+ Conflict Filter: ((excluded.b + '1'::double precision) > '0'::double precision)
+ -> Result
+ (5 rows)
+
+ insert into uv_iocu_view (a, b) values ('xyxyxy', 3)
+ on conflict (a) do update set b = excluded.b where excluded.c > 0;
+ select * from uv_iocu_tab;
+ a | b
+ --------+---
+ xyxyxy | 3
+ (1 row)
+
+ drop view uv_iocu_view;
+ drop table uv_iocu_tab;
+ -- Test whole-row references to the view
+ create table uv_iocu_tab (a int unique, b text);
+ create view uv_iocu_view as
+ select b as bb, a as aa, uv_iocu_tab::text as cc from uv_iocu_tab;
+ insert into uv_iocu_view (aa,bb) values (1,'x');
+ explain (costs off)
+ insert into uv_iocu_view (aa,bb) values (1,'y')
+ on conflict (aa) do update set bb = 'Rejected: '||excluded.*
+ where excluded.aa > 0
+ and excluded.bb != ''
+ and excluded.cc is not null;
+ QUERY PLAN
+ ---------------------------------------------------------------------------------------------------------
+ Insert on uv_iocu_tab
+ Conflict Resolution: UPDATE
+ Conflict Arbiter Indexes: uv_iocu_tab_a_key
+ Conflict Filter: ((excluded.a > 0) AND (excluded.b <> ''::text) AND ((excluded.*)::text IS NOT NULL))
+ -> Result
+ (5 rows)
+
+ insert into uv_iocu_view (aa,bb) values (1,'y')
+ on conflict (aa) do update set bb = 'Rejected: '||excluded.*
+ where excluded.aa > 0
+ and excluded.bb != ''
+ and excluded.cc is not null;
+ select * from uv_iocu_view;
+ bb | aa | cc
+ -------------------------+----+---------------------------------
+ Rejected: (y,1,"(1,y)") | 1 | (1,"Rejected: (y,1,""(1,y)"")")
+ (1 row)
+
+ -- Test omitting a column of the base relation
+ delete from uv_iocu_view;
+ insert into uv_iocu_view (aa,bb) values (1,'x');
+ insert into uv_iocu_view (aa) values (1)
+ on conflict (aa) do update set bb = 'Rejected: '||excluded.*;
+ select * from uv_iocu_view;
+ bb | aa | cc
+ -----------------------+----+-------------------------------
+ Rejected: (,1,"(1,)") | 1 | (1,"Rejected: (,1,""(1,)"")")
+ (1 row)
+
+ alter table uv_iocu_tab alter column b set default 'table default';
+ insert into uv_iocu_view (aa) values (1)
+ on conflict (aa) do update set bb = 'Rejected: '||excluded.*;
+ select * from uv_iocu_view;
+ bb | aa | cc
+ -------------------------------------------------------+----+---------------------------------------------------------------------
+ Rejected: ("table default",1,"(1,""table default"")") | 1 | (1,"Rejected: (""table default"",1,""(1,""""table default"""")"")")
+ (1 row)
+
+ alter view uv_iocu_view alter column bb set default 'view default';
+ insert into uv_iocu_view (aa) values (1)
+ on conflict (aa) do update set bb = 'Rejected: '||excluded.*;
+ select * from uv_iocu_view;
+ bb | aa | cc
+ -----------------------------------------------------+----+-------------------------------------------------------------------
+ Rejected: ("view default",1,"(1,""view default"")") | 1 | (1,"Rejected: (""view default"",1,""(1,""""view default"""")"")")
+ (1 row)
+
+ -- Should fail to update non-updatable columns
+ insert into uv_iocu_view (aa) values (1)
+ on conflict (aa) do update set cc = 'XXX';
+ ERROR: cannot insert into column "cc" of view "uv_iocu_view"
+ DETAIL: View columns that are not columns of their base relation are not updatable.
+ drop view uv_iocu_view;
+ drop table uv_iocu_tab;
+ -- ON CONFLICT DO UPDATE permissions checks
+ create user regress_view_user1;
+ create user regress_view_user2;
+ set session authorization regress_view_user1;
+ create table base_tbl(a int unique, b text, c float);
+ insert into base_tbl values (1,'xxx',1.0);
+ create view rw_view1 as select b as bb, c as cc, a as aa from base_tbl;
+ grant select (aa,bb) on rw_view1 to regress_view_user2;
+ grant insert on rw_view1 to regress_view_user2;
+ grant update (bb) on rw_view1 to regress_view_user2;
+ set session authorization regress_view_user2;
+ insert into rw_view1 values ('yyy',2.0,1)
+ on conflict (aa) do update set bb = excluded.cc; -- Not allowed
+ ERROR: permission denied for view rw_view1
+ insert into rw_view1 values ('yyy',2.0,1)
+ on conflict (aa) do update set bb = rw_view1.cc; -- Not allowed
+ ERROR: permission denied for view rw_view1
+ insert into rw_view1 values ('yyy',2.0,1)
+ on conflict (aa) do update set bb = excluded.bb; -- OK
+ insert into rw_view1 values ('zzz',2.0,1)
+ on conflict (aa) do update set bb = rw_view1.bb||'xxx'; -- OK
+ insert into rw_view1 values ('zzz',2.0,1)
+ on conflict (aa) do update set cc = 3.0; -- Not allowed
+ ERROR: permission denied for view rw_view1
+ reset session authorization;
+ select * from base_tbl;
+ a | b | c
+ ---+--------+---
+ 1 | yyyxxx | 1
+ (1 row)
+
+ set session authorization regress_view_user1;
+ grant select (a,b) on base_tbl to regress_view_user2;
+ grant insert (a,b) on base_tbl to regress_view_user2;
+ grant update (a,b) on base_tbl to regress_view_user2;
+ set session authorization regress_view_user2;
+ create view rw_view2 as select b as bb, c as cc, a as aa from base_tbl;
+ insert into rw_view2 (aa,bb) values (1,'xxx')
+ on conflict (aa) do update set bb = excluded.bb; -- Not allowed
+ ERROR: permission denied for table base_tbl
+ create view rw_view3 as select b as bb, a as aa from base_tbl;
+ insert into rw_view3 (aa,bb) values (1,'xxx')
+ on conflict (aa) do update set bb = excluded.bb; -- OK
+ reset session authorization;
+ select * from base_tbl;
+ a | b | c
+ ---+-----+---
+ 1 | xxx | 1
+ (1 row)
+
+ set session authorization regress_view_user2;
+ create view rw_view4 as select aa, bb, cc FROM rw_view1;
+ insert into rw_view4 (aa,bb) values (1,'yyy')
+ on conflict (aa) do update set bb = excluded.bb; -- Not allowed
+ ERROR: permission denied for view rw_view1
+ create view rw_view5 as select aa, bb FROM rw_view1;
+ insert into rw_view5 (aa,bb) values (1,'yyy')
+ on conflict (aa) do update set bb = excluded.bb; -- OK
+ reset session authorization;
+ select * from base_tbl;
+ a | b | c
+ ---+-----+---
+ 1 | yyy | 1
+ (1 row)
+
+ drop view rw_view5;
+ drop view rw_view4;
+ drop view rw_view3;
+ drop view rw_view2;
+ drop view rw_view1;
+ drop table base_tbl;
+ drop user regress_view_user1;
+ drop user regress_view_user2;
diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql
index a7786b2..dc6d5cb 100644
*** a/src/test/regress/sql/updatable_views.sql
--- b/src/test/regress/sql/updatable_views.sql
*************** insert into wcowrtest_v2 values (2, 'no
*** 1244,1246 ****
--- 1244,1381 ----
drop view wcowrtest_v, wcowrtest_v2;
drop table wcowrtest, sometable;
+
+ -- Check INSERT .. ON CONFLICT DO UPDATE works correctly when the view's
+ -- columns are named and ordered differently than the underlying table's.
+ create table uv_iocu_tab (a text unique, b float);
+ insert into uv_iocu_tab values ('xyxyxy', 0);
+ create view uv_iocu_view as
+ select b, b+1 as c, a, '2.0'::text as two from uv_iocu_tab;
+
+ insert into uv_iocu_view (a, b) values ('xyxyxy', 1)
+ on conflict (a) do update set b = uv_iocu_view.b;
+ select * from uv_iocu_tab;
+ insert into uv_iocu_view (a, b) values ('xyxyxy', 1)
+ on conflict (a) do update set b = excluded.b;
+ select * from uv_iocu_tab;
+
+ -- OK to access view columns that are not present in underlying base
+ -- relation in the ON CONFLICT portion of the query
+ insert into uv_iocu_view (a, b) values ('xyxyxy', 3)
+ on conflict (a) do update set b = cast(excluded.two as float);
+ select * from uv_iocu_tab;
+
+ explain (costs off)
+ insert into uv_iocu_view (a, b) values ('xyxyxy', 3)
+ on conflict (a) do update set b = excluded.b where excluded.c > 0;
+
+ insert into uv_iocu_view (a, b) values ('xyxyxy', 3)
+ on conflict (a) do update set b = excluded.b where excluded.c > 0;
+ select * from uv_iocu_tab;
+
+ drop view uv_iocu_view;
+ drop table uv_iocu_tab;
+
+ -- Test whole-row references to the view
+ create table uv_iocu_tab (a int unique, b text);
+ create view uv_iocu_view as
+ select b as bb, a as aa, uv_iocu_tab::text as cc from uv_iocu_tab;
+
+ insert into uv_iocu_view (aa,bb) values (1,'x');
+ explain (costs off)
+ insert into uv_iocu_view (aa,bb) values (1,'y')
+ on conflict (aa) do update set bb = 'Rejected: '||excluded.*
+ where excluded.aa > 0
+ and excluded.bb != ''
+ and excluded.cc is not null;
+ insert into uv_iocu_view (aa,bb) values (1,'y')
+ on conflict (aa) do update set bb = 'Rejected: '||excluded.*
+ where excluded.aa > 0
+ and excluded.bb != ''
+ and excluded.cc is not null;
+ select * from uv_iocu_view;
+
+ -- Test omitting a column of the base relation
+ delete from uv_iocu_view;
+ insert into uv_iocu_view (aa,bb) values (1,'x');
+ insert into uv_iocu_view (aa) values (1)
+ on conflict (aa) do update set bb = 'Rejected: '||excluded.*;
+ select * from uv_iocu_view;
+
+ alter table uv_iocu_tab alter column b set default 'table default';
+ insert into uv_iocu_view (aa) values (1)
+ on conflict (aa) do update set bb = 'Rejected: '||excluded.*;
+ select * from uv_iocu_view;
+
+ alter view uv_iocu_view alter column bb set default 'view default';
+ insert into uv_iocu_view (aa) values (1)
+ on conflict (aa) do update set bb = 'Rejected: '||excluded.*;
+ select * from uv_iocu_view;
+
+ -- Should fail to update non-updatable columns
+ insert into uv_iocu_view (aa) values (1)
+ on conflict (aa) do update set cc = 'XXX';
+
+ drop view uv_iocu_view;
+ drop table uv_iocu_tab;
+
+ -- ON CONFLICT DO UPDATE permissions checks
+ create user regress_view_user1;
+ create user regress_view_user2;
+
+ set session authorization regress_view_user1;
+ create table base_tbl(a int unique, b text, c float);
+ insert into base_tbl values (1,'xxx',1.0);
+ create view rw_view1 as select b as bb, c as cc, a as aa from base_tbl;
+
+ grant select (aa,bb) on rw_view1 to regress_view_user2;
+ grant insert on rw_view1 to regress_view_user2;
+ grant update (bb) on rw_view1 to regress_view_user2;
+
+ set session authorization regress_view_user2;
+ insert into rw_view1 values ('yyy',2.0,1)
+ on conflict (aa) do update set bb = excluded.cc; -- Not allowed
+ insert into rw_view1 values ('yyy',2.0,1)
+ on conflict (aa) do update set bb = rw_view1.cc; -- Not allowed
+ insert into rw_view1 values ('yyy',2.0,1)
+ on conflict (aa) do update set bb = excluded.bb; -- OK
+ insert into rw_view1 values ('zzz',2.0,1)
+ on conflict (aa) do update set bb = rw_view1.bb||'xxx'; -- OK
+ insert into rw_view1 values ('zzz',2.0,1)
+ on conflict (aa) do update set cc = 3.0; -- Not allowed
+ reset session authorization;
+ select * from base_tbl;
+
+ set session authorization regress_view_user1;
+ grant select (a,b) on base_tbl to regress_view_user2;
+ grant insert (a,b) on base_tbl to regress_view_user2;
+ grant update (a,b) on base_tbl to regress_view_user2;
+
+ set session authorization regress_view_user2;
+ create view rw_view2 as select b as bb, c as cc, a as aa from base_tbl;
+ insert into rw_view2 (aa,bb) values (1,'xxx')
+ on conflict (aa) do update set bb = excluded.bb; -- Not allowed
+ create view rw_view3 as select b as bb, a as aa from base_tbl;
+ insert into rw_view3 (aa,bb) values (1,'xxx')
+ on conflict (aa) do update set bb = excluded.bb; -- OK
+ reset session authorization;
+ select * from base_tbl;
+
+ set session authorization regress_view_user2;
+ create view rw_view4 as select aa, bb, cc FROM rw_view1;
+ insert into rw_view4 (aa,bb) values (1,'yyy')
+ on conflict (aa) do update set bb = excluded.bb; -- Not allowed
+ create view rw_view5 as select aa, bb FROM rw_view1;
+ insert into rw_view5 (aa,bb) values (1,'yyy')
+ on conflict (aa) do update set bb = excluded.bb; -- OK
+ reset session authorization;
+ select * from base_tbl;
+
+ drop view rw_view5;
+ drop view rw_view4;
+ drop view rw_view3;
+ drop view rw_view2;
+ drop view rw_view1;
+ drop table base_tbl;
+ drop user regress_view_user1;
+ drop user regress_view_user2;
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-02 10:38 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 06:52 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 09:40 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 18:46 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
2018-08-03 21:26 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 22:32 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-03 22:44 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
2018-08-03 22:58 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-04 22:37 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
@ 2018-08-06 16:41 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-08-06 16:48 ` Re: Fwd: Problem with a "complex" upsert Mario de Frutos Dieguez <mariodefrutos@gmail.com>
1 sibling, 1 reply; 30+ messages in thread
From: Tom Lane @ 2018-08-06 16:41 UTC (permalink / raw)
To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; Mario De Frutos Dieguez <mariodefrutos@gmail.com>; pgsql-bugs@lists.postgresql.org
I wrote:
> Attached is our finished patch against HEAD. This is pretty much all
> Dean's work, but I'm posting it on his behalf because it's late in the UK
> and he's gone offline for the day. In the interests of getting a
> full set of buildfarm testing on the patch before Monday's wrap deadline,
> I'm going to finish up back-porting the patch and push it tonight.
Final(?) note on this thread --- the security team realized over the
weekend that this bug constitutes a security issue, because you can do
more than crash the server. We don't normally consider simple crashes
as being CVE-worthy problems, but in this case, there's potential for
datatype confusion, which can be leveraged to allow disclosure of server
memory (as we've seen in other bugs before). We also realized that it's
possible to update a column you supposedly don't have privilege to update,
as long as there's some other column you do.
We've retroactively obtained a CVE number and will be describing this as
a security problem in the release notes.
regards, tom lane
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-02 10:38 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 06:52 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 09:40 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 18:46 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
2018-08-03 21:26 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 22:32 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-03 22:44 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
2018-08-03 22:58 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-04 22:37 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-06 16:41 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
@ 2018-08-06 16:48 ` Mario de Frutos Dieguez <mariodefrutos@gmail.com>
0 siblings, 0 replies; 30+ messages in thread
From: Mario de Frutos Dieguez @ 2018-08-06 16:48 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Dean Rasheed <dean.a.rasheed@gmail.com>; Andres Freund <andres@anarazel.de>; Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; pgsql-bugs@lists.postgresql.org
Wow glad to have discovered it by chance! Great news to have it fixed :))))
2018-08-06 18:41 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:
> I wrote:
>> Attached is our finished patch against HEAD. This is pretty much all
>> Dean's work, but I'm posting it on his behalf because it's late in the UK
>> and he's gone offline for the day. In the interests of getting a
>> full set of buildfarm testing on the patch before Monday's wrap deadline,
>> I'm going to finish up back-porting the patch and push it tonight.
>
> Final(?) note on this thread --- the security team realized over the
> weekend that this bug constitutes a security issue, because you can do
> more than crash the server. We don't normally consider simple crashes
> as being CVE-worthy problems, but in this case, there's potential for
> datatype confusion, which can be leveraged to allow disclosure of server
> memory (as we've seen in other bugs before). We also realized that it's
> possible to update a column you supposedly don't have privilege to update,
> as long as there's some other column you do.
>
> We've retroactively obtained a CVE number and will be describing this as
> a security problem in the release notes.
>
> regards, tom lane
>
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-02 10:38 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 06:52 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 09:40 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 18:46 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
2018-08-03 21:26 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 22:32 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-03 22:44 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
2018-08-03 22:58 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-04 22:37 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
@ 2018-08-07 00:50 ` Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-07 06:31 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
1 sibling, 1 reply; 30+ messages in thread
From: Amit Langote @ 2018-08-07 00:50 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; Andres Freund <andres@anarazel.de>; +Cc: Dean Rasheed <dean.a.rasheed@gmail.com>; Mario De Frutos Dieguez <mariodefrutos@gmail.com>; pgsql-bugs@lists.postgresql.org
On 2018/08/05 7:37, Tom Lane wrote:
> Attached is our finished patch against HEAD. This is pretty much all
> Dean's work, but I'm posting it on his behalf because it's late in the UK
> and he's gone offline for the day. In the interests of getting a
> full set of buildfarm testing on the patch before Monday's wrap deadline,
> I'm going to finish up back-porting the patch and push it tonight.
Thank you.
Regards,
Amit
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-02 10:38 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 06:52 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 09:40 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 18:46 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
2018-08-03 21:26 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 22:32 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-03 22:44 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
2018-08-03 22:58 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-04 22:37 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-07 00:50 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
@ 2018-08-07 06:31 ` Dean Rasheed <dean.a.rasheed@gmail.com>
0 siblings, 0 replies; 30+ messages in thread
From: Dean Rasheed @ 2018-08-07 06:31 UTC (permalink / raw)
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Andres Freund <andres@anarazel.de>; Mario De Frutos Dieguez <mariodefrutos@gmail.com>; pgsql-bugs@lists.postgresql.org
On 7 August 2018 at 01:50, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> On 2018/08/05 7:37, Tom Lane wrote:
>> Attached is our finished patch against HEAD. This is pretty much all
>> Dean's work, but I'm posting it on his behalf because it's late in the UK
>> and he's gone offline for the day. In the interests of getting a
>> full set of buildfarm testing on the patch before Monday's wrap deadline,
>> I'm going to finish up back-porting the patch and push it tonight.
>
> Thank you.
Indeed. Thank you Tom for reviewing and sorting out all the back-patches.
Regards,
Dean
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-02 10:38 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 06:52 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 09:40 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 18:46 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
2018-08-03 21:26 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 22:32 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-03 22:44 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
@ 2018-08-03 23:14 ` Peter Geoghegan <pg@bowt.ie>
1 sibling, 0 replies; 30+ messages in thread
From: Peter Geoghegan @ 2018-08-03 23:14 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Dean Rasheed <dean.a.rasheed@gmail.com>; Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; Mario De Frutos Dieguez <mariodefrutos@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Fri, Aug 3, 2018 at 3:44 PM, Andres Freund <andres@anarazel.de> wrote:
> This results in clearly inserting wrong data and/or crashing the
> server. And it's not a huge effect outside of already broken scenarios.
> I think we definitely should try to get this in.
I tend to agree. You told me privately that you had a customer that
had the same issue, so we know that it has affected multiple users.
I think that the surface area for new bugs from the fix has a lot of
overlap with cases that are already probably quite broken. I'm
concerned that existing affected users could suffer pernicious logical
corruption, that goes unnoticed for a long time but ultimately does a
lot of damage.
In the end, it's a matter for Dean, but there is definitely a good
case for proceeding with a full backpatch now, in my view.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-02 10:38 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 06:52 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 09:40 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
@ 2018-08-03 23:34 ` Peter Geoghegan <pg@bowt.ie>
3 siblings, 0 replies; 30+ messages in thread
From: Peter Geoghegan @ 2018-08-03 23:34 UTC (permalink / raw)
To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; Tom Lane <tgl@sss.pgh.pa.us>; Mario De Frutos Dieguez <mariodefrutos@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Fri, Aug 3, 2018 at 2:40 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> This patch also allows access to view columns that aren't in the
> underlying base relation. The rationale for the result in the new test
> case where it attempts to insert (1,'y') into columns (aa,bb) of the
> view is that the new view row that would have resulted if the insert
> had succeeded is ('y',1,(1,'y')), hence that's what excluded.* should
> be for the view in the "on conflict" action, and there should be no
> problem referring to any part of that excluded view tuple.
I agree with your rationale. And, I don't think that it's just a
theoretical point; it actually really matters to affected users.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-02 10:38 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 06:52 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 09:40 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
@ 2018-08-03 23:51 ` Andres Freund <andres@anarazel.de>
2018-08-04 00:01 ` Re: Fwd: Problem with a "complex" upsert Peter Geoghegan <pg@bowt.ie>
3 siblings, 1 reply; 30+ messages in thread
From: Andres Freund @ 2018-08-03 23:51 UTC (permalink / raw)
To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; Tom Lane <tgl@sss.pgh.pa.us>; Mario De Frutos Dieguez <mariodefrutos@gmail.com>; pgsql-bugs@lists.postgresql.org
Hi,
On 2018-08-03 10:40:50 +0100, Dean Rasheed wrote:
> On 3 August 2018 at 07:52, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> > This doesn't look right to me. It breaks the following case ...
>
> Here's an updated patch that fixes this.
Looking through the patch.
> diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
> new file mode 100644
> index 05f5759..87e084b
> --- a/src/backend/parser/analyze.c
> +++ b/src/backend/parser/analyze.c
> @@ -1022,9 +1022,6 @@ transformOnConflictClause(ParseState *ps
> if (onConflictClause->action == ONCONFLICT_UPDATE)
> {
> Relation targetrel = pstate->p_target_relation;
> - Var *var;
> - TargetEntry *te;
> - int attno;
>
> /*
> * All INSERT expressions have been parsed, get ready for potentially
> @@ -1043,56 +1040,8 @@ transformOnConflictClause(ParseState *ps
> false, false);
> exclRte->relkind = RELKIND_COMPOSITE_TYPE;
> exclRelIndex = list_length(pstate->p_rtable);
> -
> - /*
> - * Build a targetlist representing the columns of the EXCLUDED pseudo
> - * relation. Have to be careful to use resnos that correspond to
> - * attnos of the underlying relation.
> - */
> - for (attno = 0; attno < RelationGetNumberOfAttributes(targetrel); attno++)
> - {
> - Form_pg_attribute attr = TupleDescAttr(targetrel->rd_att, attno);
> - char *name;
> -
> - if (attr->attisdropped)
> - {
> - /*
> - * can't use atttypid here, but it doesn't really matter what
> - * type the Const claims to be.
> - */
> - var = (Var *) makeNullConst(INT4OID, -1, InvalidOid);
> - name = "";
> - }
> - else
> - {
> - var = makeVar(exclRelIndex, attno + 1,
> - attr->atttypid, attr->atttypmod,
> - attr->attcollation,
> - 0);
> - name = pstrdup(NameStr(attr->attname));
> - }
> -
> - te = makeTargetEntry((Expr *) var,
> - attno + 1,
> - name,
> - false);
> -
> - /* don't require select access yet */
> - exclRelTlist = lappend(exclRelTlist, te);
> - }
> -
> - /*
> - * Add a whole-row-Var entry to support references to "EXCLUDED.*".
> - * Like the other entries in exclRelTlist, its resno must match the
> - * Var's varattno, else the wrong things happen while resolving
> - * references in setrefs.c. This is against normal conventions for
> - * targetlists, but it's okay since we don't use this as a real tlist.
> - */
> - var = makeVar(exclRelIndex, InvalidAttrNumber,
> - targetrel->rd_rel->reltype,
> - -1, InvalidOid, 0);
> - te = makeTargetEntry((Expr *) var, InvalidAttrNumber, NULL, true);
> - exclRelTlist = lappend(exclRelTlist, te);
> + exclRelTlist = BuildOnConflictExcludedTargetlist(targetrel,
> + exclRelIndex);
>
> /*
> * Add EXCLUDED and the target RTE to the namespace, so that they can
> @@ -1124,6 +1073,75 @@ transformOnConflictClause(ParseState *ps
>
> return result;
> }
> +
> +
> +/*
> + * BuildOnConflictExcludedTargetlist
> + * Create the target list of EXCLUDED pseudo-relation of ON CONFLICT
> + *
> + * Note: Exported for use in the rewriter.
> + */
> +List *
> +BuildOnConflictExcludedTargetlist(Relation targetrel,
> + Index exclRelIndex)
> +{
> + List *result = NIL;
> + int attno;
> + Var *var;
> + TargetEntry *te;
> +
> + /*
> + * Build a targetlist representing the columns of the EXCLUDED pseudo
> + * relation. Have to be careful to use resnos that correspond to attnos
> + * of the underlying relation.
> + */
> + for (attno = 0; attno < RelationGetNumberOfAttributes(targetrel); attno++)
> + {
> + Form_pg_attribute attr = TupleDescAttr(targetrel->rd_att, attno);
> + char *name;
> +
> + if (attr->attisdropped)
> + {
> + /*
> + * can't use atttypid here, but it doesn't really matter what type
> + * the Const claims to be.
> + */
> + var = (Var *) makeNullConst(INT4OID, -1, InvalidOid);
> + name = "";
> + }
> + else
> + {
> + var = makeVar(exclRelIndex, attno + 1,
> + attr->atttypid, attr->atttypmod,
> + attr->attcollation,
> + 0);
> + name = pstrdup(NameStr(attr->attname));
> + }
> +
> + te = makeTargetEntry((Expr *) var,
> + attno + 1,
> + name,
> + false);
> +
> + /* don't require select access yet */
> + result = lappend(result, te);
> + }
> +
> + /*
> + * Add a whole-row-Var entry to support references to "EXCLUDED.*". Like
> + * the other entries in exclRelTlist, its resno must match the Var's
> + * varattno, else the wrong things happen while resolving references in
> + * setrefs.c. This is against normal conventions for targetlists, but
> + * it's okay since we don't use this as a real tlist.
> + */
> + var = makeVar(exclRelIndex, InvalidAttrNumber,
> + targetrel->rd_rel->reltype,
> + -1, InvalidOid, 0);
> + te = makeTargetEntry((Expr *) var, InvalidAttrNumber, NULL, true);
> + result = lappend(result, te);
> +
> + return result;
> +}
On a skim this purely is moving code around - no functional changes, right?
> +-- Check INSERT .. ON CONFLICT DO UPDATE works correctly when the view's
> +-- columns are named and ordered differently than the underlying table's.
> +create table uv_iocu_tab (a text unique, b float);
> +insert into uv_iocu_tab values ('xyxyxy', 1);
> +create view uv_iocu_view as select b, b+1 as c, a from uv_iocu_tab;
> +insert into uv_iocu_view (a, b) values ('xyxyxy', 2)
> + on conflict (a) do update set b = uv_iocu_view.b;
> +
> +-- OK to access view columns that are not present in underlying base
> +-- relation in the ON CONFLICT portion of the query
> +explain (costs off)
> +insert into uv_iocu_view (a, b) values ('xyxyxy', 3)
> + on conflict (a) do update set b = excluded.b where excluded.c > 0;
> +
> +insert into uv_iocu_view (a, b) values ('xyxyxy', 3)
> + on conflict (a) do update set b = excluded.b where excluded.c > 0;
> +-- should display 'xyxyxy, 3'
> +select * from uv_iocu_tab;
> +drop view uv_iocu_view;
> +drop table uv_iocu_tab;
> +
> +-- Example with whole-row references to the view
> +create table uv_iocu_tab (a int unique, b text);
> +create view uv_iocu_view as
> + select b as bb, a as aa, uv_iocu_tab::text as cc from uv_iocu_tab;
> +
> +insert into uv_iocu_view (aa,bb) values (1,'x');
> +explain (costs off)
> +insert into uv_iocu_view (aa,bb) values (1,'y')
> + on conflict (aa) do update set bb = 'Rejected: '||excluded.*
> + where excluded.aa > 0
> + and excluded.bb != ''
> + and excluded.cc is not null;
> +insert into uv_iocu_view (aa,bb) values (1,'y')
> + on conflict (aa) do update set bb = 'Rejected: '||excluded.*
> + where excluded.aa > 0
> + and excluded.bb != ''
> + and excluded.cc is not null;
> +select * from uv_iocu_view;
> +
> +-- Test omiting a column of the base relation
> +delete from uv_iocu_view;
> +insert into uv_iocu_view (aa,bb) values (1,'x');
> +insert into uv_iocu_view (aa) values (1)
> + on conflict (aa) do update set bb = 'Rejected: '||excluded.*;
> +select * from uv_iocu_view;
> +
> +-- Should fail to update non-updatable columns
> +insert into uv_iocu_view (aa) values (1)
> + on conflict (aa) do update set cc = 'XXX';
> +
> +drop view uv_iocu_view;
> +drop table uv_iocu_tab;
Could you add a column that's just a const and one that that's now() or
something? And based on those add a test that makes sure we don't do
stupid thinks when they're referred to via EXCLUDED? AFAICS that
currently should work correctly, but it'd be good to test that.
Peter, I think, independent of this bug, I think it'd be good to add a
few tests around arbiter expression for ON CONFLICT over a view.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Fwd: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Fwd: Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 17:05 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-07-10 20:59 ` Re: Fwd: Problem with a "complex" upsert Tom Lane <tgl@sss.pgh.pa.us>
2018-08-02 10:38 ` Re: Fwd: Problem with a "complex" upsert Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 06:52 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 09:40 ` Re: Fwd: Problem with a "complex" upsert Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 23:51 ` Re: Fwd: Problem with a "complex" upsert Andres Freund <andres@anarazel.de>
@ 2018-08-04 00:01 ` Peter Geoghegan <pg@bowt.ie>
0 siblings, 0 replies; 30+ messages in thread
From: Peter Geoghegan @ 2018-08-04 00:01 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Dean Rasheed <dean.a.rasheed@gmail.com>; Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>; Tom Lane <tgl@sss.pgh.pa.us>; Mario De Frutos Dieguez <mariodefrutos@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Fri, Aug 3, 2018 at 4:51 PM, Andres Freund <andres@anarazel.de> wrote:
> Peter, I think, independent of this bug, I think it'd be good to add a
> few tests around arbiter expression for ON CONFLICT over a view.
I'll look at this tomorrow.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
@ 2018-06-21 14:40 ` Geoff Winkless <pgsqladmin@geoff.dj>
2018-06-21 14:46 ` Re: Problem with a "complex" upsert Mario de Frutos Dieguez <mariodefrutos@gmail.com>
1 sibling, 1 reply; 30+ messages in thread
From: Geoff Winkless @ 2018-06-21 14:40 UTC (permalink / raw)
To: mariodefrutos@gmail.com; +Cc: pgsql-admin@lists.postgresql.org
On Thu, 21 Jun 2018 at 15:07, Mario De Frutos Dieguez <
mariodefrutos@gmail.com> wrote:
> ON CONFLICT (geoid) DO UPDATE SET (b01003001) = ROW(EXCLUDED.b01003001);
>
At first glance, shouldn't the query simply be
SET b01003001 = EXCLUDED.b01003001;
?
The second part of your email does suggest a bug though, syntax shouldn't
cause a crash.
Geoff
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:40 ` Re: Problem with a "complex" upsert Geoff Winkless <pgsqladmin@geoff.dj>
@ 2018-06-21 14:46 ` Mario de Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 16:48 ` Re: Problem with a "complex" upsert Geoff Winkless <pgsqladmin@geoff.dj>
0 siblings, 1 reply; 30+ messages in thread
From: Mario de Frutos Dieguez @ 2018-06-21 14:46 UTC (permalink / raw)
To: ; +Cc: pgsql-admin@lists.postgresql.org
In this case is just one column but in other queries I'm updating
multiple columns thats why I set the ROW thing
2018-06-21 16:40 GMT+02:00 Geoff Winkless <pgsqladmin@geoff.dj>:
> On Thu, 21 Jun 2018 at 15:07, Mario De Frutos Dieguez
> <mariodefrutos@gmail.com> wrote:
>>
>> ON CONFLICT (geoid) DO UPDATE SET (b01003001) = ROW(EXCLUDED.b01003001);
>
> At first glance, shouldn't the query simply be
>
> SET b01003001 = EXCLUDED.b01003001;
>
> ?
>
> The second part of your email does suggest a bug though, syntax shouldn't
> cause a crash.
>
> Geoff
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Problem with a "complex" upsert
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:40 ` Re: Problem with a "complex" upsert Geoff Winkless <pgsqladmin@geoff.dj>
2018-06-21 14:46 ` Re: Problem with a "complex" upsert Mario de Frutos Dieguez <mariodefrutos@gmail.com>
@ 2018-06-21 16:48 ` Geoff Winkless <pgsqladmin@geoff.dj>
0 siblings, 0 replies; 30+ messages in thread
From: Geoff Winkless @ 2018-06-21 16:48 UTC (permalink / raw)
To: Mario De Frutos Dieguez <mariodefrutos@gmail.com>; +Cc: pgsql-admin@lists.postgresql.org
On Thu, 21 Jun 2018 at 15:46, Mario de Frutos Dieguez
<mariodefrutos@gmail.com> wrote:
> In this case is just one column but in other queries I'm updating
> multiple columns thats why I set the ROW thing
Hmm. The documentation isn't explicit that that's valid syntax. Choices are
({expression|DEFAULT} [,...] |
( sub-SELECT )
and although the docs do say row constructors are valid as sub-SELECT
for comparisons I'm not sure that it fits here.
It does seem like it's trying to work anyway but my guess is that
"Table has type character varying". doesn't mean it's trying to use
the geoid value, but rather that it's implying that the ROW() (which
returns an anonymous type) is the same type as the target table of the
main INSERT query (rather than the type of the column in brackets).
Whether it counts as a bug or not (given that it's not explicitly
defined as allowed syntax) is probably an esoteric argument.
Are you using the ROW() syntax just because it's easier to build the
query programmatically? Otherwise I can't see why you wouldn't just
use SET col1=EXCLUDED.col1, col2=EXCLUDED.col2 [, ....]
Does
SET (b01003001) = (SELECT b01003001 FROM EXCLUDED)
work?
Geoff
^ permalink raw reply [nested|flat] 30+ messages in thread
end of thread, other threads:[~2018-08-07 06:31 UTC | newest]
Thread overview: 30+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-06-21 14:07 Problem with a "complex" upsert Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 14:11 ` Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 16:55 ` Peter Geoghegan <pg@bowt.ie>
2018-06-21 17:05 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-06-22 08:11 ` Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-06-22 09:15 ` Mario De Frutos Dieguez <mariodefrutos@gmail.com>
2018-07-10 20:59 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-07-30 22:53 ` Peter Geoghegan <pg@bowt.ie>
2018-07-31 01:11 ` Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-02 10:38 ` Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 06:52 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 09:40 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 10:17 ` Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-03 18:46 ` Andres Freund <andres@anarazel.de>
2018-08-03 21:26 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 22:32 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-08-03 22:44 ` Andres Freund <andres@anarazel.de>
2018-08-03 22:58 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-08-04 22:37 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-08-06 16:41 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-08-06 16:48 ` Mario de Frutos Dieguez <mariodefrutos@gmail.com>
2018-08-07 00:50 ` Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
2018-08-07 06:31 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2018-08-03 23:14 ` Peter Geoghegan <pg@bowt.ie>
2018-08-03 23:34 ` Peter Geoghegan <pg@bowt.ie>
2018-08-03 23:51 ` Andres Freund <andres@anarazel.de>
2018-08-04 00:01 ` Peter Geoghegan <pg@bowt.ie>
2018-06-21 14:40 ` Geoff Winkless <pgsqladmin@geoff.dj>
2018-06-21 14:46 ` Mario de Frutos Dieguez <mariodefrutos@gmail.com>
2018-06-21 16:48 ` Geoff Winkless <pgsqladmin@geoff.dj>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox