Don't throw serialization errors for self-conflicts in INSERT ON CONFLICT.

A transaction that conflicts against itself, for example
	INSERT INTO t(pk) VALUES (1),(1) ON CONFLICT DO NOTHING;
should behave the same regardless of isolation level.  It certainly
shouldn't throw a serialization error, as retrying will not help.
We got this wrong due to the ON CONFLICT logic not considering the case,
as reported by Jason Dusek.

Core of this patch is by Peter Geoghegan (based on an earlier patch by
Thomas Munro), though I didn't take his proposed code refactoring for fear
that it might have unexpected side-effects.  Test cases by Thomas Munro
and myself.

Report: <CAO3NbwOycQjt2Oqy2VW-eLTq2M5uGMyHnGm=RNga4mjqcYD7gQ@mail.gmail.com>
Related-Discussion: <57EE93C8.8080504@postgrespro.ru>

diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index af7b26c0ef01ae5db18394db8740cef7e06afe1f..b056dd9e9510c0dcd862d6f68f9512b38a71fbd0 100644 (file)
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -195,9 +195,18 @@ ExecCheckHeapTupleVisible(EState *estate,
                return;
 
        if (!HeapTupleSatisfiesVisibility(tuple, estate->es_snapshot, buffer))
-               ereport(ERROR,
-                               (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
+       {
+               /*
+                * We should not raise a serialization failure if the conflict is
+                * against a tuple inserted by our own transaction, even if it's not
+                * visible to our snapshot.  (This would happen, for example, if
+                * conflicting keys are proposed for insertion in a single command.)
+                */
+               if (!TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(tuple->t_data)))
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
                         errmsg("could not serialize access due to concurrent update")));
+       }
 }
 
 /*

diff --git a/src/test/isolation/expected/insert-conflict-do-nothing-2.out b/src/test/isolation/expected/insert-conflict-do-nothing-2.out
new file mode 100644 (file)
index 0000000..2332f96
--- /dev/null
+++ b/src/test/isolation/expected/insert-conflict-do-nothing-2.out
@@ -0,0 +1,105 @@
+Parsed test spec with 2 sessions
+
+starting permutation: beginrr1 beginrr2 donothing1 c1 donothing2 c2 show
+step beginrr1: BEGIN ISOLATION LEVEL REPEATABLE READ;
+step beginrr2: BEGIN ISOLATION LEVEL REPEATABLE READ;
+step donothing1: INSERT INTO ints(key, val) VALUES(1, 'donothing1') ON CONFLICT DO NOTHING;
+step c1: COMMIT;
+step donothing2: INSERT INTO ints(key, val) VALUES(1, 'donothing2'), (1, 'donothing3') ON CONFLICT DO NOTHING;
+step c2: COMMIT;
+step show: SELECT * FROM ints;
+key            val            
+
+1              donothing1     
+
+starting permutation: beginrr1 beginrr2 donothing2 c2 donothing1 c1 show
+step beginrr1: BEGIN ISOLATION LEVEL REPEATABLE READ;
+step beginrr2: BEGIN ISOLATION LEVEL REPEATABLE READ;
+step donothing2: INSERT INTO ints(key, val) VALUES(1, 'donothing2'), (1, 'donothing3') ON CONFLICT DO NOTHING;
+step c2: COMMIT;
+step donothing1: INSERT INTO ints(key, val) VALUES(1, 'donothing1') ON CONFLICT DO NOTHING;
+step c1: COMMIT;
+step show: SELECT * FROM ints;
+key            val            
+
+1              donothing2     
+
+starting permutation: beginrr1 beginrr2 donothing1 donothing2 c1 c2 show
+step beginrr1: BEGIN ISOLATION LEVEL REPEATABLE READ;
+step beginrr2: BEGIN ISOLATION LEVEL REPEATABLE READ;
+step donothing1: INSERT INTO ints(key, val) VALUES(1, 'donothing1') ON CONFLICT DO NOTHING;
+step donothing2: INSERT INTO ints(key, val) VALUES(1, 'donothing2'), (1, 'donothing3') ON CONFLICT DO NOTHING; <waiting ...>
+step c1: COMMIT;
+step donothing2: <... completed>
+error in steps c1 donothing2: ERROR:  could not serialize access due to concurrent update
+step c2: COMMIT;
+step show: SELECT * FROM ints;
+key            val            
+
+1              donothing1     
+
+starting permutation: beginrr1 beginrr2 donothing2 donothing1 c2 c1 show
+step beginrr1: BEGIN ISOLATION LEVEL REPEATABLE READ;
+step beginrr2: BEGIN ISOLATION LEVEL REPEATABLE READ;
+step donothing2: INSERT INTO ints(key, val) VALUES(1, 'donothing2'), (1, 'donothing3') ON CONFLICT DO NOTHING;
+step donothing1: INSERT INTO ints(key, val) VALUES(1, 'donothing1') ON CONFLICT DO NOTHING; <waiting ...>
+step c2: COMMIT;
+step donothing1: <... completed>
+error in steps c2 donothing1: ERROR:  could not serialize access due to concurrent update
+step c1: COMMIT;
+step show: SELECT * FROM ints;
+key            val            
+
+1              donothing2     
+
+starting permutation: begins1 begins2 donothing1 c1 donothing2 c2 show
+step begins1: BEGIN ISOLATION LEVEL SERIALIZABLE;
+step begins2: BEGIN ISOLATION LEVEL SERIALIZABLE;
+step donothing1: INSERT INTO ints(key, val) VALUES(1, 'donothing1') ON CONFLICT DO NOTHING;
+step c1: COMMIT;
+step donothing2: INSERT INTO ints(key, val) VALUES(1, 'donothing2'), (1, 'donothing3') ON CONFLICT DO NOTHING;
+step c2: COMMIT;
+step show: SELECT * FROM ints;
+key            val            
+
+1              donothing1     
+
+starting permutation: begins1 begins2 donothing2 c2 donothing1 c1 show
+step begins1: BEGIN ISOLATION LEVEL SERIALIZABLE;
+step begins2: BEGIN ISOLATION LEVEL SERIALIZABLE;
+step donothing2: INSERT INTO ints(key, val) VALUES(1, 'donothing2'), (1, 'donothing3') ON CONFLICT DO NOTHING;
+step c2: COMMIT;
+step donothing1: INSERT INTO ints(key, val) VALUES(1, 'donothing1') ON CONFLICT DO NOTHING;
+step c1: COMMIT;
+step show: SELECT * FROM ints;
+key            val            
+
+1              donothing2     
+
+starting permutation: begins1 begins2 donothing1 donothing2 c1 c2 show
+step begins1: BEGIN ISOLATION LEVEL SERIALIZABLE;
+step begins2: BEGIN ISOLATION LEVEL SERIALIZABLE;
+step donothing1: INSERT INTO ints(key, val) VALUES(1, 'donothing1') ON CONFLICT DO NOTHING;
+step donothing2: INSERT INTO ints(key, val) VALUES(1, 'donothing2'), (1, 'donothing3') ON CONFLICT DO NOTHING; <waiting ...>
+step c1: COMMIT;
+step donothing2: <... completed>
+error in steps c1 donothing2: ERROR:  could not serialize access due to concurrent update
+step c2: COMMIT;
+step show: SELECT * FROM ints;
+key            val            
+
+1              donothing1     
+
+starting permutation: begins1 begins2 donothing2 donothing1 c2 c1 show
+step begins1: BEGIN ISOLATION LEVEL SERIALIZABLE;
+step begins2: BEGIN ISOLATION LEVEL SERIALIZABLE;
+step donothing2: INSERT INTO ints(key, val) VALUES(1, 'donothing2'), (1, 'donothing3') ON CONFLICT DO NOTHING;
+step donothing1: INSERT INTO ints(key, val) VALUES(1, 'donothing1') ON CONFLICT DO NOTHING; <waiting ...>
+step c2: COMMIT;
+step donothing1: <... completed>
+error in steps c2 donothing1: ERROR:  could not serialize access due to concurrent update
+step c1: COMMIT;
+step show: SELECT * FROM ints;
+key            val            
+
+1              donothing2     

diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index a96a31898717b343aad67fb8c1e95e234aaa60cb..2606a276247ee720be8a7c1000e46b9557b47b9b 100644 (file)
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -25,6 +25,7 @@ test: eval-plan-qual
 test: lock-update-delete
 test: lock-update-traversal
 test: insert-conflict-do-nothing
+test: insert-conflict-do-nothing-2
 test: insert-conflict-do-update
 test: insert-conflict-do-update-2
 test: insert-conflict-do-update-3

diff --git a/src/test/isolation/specs/insert-conflict-do-nothing-2.spec b/src/test/isolation/specs/insert-conflict-do-nothing-2.spec
new file mode 100644 (file)
index 0000000..f1e5bde
--- /dev/null
+++ b/src/test/isolation/specs/insert-conflict-do-nothing-2.spec
@@ -0,0 +1,34 @@
+# INSERT...ON CONFLICT DO NOTHING test with multiple rows
+# in higher isolation levels
+
+setup
+{
+  CREATE TABLE ints (key int primary key, val text);
+}
+
+teardown
+{
+  DROP TABLE ints;
+}
+
+session "s1"
+step "beginrr1" { BEGIN ISOLATION LEVEL REPEATABLE READ; }
+step "begins1" { BEGIN ISOLATION LEVEL SERIALIZABLE; }
+step "donothing1" { INSERT INTO ints(key, val) VALUES(1, 'donothing1') ON CONFLICT DO NOTHING; }
+step "c1" { COMMIT; }
+step "show" { SELECT * FROM ints; }
+
+session "s2"
+step "beginrr2" { BEGIN ISOLATION LEVEL REPEATABLE READ; }
+step "begins2" { BEGIN ISOLATION LEVEL SERIALIZABLE; }
+step "donothing2" { INSERT INTO ints(key, val) VALUES(1, 'donothing2'), (1, 'donothing3') ON CONFLICT DO NOTHING; }
+step "c2" { COMMIT; }
+
+permutation "beginrr1" "beginrr2" "donothing1" "c1" "donothing2" "c2" "show"
+permutation "beginrr1" "beginrr2" "donothing2" "c2" "donothing1" "c1" "show"
+permutation "beginrr1" "beginrr2" "donothing1" "donothing2" "c1" "c2" "show"
+permutation "beginrr1" "beginrr2" "donothing2" "donothing1" "c2" "c1" "show"
+permutation "begins1" "begins2" "donothing1" "c1" "donothing2" "c2" "show"
+permutation "begins1" "begins2" "donothing2" "c2" "donothing1" "c1" "show"
+permutation "begins1" "begins2" "donothing1" "donothing2" "c1" "c2" "show"
+permutation "begins1" "begins2" "donothing2" "donothing1" "c2" "c1" "show"

diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out
index 155774ecfab63a8931434baa79802d00f53703e0..8d8d69b1ad8d5e79302ab33359cb10d7743387b6 100644 (file)
--- a/src/test/regress/expected/insert_conflict.out
+++ b/src/test/regress/expected/insert_conflict.out
@@ -727,3 +727,38 @@ select * from twoconstraints;
 (1 row)
 
 drop table twoconstraints;
+-- check handling of self-conflicts at various isolation levels
+create table selfconflict (f1 int primary key, f2 int);
+begin transaction isolation level read committed;
+insert into selfconflict values (1,1), (1,2) on conflict do nothing;
+commit;
+begin transaction isolation level repeatable read;
+insert into selfconflict values (2,1), (2,2) on conflict do nothing;
+commit;
+begin transaction isolation level serializable;
+insert into selfconflict values (3,1), (3,2) on conflict do nothing;
+commit;
+begin transaction isolation level read committed;
+insert into selfconflict values (4,1), (4,2) on conflict(f1) do update set f2 = 0;
+ERROR:  ON CONFLICT DO UPDATE command cannot affect row a second time
+HINT:  Ensure that no rows proposed for insertion within the same command have duplicate constrained values.
+commit;
+begin transaction isolation level repeatable read;
+insert into selfconflict values (5,1), (5,2) on conflict(f1) do update set f2 = 0;
+ERROR:  ON CONFLICT DO UPDATE command cannot affect row a second time
+HINT:  Ensure that no rows proposed for insertion within the same command have duplicate constrained values.
+commit;
+begin transaction isolation level serializable;
+insert into selfconflict values (6,1), (6,2) on conflict(f1) do update set f2 = 0;
+ERROR:  ON CONFLICT DO UPDATE command cannot affect row a second time
+HINT:  Ensure that no rows proposed for insertion within the same command have duplicate constrained values.
+commit;
+select * from selfconflict;
+ f1 | f2 
+----+----
+  1 |  1
+  2 |  1
+  3 |  1
+(3 rows)
+
+drop table selfconflict;

diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql
index 190c6055623166dbfd6a0200f59fb36d7c731a8f..81c4a7ca4bade042d53fa54107ea5aa222d76063 100644 (file)
--- a/src/test/regress/sql/insert_conflict.sql
+++ b/src/test/regress/sql/insert_conflict.sql
@@ -421,3 +421,35 @@ 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;
+
+-- check handling of self-conflicts at various isolation levels
+
+create table selfconflict (f1 int primary key, f2 int);
+
+begin transaction isolation level read committed;
+insert into selfconflict values (1,1), (1,2) on conflict do nothing;
+commit;
+
+begin transaction isolation level repeatable read;
+insert into selfconflict values (2,1), (2,2) on conflict do nothing;
+commit;
+
+begin transaction isolation level serializable;
+insert into selfconflict values (3,1), (3,2) on conflict do nothing;
+commit;
+
+begin transaction isolation level read committed;
+insert into selfconflict values (4,1), (4,2) on conflict(f1) do update set f2 = 0;
+commit;
+
+begin transaction isolation level repeatable read;
+insert into selfconflict values (5,1), (5,2) on conflict(f1) do update set f2 = 0;
+commit;
+
+begin transaction isolation level serializable;
+insert into selfconflict values (6,1), (6,2) on conflict(f1) do update set f2 = 0;
+commit;
+
+select * from selfconflict;
+
+drop table selfconflict;