Hi.
On 2018/02/20 5:45, Alvaro Herrera wrote:
> I pushed this now, with fixes for the last few comments there were.
I noticed with the commit that, while ON CONFLICT (conflict_target) DO
UPDATE gives a less surprising error message by catching it in the parser,
ON CONFLICT (conflict_target) DO NOTHING will go into the executor without
the necessary code to handle the case. Example:
create table p (a int primary key, b text) partition by list (a);
create table p12 partition of p for values in (1, 2);
create table p3 partition of p (a unique) for values in (3);
insert into p values (1, 'a') on conflict (a) do nothing;
ERROR: unexpected failure to find arbiter index
Attached is a patch to fix that. Actually, there are two -- one that
adjusts the partitioned table tests in insert_conflict.sql to have a
partitioned unique index and another that fixes the code.
I suppose we'd need to apply this temporarily until we fix the ON CONFLICT
(conflict_target) case to be able to use partitioned indexes.
Thanks,
Amit
From c5536f89c6723db7b2af8d93ab544254763b522a Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Tue, 20 Feb 2018 18:02:30 +0900
Subject: [PATCH v1 1/2] Adjust partitioned table tests in insert_conflict.sql
---
src/test/regress/expected/insert_conflict.out | 24 +++++++++++++++++-------
src/test/regress/sql/insert_conflict.sql | 16 +++++++++-------
2 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/src/test/regress/expected/insert_conflict.out
b/src/test/regress/expected/insert_conflict.out
index 2650faedee..a46fe7ec60 100644
--- a/src/test/regress/expected/insert_conflict.out
+++ b/src/test/regress/expected/insert_conflict.out
@@ -788,14 +788,24 @@ select * from selfconflict;
drop table selfconflict;
-- check that the following works:
-- insert into partitioned_table on conflict do nothing
-create table parted_conflict_test (a int, b char) partition by list (a);
-create table parted_conflict_test_1 partition of parted_conflict_test (b
unique) for values in (1);
-insert into parted_conflict_test values (1, 'a') on conflict do nothing;
-insert into parted_conflict_test values (1, 'a') on conflict do nothing;
+create table parted_conflict_test (a int unique, b char) partition by list (a);
+create table parted_conflict_test_1 partition of parted_conflict_test (b
unique) for values in (1, 2);
+insert into parted_conflict_test values (1, 'a') on conflict (a) do nothing;
+ERROR: unexpected failure to find arbiter index
+insert into parted_conflict_test values (1, 'a') on conflict (a) do nothing;
+ERROR: unexpected failure to find arbiter index
-- however, on conflict do update is not supported yet
-insert into parted_conflict_test values (1) on conflict (b) do update set a =
excluded.a;
+insert into parted_conflict_test values (1, 'a') on conflict (b) do update set
a = excluded.a;
+ERROR: ON CONFLICT DO UPDATE cannot be applied to partitioned table
"parted_conflict_test"
+insert into parted_conflict_test values (1, 'a') on conflict (a) do update set
b = excluded.b;
ERROR: ON CONFLICT DO UPDATE cannot be applied to partitioned table
"parted_conflict_test"
-- but it works OK if we target the partition directly
-insert into parted_conflict_test_1 values (1) on conflict (b) do
-update set a = excluded.a;
+insert into parted_conflict_test_1 values (2, 'a') on conflict (b) do update
set a = excluded.a;
+insert into parted_conflict_test_1 values (2, 'b') on conflict (a) do update
set b = excluded.b;
+select * from parted_conflict_test order by a;
+ a | b
+---+---
+ 2 | b
+(1 row)
+
drop table parted_conflict_test;
diff --git a/src/test/regress/sql/insert_conflict.sql
b/src/test/regress/sql/insert_conflict.sql
index 32c647e3f8..97affc3726 100644
--- a/src/test/regress/sql/insert_conflict.sql
+++ b/src/test/regress/sql/insert_conflict.sql
@@ -474,13 +474,15 @@ drop table selfconflict;
-- check that the following works:
-- insert into partitioned_table on conflict do nothing
-create table parted_conflict_test (a int, b char) partition by list (a);
-create table parted_conflict_test_1 partition of parted_conflict_test (b
unique) for values in (1);
-insert into parted_conflict_test values (1, 'a') on conflict do nothing;
-insert into parted_conflict_test values (1, 'a') on conflict do nothing;
+create table parted_conflict_test (a int unique, b char) partition by list (a);
+create table parted_conflict_test_1 partition of parted_conflict_test (b
unique) for values in (1, 2);
+insert into parted_conflict_test values (1, 'a') on conflict (a) do nothing;
+insert into parted_conflict_test values (1, 'a') on conflict (a) do nothing;
-- however, on conflict do update is not supported yet
-insert into parted_conflict_test values (1) on conflict (b) do update set a =
excluded.a;
+insert into parted_conflict_test values (1, 'a') on conflict (b) do update set
a = excluded.a;
+insert into parted_conflict_test values (1, 'a') on conflict (a) do update set
b = excluded.b;
-- but it works OK if we target the partition directly
-insert into parted_conflict_test_1 values (1) on conflict (b) do
-update set a = excluded.a;
+insert into parted_conflict_test_1 values (2, 'a') on conflict (b) do update
set a = excluded.a;
+insert into parted_conflict_test_1 values (2, 'b') on conflict (a) do update
set b = excluded.b;
+select * from parted_conflict_test order by a;
drop table parted_conflict_test;
--
2.11.0
From e88b485ec4aab08b80061c1899b38ad12736d8f9 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Tue, 20 Feb 2018 18:02:59 +0900
Subject: [PATCH v1 2/2] Fix ON CONFLICT DO NOTHING with partitioned indexes
---
src/backend/optimizer/plan/createplan.c | 41 +++++++++++++++++++++------
src/test/regress/expected/insert_conflict.out | 2 --
2 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/src/backend/optimizer/plan/createplan.c
b/src/backend/optimizer/plan/createplan.c
index da0cc7f266..98825b0b27 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -6504,20 +6504,43 @@ make_modifytable(PlannerInfo *root,
}
else
{
+ RangeTblEntry *rte;
+
node->onConflictAction = onconflict->action;
- node->onConflictSet = onconflict->onConflictSet;
- node->onConflictWhere = onconflict->onConflictWhere;
/*
- * If a set of unique index inference elements was provided (an
- * INSERT...ON CONFLICT "inference specification"), then infer
- * appropriate unique indexes (or throw an error if none are
- * available).
+ * Partitioned tables don't yet have the executor support needed
+ * to handle ON CONFLICT actions that rely on partitioned
indexes,
+ * so leave the following fields unset for them.
*/
- node->arbiterIndexes = infer_arbiter_indexes(root);
- node->exclRelRTI = onconflict->exclRelIndex;
- node->exclRelTlist = onconflict->exclRelTlist;
+ /* Must have only one result relation in the case of INSERT. */
+ Assert(list_length(node->resultRelations) == 1);
+ rte = planner_rt_fetch(linitial_int(node->resultRelations),
root);
+ Assert(rte->rtekind == RTE_RELATION);
+ if (rte->relkind != RELKIND_PARTITIONED_TABLE)
+ {
+ node->onConflictSet = onconflict->onConflictSet;
+ node->onConflictWhere = onconflict->onConflictWhere;
+
+ /*
+ * If a set of unique index inference elements was
provided (an
+ * INSERT...ON CONFLICT "inference specification"),
then infer
+ * appropriate unique indexes (or throw an error if
none are
+ * available).
+ */
+ node->arbiterIndexes = infer_arbiter_indexes(root);
+ node->exclRelRTI = onconflict->exclRelIndex;
+ node->exclRelTlist = onconflict->exclRelTlist;
+ }
+ else
+ {
+ node->onConflictSet = NIL;
+ node->onConflictWhere = NULL;
+ node->arbiterIndexes = NIL;
+ node->exclRelRTI = 0;
+ node->exclRelTlist = NIL;
+ }
}
node->withCheckOptionLists = withCheckOptionLists;
node->returningLists = returningLists;
diff --git a/src/test/regress/expected/insert_conflict.out
b/src/test/regress/expected/insert_conflict.out
index a46fe7ec60..7fa4282e0d 100644
--- a/src/test/regress/expected/insert_conflict.out
+++ b/src/test/regress/expected/insert_conflict.out
@@ -791,9 +791,7 @@ drop table selfconflict;
create table parted_conflict_test (a int unique, b char) partition by list (a);
create table parted_conflict_test_1 partition of parted_conflict_test (b
unique) for values in (1, 2);
insert into parted_conflict_test values (1, 'a') on conflict (a) do nothing;
-ERROR: unexpected failure to find arbiter index
insert into parted_conflict_test values (1, 'a') on conflict (a) do nothing;
-ERROR: unexpected failure to find arbiter index
-- however, on conflict do update is not supported yet
insert into parted_conflict_test values (1, 'a') on conflict (b) do update set
a = excluded.a;
ERROR: ON CONFLICT DO UPDATE cannot be applied to partitioned table
"parted_conflict_test"
--
2.11.0
