mirror of
https://github.com/postgres/postgres.git
synced 2026-01-26 09:41:40 +03:00
Remove faulty Assert in partitioned INSERT...ON CONFLICT DO UPDATE.
Commit f16241bef mistakenly supposed that INSERT...ON CONFLICT DO
UPDATE rejects partitioned target tables. (This may have been
accurate when the patch was written, but it was already obsolete
when committed.) Hence, there's an assertion that we can't see
ItemPointerIndicatesMovedPartitions() in that path, but the assertion
is triggerable.
Some other places throw error if they see a moved-across-partitions
tuple, but there seems no need for that here, because if we just retry
then we get the same behavior as in the update-within-partition case,
as demonstrated by the new isolation test. So fix by deleting the
faulty Assert. (The fact that this is the fix doubtless explains
why we've heard no field complaints: the behavior of a non-assert
build is fine.)
The TM_Deleted case contains a cargo-culted copy of the same Assert,
which I also deleted to avoid confusion, although I believe that one
is actually not triggerable.
Per our code coverage report, neither the TM_Updated nor the
TM_Deleted case were reached at all by existing tests, so this
patch adds tests for both.
Reported-by: Dmitry Koval <d.koval@postgrespro.ru>
Author: Joseph Koshakow <koshy44@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/f5fffe4b-11b2-4557-a864-3587ff9b4c36@postgrespro.ru
Backpatch-through: 14
This commit is contained in:
@@ -2809,14 +2809,6 @@ ExecOnConflictUpdate(ModifyTableContext *context,
|
||||
(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
|
||||
errmsg("could not serialize access due to concurrent update")));
|
||||
|
||||
/*
|
||||
* As long as we don't support an UPDATE of INSERT ON CONFLICT for
|
||||
* a partitioned table we shouldn't reach to a case where tuple to
|
||||
* be lock is moved to another partition due to concurrent update
|
||||
* of the partition key.
|
||||
*/
|
||||
Assert(!ItemPointerIndicatesMovedPartitions(&tmfd.ctid));
|
||||
|
||||
/*
|
||||
* Tell caller to try again from the very start.
|
||||
*
|
||||
@@ -2834,7 +2826,6 @@ ExecOnConflictUpdate(ModifyTableContext *context,
|
||||
errmsg("could not serialize access due to concurrent delete")));
|
||||
|
||||
/* see TM_Updated case */
|
||||
Assert(!ItemPointerIndicatesMovedPartitions(&tmfd.ctid));
|
||||
ExecClearTuple(existing);
|
||||
return false;
|
||||
|
||||
|
||||
63
src/test/isolation/expected/insert-conflict-do-update-4.out
Normal file
63
src/test/isolation/expected/insert-conflict-do-update-4.out
Normal file
@@ -0,0 +1,63 @@
|
||||
Parsed test spec with 2 sessions
|
||||
|
||||
starting permutation: lock2 insert1 update2a c2 select1 c1
|
||||
step lock2: SELECT * FROM upsert WHERE i = 1 FOR UPDATE;
|
||||
i| j| k
|
||||
-+--+---
|
||||
1|10|100
|
||||
(1 row)
|
||||
|
||||
step insert1: INSERT INTO upsert VALUES (1, 11, 111)
|
||||
ON CONFLICT (i) DO UPDATE SET k = EXCLUDED.k; <waiting ...>
|
||||
step update2a: UPDATE upsert SET i = i + 10 WHERE i = 1;
|
||||
step c2: COMMIT;
|
||||
step insert1: <... completed>
|
||||
step select1: SELECT * FROM upsert;
|
||||
i| j| k
|
||||
--+--+---
|
||||
11|10|100
|
||||
1|11|111
|
||||
(2 rows)
|
||||
|
||||
step c1: COMMIT;
|
||||
|
||||
starting permutation: lock2 insert1 update2b c2 select1 c1
|
||||
step lock2: SELECT * FROM upsert WHERE i = 1 FOR UPDATE;
|
||||
i| j| k
|
||||
-+--+---
|
||||
1|10|100
|
||||
(1 row)
|
||||
|
||||
step insert1: INSERT INTO upsert VALUES (1, 11, 111)
|
||||
ON CONFLICT (i) DO UPDATE SET k = EXCLUDED.k; <waiting ...>
|
||||
step update2b: UPDATE upsert SET i = i + 150 WHERE i = 1;
|
||||
step c2: COMMIT;
|
||||
step insert1: <... completed>
|
||||
step select1: SELECT * FROM upsert;
|
||||
i| j| k
|
||||
---+--+---
|
||||
1|11|111
|
||||
151|10|100
|
||||
(2 rows)
|
||||
|
||||
step c1: COMMIT;
|
||||
|
||||
starting permutation: lock2 insert1 delete2 c2 select1 c1
|
||||
step lock2: SELECT * FROM upsert WHERE i = 1 FOR UPDATE;
|
||||
i| j| k
|
||||
-+--+---
|
||||
1|10|100
|
||||
(1 row)
|
||||
|
||||
step insert1: INSERT INTO upsert VALUES (1, 11, 111)
|
||||
ON CONFLICT (i) DO UPDATE SET k = EXCLUDED.k; <waiting ...>
|
||||
step delete2: DELETE FROM upsert WHERE i = 1;
|
||||
step c2: COMMIT;
|
||||
step insert1: <... completed>
|
||||
step select1: SELECT * FROM upsert;
|
||||
i| j| k
|
||||
-+--+---
|
||||
1|11|111
|
||||
(1 row)
|
||||
|
||||
step c1: COMMIT;
|
||||
@@ -52,6 +52,7 @@ test: insert-conflict-do-nothing-2
|
||||
test: insert-conflict-do-update
|
||||
test: insert-conflict-do-update-2
|
||||
test: insert-conflict-do-update-3
|
||||
test: insert-conflict-do-update-4
|
||||
test: insert-conflict-specconflict
|
||||
test: merge-insert-update
|
||||
test: merge-delete
|
||||
|
||||
42
src/test/isolation/specs/insert-conflict-do-update-4.spec
Normal file
42
src/test/isolation/specs/insert-conflict-do-update-4.spec
Normal file
@@ -0,0 +1,42 @@
|
||||
# INSERT...ON CONFLICT DO UPDATE test with partitioned table
|
||||
#
|
||||
# We use SELECT FOR UPDATE to block the INSERT at the point where
|
||||
# it has found an existing tuple and is attempting to update it.
|
||||
# Then we can execute a conflicting update and verify the results.
|
||||
# Of course, this only works in READ COMMITTED mode, else we'd get an error.
|
||||
|
||||
setup
|
||||
{
|
||||
CREATE TABLE upsert (i int PRIMARY KEY, j int, k int) PARTITION BY RANGE (i);
|
||||
CREATE TABLE upsert_1 PARTITION OF upsert FOR VALUES FROM (1) TO (100);
|
||||
CREATE TABLE upsert_2 PARTITION OF upsert FOR VALUES FROM (100) TO (200);
|
||||
|
||||
INSERT INTO upsert VALUES (1, 10, 100);
|
||||
}
|
||||
|
||||
teardown
|
||||
{
|
||||
DROP TABLE upsert;
|
||||
}
|
||||
|
||||
session s1
|
||||
setup { BEGIN ISOLATION LEVEL READ COMMITTED; }
|
||||
step insert1 { INSERT INTO upsert VALUES (1, 11, 111)
|
||||
ON CONFLICT (i) DO UPDATE SET k = EXCLUDED.k; }
|
||||
step select1 { SELECT * FROM upsert; }
|
||||
step c1 { COMMIT; }
|
||||
|
||||
session s2
|
||||
setup { BEGIN ISOLATION LEVEL READ COMMITTED; }
|
||||
step lock2 { SELECT * FROM upsert WHERE i = 1 FOR UPDATE; }
|
||||
step update2a { UPDATE upsert SET i = i + 10 WHERE i = 1; }
|
||||
step update2b { UPDATE upsert SET i = i + 150 WHERE i = 1; }
|
||||
step delete2 { DELETE FROM upsert WHERE i = 1; }
|
||||
step c2 { COMMIT; }
|
||||
|
||||
# Test case where concurrent update moves the target row within the partition
|
||||
permutation lock2 insert1 update2a c2 select1 c1
|
||||
# Test case where concurrent update moves the target row to another partition
|
||||
permutation lock2 insert1 update2b c2 select1 c1
|
||||
# Test case where target row is concurrently deleted
|
||||
permutation lock2 insert1 delete2 c2 select1 c1
|
||||
Reference in New Issue
Block a user