From 7371c7fa5d5d618561725f81238539c8afb59e50 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Thu, 23 Jul 2026 20:18:45 +0000 Subject: [PATCH v2] Take SIREAD lock on rows read by ON CONFLICT DO SELECT The arbiter index probe runs with a dirty snapshot and the conflicting row is fetched with SnapshotAny, so no predicate lock is ever taken on the row that DO SELECT hands back, and the SELECT path writes nothing that would trigger conflict-in detection. A serializable transaction could therefore read the existing row without SSI recording the read, letting a concurrent writer of that row go unnoticed and allowing write skew. To fix, read the tuple with the query snapshot in ExecOnConflictSelect, making the read visible to SSI. This happens before the tuple lock is taken, which can succeed without noticing a concurrent writer (FOR KEY SHARE does not conflict with a non-key update), and before the WHERE clause is evaluated, so rows filtered out by it are covered as well. --- src/backend/executor/nodeModifyTable.c | 26 ++++++++++- .../expected/insert-conflict-do-select-2.out | 40 +++++++++++++++++ src/test/isolation/isolation_schedule | 1 + .../specs/insert-conflict-do-select-2.spec | 45 +++++++++++++++++++ 4 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 src/test/isolation/expected/insert-conflict-do-select-2.out create mode 100644 src/test/isolation/specs/insert-conflict-do-select-2.spec diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index ca954729f1e..d092eeea127 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -3293,6 +3293,7 @@ ExecOnConflictSelect(ModifyTableContext *context, ExprState *onConflictSelectWhere = resultRelInfo->ri_onConflict->oc_WhereClause; TupleTableSlot *existing = resultRelInfo->ri_onConflict->oc_Existing; LockClauseStrength lockStrength = resultRelInfo->ri_onConflict->oc_LockStrength; + bool fetched = false; /* * Parse analysis should have blocked ON CONFLICT for all system @@ -3301,10 +3302,33 @@ ExecOnConflictSelect(ModifyTableContext *context, */ Assert(!resultRelInfo->ri_needLockTagTuple); + /* + * At SERIALIZABLE, read the tuple with the query snapshot first: a read + * only becomes visible to SSI when it is made through an MVCC snapshot, + * and none of the other reads here qualifies -- the arbiter index probe + * uses a dirty snapshot, the fetch below SnapshotAny, and the tuple lock + * checks no snapshot at all -- while the SELECT path writes nothing that + * would make the conflict visible from the writer's side. This read + * must precede the tuple lock: the lock can succeed without noticing a + * concurrent writer, as FOR KEY SHARE does not conflict with a non-key + * update. + * + * A tuple invisible to the query snapshot was written either by this + * transaction, which needs no conflict detection against itself, or by + * a concurrent one, which ExecCheckTupleVisible below turns into a + * serialization failure. + */ + if (IsolationIsSerializable()) + fetched = table_tuple_fetch_row_version(relation, + conflictTid, + context->estate->es_snapshot, + existing); + /* Fetch/lock existing tuple, according to the requested lock strength */ if (lockStrength == LCS_NONE) { - if (!table_tuple_fetch_row_version(relation, + if (!fetched && + !table_tuple_fetch_row_version(relation, conflictTid, SnapshotAny, existing)) diff --git a/src/test/isolation/expected/insert-conflict-do-select-2.out b/src/test/isolation/expected/insert-conflict-do-select-2.out new file mode 100644 index 00000000000..b0af050897f --- /dev/null +++ b/src/test/isolation/expected/insert-conflict-do-select-2.out @@ -0,0 +1,40 @@ +Parsed test spec with 2 sessions + +starting permutation: select1 count2 update2 insert1 c1 c2 +step select1: INSERT INTO doselect_a VALUES (1, 99) ON CONFLICT (key) DO SELECT RETURNING val; +val +--- + 0 +(1 row) + +step count2: SELECT count(*) FROM doselect_b; +count +----- + 0 +(1 row) + +step update2: UPDATE doselect_a SET val = 1 WHERE key = 1; +step insert1: INSERT INTO doselect_b VALUES (1, 10); +step c1: COMMIT; +step c2: COMMIT; +ERROR: could not serialize access due to read/write dependencies among transactions + +starting permutation: select1_update count2 update2 insert1 c1 c2 +step select1_update: INSERT INTO doselect_a VALUES (1, 99) ON CONFLICT (key) DO SELECT FOR UPDATE RETURNING val; +val +--- + 0 +(1 row) + +step count2: SELECT count(*) FROM doselect_b; +count +----- + 0 +(1 row) + +step update2: UPDATE doselect_a SET val = 1 WHERE key = 1; +step insert1: INSERT INTO doselect_b VALUES (1, 10); +step c1: COMMIT; +step update2: <... completed> +ERROR: could not serialize access due to read/write dependencies among transactions +step c2: COMMIT; diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 6469aafa2e1..6a0539fbfa0 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -58,6 +58,7 @@ test: insert-conflict-do-update-3 test: insert-conflict-do-update-4 test: insert-conflict-specconflict test: insert-conflict-do-select +test: insert-conflict-do-select-2 test: merge-insert-update test: merge-delete test: merge-update diff --git a/src/test/isolation/specs/insert-conflict-do-select-2.spec b/src/test/isolation/specs/insert-conflict-do-select-2.spec new file mode 100644 index 00000000000..0c53a078d9e --- /dev/null +++ b/src/test/isolation/specs/insert-conflict-do-select-2.spec @@ -0,0 +1,45 @@ +# INSERT...ON CONFLICT DO SELECT test at SERIALIZABLE +# +# Returning the existing row is a read for SSI purposes, so a concurrent +# transaction writing that row must create a rw-antidependency. These +# permutations build the classic write-skew cycle: s1 reads a and writes +# b, while s2 reads b and writes a. One of the two transactions must +# fail with a serialization error. + +setup +{ + CREATE TABLE doselect_a (key int PRIMARY KEY, val int); + CREATE TABLE doselect_b (key int PRIMARY KEY, val int); + INSERT INTO doselect_a VALUES (1, 0); +} + +teardown +{ + DROP TABLE doselect_a, doselect_b; +} + +session s1 +setup +{ + BEGIN ISOLATION LEVEL SERIALIZABLE; +} +step select1 { INSERT INTO doselect_a VALUES (1, 99) ON CONFLICT (key) DO SELECT RETURNING val; } +step select1_update { INSERT INTO doselect_a VALUES (1, 99) ON CONFLICT (key) DO SELECT FOR UPDATE RETURNING val; } +step insert1 { INSERT INTO doselect_b VALUES (1, 10); } +step c1 { COMMIT; } + +session s2 +setup +{ + BEGIN ISOLATION LEVEL SERIALIZABLE; +} +step count2 { SELECT count(*) FROM doselect_b; } +step update2 { UPDATE doselect_a SET val = 1 WHERE key = 1; } +step c2 { COMMIT; } + +# Write skew with DO SELECT: s2 must fail to commit +permutation select1 count2 update2 insert1 c1 c2 + +# Write skew with DO SELECT FOR UPDATE: update2 blocks on the tuple lock +# and must fail once s1 commits +permutation select1_update count2 update2 insert1 c1 c2 -- 2.54.0