Fix failure to handle conflicts in non-arbiter exclusion constraints.

ExecInsertIndexTuples treated an exclusion constraint as subject to
noDupErr processing even when it was not listed in arbiterIndexes, and
would therefore not error out for a conflict in such a constraint, instead
returning it as an arbiter-index failure.  That led to an infinite loop in
ExecInsert, since ExecCheckIndexConstraints ignored the index as-intended
and therefore didn't throw the expected error.  To fix, make the exclusion
constraint code path use the same condition as the index_insert call does
to decide whether no-error-for-duplicates behavior is appropriate.  While
at it, refactor a little bit to avoid unnecessary list_member_oid calls.
(That surely wouldn't save anything worth noticing, but I find the code
a bit clearer this way.)

Per bug report from Heikki Rauhala.  Back-patch to 9.5 where ON CONFLICT
was introduced.

Report: <4C976D6B-76B4-434C-8052-D009F7B7AEDA@reaktor.fi>

diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c
index c819d19db4229faec82999c765f1e9018797a365..0e2d834ed1d45f1ada4337a7ad2a550abc538f12 100644 (file)
--- a/src/backend/executor/execIndexing.c
+++ b/src/backend/executor/execIndexing.c
@@ -259,6 +259,9 @@ ExecCloseIndices(ResultRelInfo *resultRelInfo)
  *     the same is done for non-deferred constraints, but report
  *     if conflict was speculative or deferred conflict to caller)
  *
+ *     If 'arbiterIndexes' is nonempty, noDupErr applies only to
+ *     those indexes.  NIL means noDupErr applies to all indexes.
+ *
  *     CAUTION: this must not be called for a HOT update.
  *     We can't defend against that here for lack of info.
  *     Should we change the API to make it safer?
@@ -308,19 +311,15 @@ ExecInsertIndexTuples(TupleTableSlot *slot,
    {
        Relation    indexRelation = relationDescs[i];
        IndexInfo  *indexInfo;
+       bool        applyNoDupErr;
        IndexUniqueCheck checkUnique;
        bool        satisfiesConstraint;
-       bool        arbiter;
 
        if (indexRelation == NULL)
            continue;
 
        indexInfo = indexInfoArray[i];
 
-       /* Record if speculative insertion arbiter */
-       arbiter = list_member_oid(arbiterIndexes,
-                                 indexRelation->rd_index->indexrelid);
-
        /* If the index is marked as read-only, ignore it */
        if (!indexInfo->ii_ReadyForInserts)
            continue;
@@ -358,6 +357,12 @@ ExecInsertIndexTuples(TupleTableSlot *slot,
                       values,
                       isnull);
 
+       /* Check whether to apply noDupErr to this index */
+       applyNoDupErr = noDupErr &&
+           (arbiterIndexes == NIL ||
+            list_member_oid(arbiterIndexes,
+                            indexRelation->rd_index->indexrelid));
+
        /*
         * The index AM does the actual insertion, plus uniqueness checking.
         *
@@ -373,7 +378,7 @@ ExecInsertIndexTuples(TupleTableSlot *slot,
         */
        if (!indexRelation->rd_index->indisunique)
            checkUnique = UNIQUE_CHECK_NO;
-       else if (noDupErr && (arbiterIndexes == NIL || arbiter))
+       else if (applyNoDupErr)
            checkUnique = UNIQUE_CHECK_PARTIAL;
        else if (indexRelation->rd_index->indimmediate)
            checkUnique = UNIQUE_CHECK_YES;
@@ -407,7 +412,7 @@ ExecInsertIndexTuples(TupleTableSlot *slot,
            bool        violationOK;
            CEOUC_WAIT_MODE waitMode;
 
-           if (noDupErr)
+           if (applyNoDupErr)
            {
                violationOK = true;
                waitMode = CEOUC_LIVELOCK_PREVENTING_WAIT;

diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out
index 6f4ec867d9f48244e82a23ab80816abac9abd579..155774ecfab63a8931434baa79802d00f53703e0 100644 (file)
--- a/src/test/regress/expected/insert_conflict.out
+++ b/src/test/regress/expected/insert_conflict.out
@@ -704,3 +704,26 @@ insert into dropcol(key, keep1, keep2) values(1, '5', 5) on conflict(key)
 
 ;
 DROP TABLE dropcol;
+-- check handling of regular btree constraint along with gist constraint
+create table twoconstraints (f1 int unique, f2 box,
+                             exclude using gist(f2 with &&));
+insert into twoconstraints values(1, '((0,0),(1,1))');
+insert into twoconstraints values(1, '((2,2),(3,3))');  -- fail on f1
+ERROR:  duplicate key value violates unique constraint "twoconstraints_f1_key"
+DETAIL:  Key (f1)=(1) already exists.
+insert into twoconstraints values(2, '((0,0),(1,2))');  -- fail on f2
+ERROR:  conflicting key value violates exclusion constraint "twoconstraints_f2_excl"
+DETAIL:  Key (f2)=((1,2),(0,0)) conflicts with existing key (f2)=((1,1),(0,0)).
+insert into twoconstraints values(2, '((0,0),(1,2))')
+  on conflict on constraint twoconstraints_f1_key do nothing;  -- fail on f2
+ERROR:  conflicting key value violates exclusion constraint "twoconstraints_f2_excl"
+DETAIL:  Key (f2)=((1,2),(0,0)) conflicts with existing key (f2)=((1,1),(0,0)).
+insert into twoconstraints values(2, '((0,0),(1,2))')
+  on conflict on constraint twoconstraints_f2_excl do nothing;  -- do nothing
+select * from twoconstraints;
+ f1 |     f2      
+----+-------------
+  1 | (1,1),(0,0)
+(1 row)
+
+drop table twoconstraints;

diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql
index 94b1b0aadeebb2d37f961403c8e4e1a6f0a0449e..190c6055623166dbfd6a0200f59fb36d7c731a8f 100644 (file)
--- a/src/test/regress/sql/insert_conflict.sql
+++ b/src/test/regress/sql/insert_conflict.sql
@@ -407,3 +407,17 @@ insert into dropcol(key, keep1, keep2) values(1, '5', 5) on conflict(key)
 ;
 
 DROP TABLE dropcol;
+
+-- check handling of regular btree constraint along with gist constraint
+
+create table twoconstraints (f1 int unique, f2 box,
+                             exclude using gist(f2 with &&));
+insert into twoconstraints values(1, '((0,0),(1,1))');
+insert into twoconstraints values(1, '((2,2),(3,3))');  -- fail on f1
+insert into twoconstraints values(2, '((0,0),(1,2))');  -- fail on f2
+insert into twoconstraints values(2, '((0,0),(1,2))')
+  on conflict on constraint twoconstraints_f1_key do nothing;  -- fail on f2
+insert into twoconstraints values(2, '((0,0),(1,2))')
+  on conflict on constraint twoconstraints_f2_excl do nothing;  -- do nothing
+select * from twoconstraints;
+drop table twoconstraints;