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

diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 9f583266de90d300944be3f1abfaecfe265d749a..b1167a40e6a1d565b5489fec22da4863d8a2be39 100644 (file)
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -3288,10 +3288,15 @@ ALTER TABLE measurement ATTACH PARTITION measurement_y2008m02
      <listitem>
       <para>
        Using the <literal>ON CONFLICT</literal> 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
+       <xref linkend="sql-on-conflict" /> for more details on how the clause
+       works).  Therefore, it is not possible to specify
+       <literal>DO UPDATE</literal> as the alternative action, because
+       specifying the conflict target is mandatory in that case.  On the other
+       hand, specifying <literal>DO NOTHING</literal> 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.
       </para>
      </listitem>
 

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 13eb9e34ba2e85758bbc2bd7cec3599eb51498e1..bace390470fbec39fa17f1a6a63496d02016297d 100644 (file)
--- 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 34875945e81412eca0b74c15b68c4715c638481c..d545af2b677fa80b3377a0eba415710b81b56408 100644 (file)
--- 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 1e3ece9b34cc970dc6330605687290657e4d0d81..afb83ed3ae1d6c1b75cf73d06f9dd5fdab7118c2 100644 (file)
--- 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 757a4a8fd12cc68011d5c1d26645e47963c50b20..d680d2285c63763957c268307174b362fbbdd6b1 100644 (file)
--- 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 43ca9908aa6af7e121ab05cff2a6be816fc54c37..86a199d16994aafc4d14231acbe7c394197a421b 100644 (file)
--- 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 8d005fddd46c4c56840a59ade08d3f71c00c8983..8fd2027d6a6ef4b62c766de18b5b7f4aa6cd7576 100644 (file)
--- 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 df3a9b59b5b0882549c1d9aa70b6330fee2f7064..32c647e3f88e34ae66eb6153ca113ed4b5d9bf1d 100644 (file)
--- 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;