From a6c0a5b6e8a9498540c6a7bb1b6d68958acc9bc6 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 23 Oct 2016 18:36:13 -0400 Subject: [PATCH] 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: Related-Discussion: <57EE93C8.8080504@postgrespro.ru> --- src/backend/executor/nodeModifyTable.c | 13 ++- .../expected/insert-conflict-do-nothing-2.out | 105 ++++++++++++++++++ src/test/isolation/isolation_schedule | 1 + .../specs/insert-conflict-do-nothing-2.spec | 34 ++++++ src/test/regress/expected/insert_conflict.out | 35 ++++++ src/test/regress/sql/insert_conflict.sql | 32 ++++++ 6 files changed, 218 insertions(+), 2 deletions(-) create mode 100644 src/test/isolation/expected/insert-conflict-do-nothing-2.out create mode 100644 src/test/isolation/specs/insert-conflict-do-nothing-2.spec diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index af7b26c0ef..b056dd9e95 100644 --- 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 index 0000000000..2332f96978 --- /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; +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; +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; +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; +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 a96a318987..2606a27624 100644 --- 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 index 0000000000..f1e5bde357 --- /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 155774ecfa..8d8d69b1ad 100644 --- 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 190c605562..81c4a7ca4b 100644 --- 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;