From 87c37e3291cb75273ccdf4645b9472dd805c4493 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Fri, 1 Dec 2017 12:53:21 -0500 Subject: [PATCH] Re-allow INSERT .. ON CONFLICT DO NOTHING on partitioned tables. Commit 8355a011a0124bdf7ccbada206a967d427039553 was reverted in f05230752d53c4aa74cffa9b699983bbb6bcb118, but this attempt is hopefully better-considered: we now pass the correct value to ExecOpenIndices, which should avoid the crash that we hit before. Amit Langote, reviewed by Simon Riggs and by me. Some final editing by me. Discussion: http://postgr.es/m/7ff1e8ec-dc39-96b1-7f47-ff5965dceeac@lab.ntt.co.jp --- doc/src/sgml/ddl.sgml | 13 +++++++++---- src/backend/commands/copy.c | 3 ++- src/backend/executor/execPartition.c | 15 ++++++++++----- src/backend/executor/nodeModifyTable.c | 3 ++- src/backend/parser/analyze.c | 8 -------- src/include/executor/execPartition.h | 3 ++- src/test/regress/expected/insert_conflict.out | 13 +++++++++++++ src/test/regress/sql/insert_conflict.sql | 13 +++++++++++++ 8 files changed, 51 insertions(+), 20 deletions(-) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 9f583266de..b1167a40e6 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -3288,10 +3288,15 @@ ALTER TABLE measurement ATTACH PARTITION measurement_y2008m02 Using the ON CONFLICT clause with partitioned tables - will cause an error, because unique or exclusion constraints can only be - created on individual partitions. There is no support for enforcing - uniqueness (or an exclusion constraint) across an entire partitioning - hierarchy. + will cause an error if the conflict target is specified (see + for more details on how the clause + works). Therefore, it is not possible to specify + DO UPDATE as the alternative action, because + specifying the conflict target is mandatory in that case. On the other + hand, specifying DO NOTHING as the alternative action + works fine provided the conflict target is not specified. In that case, + unique constraints (or exclusion constraints) of the individual leaf + partitions are considered. diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 13eb9e34ba..bace390470 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -2478,7 +2478,8 @@ CopyFrom(CopyState cstate) int num_parted, num_partitions; - ExecSetupPartitionTupleRouting(cstate->rel, + ExecSetupPartitionTupleRouting(NULL, + cstate->rel, 1, estate, &partition_dispatch_info, diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 34875945e8..d545af2b67 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -63,7 +63,8 @@ static char *ExecBuildSlotPartitionKeyDescription(Relation rel, * RowExclusiveLock mode upon return from this function. */ void -ExecSetupPartitionTupleRouting(Relation rel, +ExecSetupPartitionTupleRouting(ModifyTableState *mtstate, + Relation rel, Index resultRTindex, EState *estate, PartitionDispatch **pd, @@ -133,13 +134,17 @@ ExecSetupPartitionTupleRouting(Relation rel, CheckValidResultRel(leaf_part_rri, CMD_INSERT); /* - * Open partition indices (remember we do not support ON CONFLICT in - * case of partitioned tables, so we do not need support information - * for speculative insertion) + * Open partition indices. The user may have asked to check for + * conflicts within this leaf partition and do "nothing" instead of + * throwing an error. Be prepared in that case by initializing the + * index information needed by ExecInsert() to perform speculative + * insertions. */ if (leaf_part_rri->ri_RelationDesc->rd_rel->relhasindex && leaf_part_rri->ri_IndexRelationDescs == NULL) - ExecOpenIndices(leaf_part_rri, false); + ExecOpenIndices(leaf_part_rri, + mtstate != NULL && + mtstate->mt_onconflict != ONCONFLICT_NONE); estate->es_leaf_result_relations = lappend(estate->es_leaf_result_relations, leaf_part_rri); diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 1e3ece9b34..afb83ed3ae 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1953,7 +1953,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) int num_parted, num_partitions; - ExecSetupPartitionTupleRouting(rel, + ExecSetupPartitionTupleRouting(mtstate, + rel, node->nominalRelation, estate, &partition_dispatch_info, diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 757a4a8fd1..d680d2285c 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -847,16 +847,8 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) /* Process ON CONFLICT, if any. */ if (stmt->onConflictClause) - { - /* Bail out if target relation is partitioned table */ - if (pstate->p_target_rangetblentry->relkind == RELKIND_PARTITIONED_TABLE) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("ON CONFLICT clause is not supported with partitioned tables"))); - qry->onConflict = transformOnConflictClause(pstate, stmt->onConflictClause); - } /* * If we have a RETURNING clause, we need to add the target relation to diff --git a/src/include/executor/execPartition.h b/src/include/executor/execPartition.h index 43ca9908aa..86a199d169 100644 --- a/src/include/executor/execPartition.h +++ b/src/include/executor/execPartition.h @@ -49,7 +49,8 @@ typedef struct PartitionDispatchData typedef struct PartitionDispatchData *PartitionDispatch; -extern void ExecSetupPartitionTupleRouting(Relation rel, +extern void ExecSetupPartitionTupleRouting(ModifyTableState *mtstate, + Relation rel, Index resultRTindex, EState *estate, PartitionDispatch **pd, diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out index 8d005fddd4..8fd2027d6a 100644 --- a/src/test/regress/expected/insert_conflict.out +++ b/src/test/regress/expected/insert_conflict.out @@ -786,3 +786,16 @@ select * from selfconflict; (3 rows) 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; +-- 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; +ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification +-- 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; +drop table parted_conflict_test; diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql index df3a9b59b5..32c647e3f8 100644 --- a/src/test/regress/sql/insert_conflict.sql +++ b/src/test/regress/sql/insert_conflict.sql @@ -471,3 +471,16 @@ commit; 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; +-- 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; +-- 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; +drop table parted_conflict_test;