From a5490eaf22e3c6d0145c4daf3272a7c0bbd6dc6f Mon Sep 17 00:00:00 2001 From: Yugo Nagata Date: Mon, 25 Aug 2025 01:57:25 +0900 Subject: [PATCH v2] Fix misuse of TM_FailureData.ctid in ExecMergeMatched Previously, the ctid field of TM_FailureData returned by table_tuple_lock() was used as a reference to the latest version of oldtuple, but this was not always correct. Instead, the tupleid passed to table_tuple_lock() should be used. This misuse caused incorrect UPDATE results when more than two MERGE commands were executed concurrently. --- src/backend/executor/nodeModifyTable.c | 7 +- .../expected/merge-match-recheck.out | 175 +++++++++++++++++- .../isolation/specs/merge-match-recheck.spec | 18 ++ 3 files changed, 196 insertions(+), 4 deletions(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 7c6c2c1f6e4..ea1457ce3bf 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -3450,12 +3450,13 @@ lmerge_matched: if (ItemPointerIsValid(&lockedtid)) UnlockTuple(resultRelInfo->ri_RelationDesc, &lockedtid, InplaceUpdateTupleLock); - LockTuple(resultRelInfo->ri_RelationDesc, &context->tmfd.ctid, + LockTuple(resultRelInfo->ri_RelationDesc, tupleid, InplaceUpdateTupleLock); - lockedtid = context->tmfd.ctid; + lockedtid = *tupleid; } + if (!table_tuple_fetch_row_version(resultRelationDesc, - &context->tmfd.ctid, + tupleid, SnapshotAny, resultRelInfo->ri_oldTupleSlot)) elog(ERROR, "failed to fetch the target tuple"); diff --git a/src/test/isolation/expected/merge-match-recheck.out b/src/test/isolation/expected/merge-match-recheck.out index 90300f1db5a..5f91b221a21 100644 --- a/src/test/isolation/expected/merge-match-recheck.out +++ b/src/test/isolation/expected/merge-match-recheck.out @@ -1,4 +1,4 @@ -Parsed test spec with 2 sessions +Parsed test spec with 3 sessions starting permutation: update1 merge_status c2 select1 c1 step update1: UPDATE target t SET balance = balance + 10, val = t.val || ' updated by update1' WHERE t.key = 1; @@ -271,6 +271,179 @@ key|balance|status|val step c1: COMMIT; +starting permutation: update1 merge_bal c2 select1 c1 +step update1: UPDATE target t SET balance = balance + 10, val = t.val || ' updated by update1' WHERE t.key = 1; +step merge_bal: + MERGE INTO target t + USING (SELECT 1 as key) s + ON s.key = t.key + WHEN MATCHED AND balance < 100 THEN + UPDATE SET balance = balance * 2, val = t.val || ' when1' + WHEN MATCHED AND balance < 200 THEN + UPDATE SET balance = balance * 4, val = t.val || ' when2' + WHEN MATCHED AND balance < 300 THEN + UPDATE SET balance = balance * 8, val = t.val || ' when3'; + +step c2: COMMIT; +step merge_bal: <... completed> +step select1: SELECT * FROM target; +key|balance|status|val +---+-------+------+------------------------------ + 1| 680|s1 |setup updated by update1 when2 +(1 row) + +step c1: COMMIT; + +starting permutation: update1_pa merge_bal_pa c2 select1_pa c1 +step update1_pa: UPDATE target_pa t SET balance = balance + 10, val = t.val || ' updated by update1_pa' WHERE t.key = 1; +step merge_bal_pa: + MERGE INTO target_pa t + USING (SELECT 1 as key) s + ON s.key = t.key + WHEN MATCHED AND balance < 100 THEN + UPDATE SET balance = balance * 2, val = t.val || ' when1' + WHEN MATCHED AND balance < 200 THEN + UPDATE SET balance = balance * 4, val = t.val || ' when2' + WHEN MATCHED AND balance < 300 THEN + UPDATE SET balance = balance * 8, val = t.val || ' when3'; + +step c2: COMMIT; +step merge_bal_pa: <... completed> +step select1_pa: SELECT * FROM target_pa; +key|balance|status|val +---+-------+------+--------------------------------- + 1| 680|s1 |setup updated by update1_pa when2 +(1 row) + +step c1: COMMIT; + +starting permutation: update1_tg merge_bal_tg c2 select1_tg c1 +s2: NOTICE: Update: (1,160,s1,setup) -> (1,170,s1,"setup updated by update1_tg") +step update1_tg: UPDATE target_tg t SET balance = balance + 10, val = t.val || ' updated by update1_tg' WHERE t.key = 1; +step merge_bal_tg: + WITH t AS ( + MERGE INTO target_tg t + USING (SELECT 1 as key) s + ON s.key = t.key + WHEN MATCHED AND balance < 100 THEN + UPDATE SET balance = balance * 2, val = t.val || ' when1' + WHEN MATCHED AND balance < 200 THEN + UPDATE SET balance = balance * 4, val = t.val || ' when2' + WHEN MATCHED AND balance < 300 THEN + UPDATE SET balance = balance * 8, val = t.val || ' when3' + RETURNING t.* + ) + SELECT * FROM t; + +step c2: COMMIT; +s1: NOTICE: Update: (1,170,s1,"setup updated by update1_tg") -> (1,680,s1,"setup updated by update1_tg when2") +step merge_bal_tg: <... completed> +key|balance|status|val +---+-------+------+--------------------------------- + 1| 680|s1 |setup updated by update1_tg when2 +(1 row) + +step select1_tg: SELECT * FROM target_tg; +key|balance|status|val +---+-------+------+--------------------------------- + 1| 680|s1 |setup updated by update1_tg when2 +(1 row) + +step c1: COMMIT; + +starting permutation: update1 b3 update1b merge_bal c2 c3 select1 c1 +step update1: UPDATE target t SET balance = balance + 10, val = t.val || ' updated by update1' WHERE t.key = 1; +step b3: BEGIN ISOLATION LEVEL READ COMMITTED; +step update1b: UPDATE target t SET balance = balance - 100, val = t.val || ' updated by update1b' WHERE t.key = 1; +step merge_bal: + MERGE INTO target t + USING (SELECT 1 as key) s + ON s.key = t.key + WHEN MATCHED AND balance < 100 THEN + UPDATE SET balance = balance * 2, val = t.val || ' when1' + WHEN MATCHED AND balance < 200 THEN + UPDATE SET balance = balance * 4, val = t.val || ' when2' + WHEN MATCHED AND balance < 300 THEN + UPDATE SET balance = balance * 8, val = t.val || ' when3'; + +step c2: COMMIT; +step update1b: <... completed> +step c3: COMMIT; +step merge_bal: <... completed> +step select1: SELECT * FROM target; +key|balance|status|val +---+-------+------+-------------------------------------------------- + 1| 140|s1 |setup updated by update1 updated by update1b when1 +(1 row) + +step c1: COMMIT; + +starting permutation: update1_pa b3 update1b_pa merge_bal_pa c2 c3 select1_pa c1 +step update1_pa: UPDATE target_pa t SET balance = balance + 10, val = t.val || ' updated by update1_pa' WHERE t.key = 1; +step b3: BEGIN ISOLATION LEVEL READ COMMITTED; +step update1b_pa: UPDATE target_pa t SET balance = balance - 100, val = t.val || ' updated by update1b_pa' WHERE t.key = 1; +step merge_bal_pa: + MERGE INTO target_pa t + USING (SELECT 1 as key) s + ON s.key = t.key + WHEN MATCHED AND balance < 100 THEN + UPDATE SET balance = balance * 2, val = t.val || ' when1' + WHEN MATCHED AND balance < 200 THEN + UPDATE SET balance = balance * 4, val = t.val || ' when2' + WHEN MATCHED AND balance < 300 THEN + UPDATE SET balance = balance * 8, val = t.val || ' when3'; + +step c2: COMMIT; +step update1b_pa: <... completed> +step c3: COMMIT; +step merge_bal_pa: <... completed> +step select1_pa: SELECT * FROM target_pa; +key|balance|status|val +---+-------+------+-------------------------------------------------------- + 1| 140|s1 |setup updated by update1_pa updated by update1b_pa when1 +(1 row) + +step c1: COMMIT; + +starting permutation: update1_tg b3 update1b_tg merge_bal_tg c2 c3 select1_tg c1 +s2: NOTICE: Update: (1,160,s1,setup) -> (1,170,s1,"setup updated by update1_tg") +step update1_tg: UPDATE target_tg t SET balance = balance + 10, val = t.val || ' updated by update1_tg' WHERE t.key = 1; +step b3: BEGIN ISOLATION LEVEL READ COMMITTED; +step update1b_tg: UPDATE target_tg t SET balance = balance - 100, val = t.val || ' updated by update1b_tg' WHERE t.key = 1; +step merge_bal_tg: + WITH t AS ( + MERGE INTO target_tg t + USING (SELECT 1 as key) s + ON s.key = t.key + WHEN MATCHED AND balance < 100 THEN + UPDATE SET balance = balance * 2, val = t.val || ' when1' + WHEN MATCHED AND balance < 200 THEN + UPDATE SET balance = balance * 4, val = t.val || ' when2' + WHEN MATCHED AND balance < 300 THEN + UPDATE SET balance = balance * 8, val = t.val || ' when3' + RETURNING t.* + ) + SELECT * FROM t; + +step c2: COMMIT; +s3: NOTICE: Update: (1,170,s1,"setup updated by update1_tg") -> (1,70,s1,"setup updated by update1_tg updated by update1b_tg") +step update1b_tg: <... completed> +step c3: COMMIT; +s1: NOTICE: Update: (1,70,s1,"setup updated by update1_tg updated by update1b_tg") -> (1,140,s1,"setup updated by update1_tg updated by update1b_tg when1") +step merge_bal_tg: <... completed> +key|balance|status|val +---+-------+------+-------------------------------------------------------- + 1| 140|s1 |setup updated by update1_tg updated by update1b_tg when1 +(1 row) + +step select1_tg: SELECT * FROM target_tg; +key|balance|status|val +---+-------+------+-------------------------------------------------------- + 1| 140|s1 |setup updated by update1_tg updated by update1b_tg when1 +(1 row) + +step c1: COMMIT; + starting permutation: update1 merge_delete c2 select1 c1 step update1: UPDATE target t SET balance = balance + 10, val = t.val || ' updated by update1' WHERE t.key = 1; step merge_delete: diff --git a/src/test/isolation/specs/merge-match-recheck.spec b/src/test/isolation/specs/merge-match-recheck.spec index 15226e40c9e..6eba77aca13 100644 --- a/src/test/isolation/specs/merge-match-recheck.spec +++ b/src/test/isolation/specs/merge-match-recheck.spec @@ -146,6 +146,7 @@ setup BEGIN ISOLATION LEVEL READ COMMITTED; } step "update1" { UPDATE target t SET balance = balance + 10, val = t.val || ' updated by update1' WHERE t.key = 1; } +step "update1_pa" { UPDATE target_pa t SET balance = balance + 10, val = t.val || ' updated by update1_pa' WHERE t.key = 1; } step "update1_tg" { UPDATE target_tg t SET balance = balance + 10, val = t.val || ' updated by update1_tg' WHERE t.key = 1; } step "update2" { UPDATE target t SET status = 's2', val = t.val || ' updated by update2' WHERE t.key = 1; } step "update2_tg" { UPDATE target_tg t SET status = 's2', val = t.val || ' updated by update2_tg' WHERE t.key = 1; } @@ -158,6 +159,13 @@ step "update_bal1_pa" { UPDATE target_pa t SET balance = 50, val = t.val || ' up step "update_bal1_tg" { UPDATE target_tg t SET balance = 50, val = t.val || ' updated by update_bal1_tg' WHERE t.key = 1; } step "c2" { COMMIT; } +session "s3" +step "b3" { BEGIN ISOLATION LEVEL READ COMMITTED; } +step "update1b" { UPDATE target t SET balance = balance - 100, val = t.val || ' updated by update1b' WHERE t.key = 1; } +step "update1b_pa" { UPDATE target_pa t SET balance = balance - 100, val = t.val || ' updated by update1b_pa' WHERE t.key = 1; } +step "update1b_tg" { UPDATE target_tg t SET balance = balance - 100, val = t.val || ' updated by update1b_tg' WHERE t.key = 1; } +step "c3" { COMMIT; } + # merge_status sees concurrently updated row and rechecks WHEN conditions, but recheck passes and final status = 's2' permutation "update1" "merge_status" "c2" "select1" "c1" permutation "update1_tg" "merge_status_tg" "c2" "select1_tg" "c1" @@ -179,6 +187,16 @@ permutation "update_bal1" "merge_bal" "c2" "select1" "c1" permutation "update_bal1_pa" "merge_bal_pa" "c2" "select1_pa" "c1" permutation "update_bal1_tg" "merge_bal_tg" "c2" "select1_tg" "c1" +# merge_bal sees concurrently updated row and rechecks WHEN conditions, recheck passes, so final balance = 680 +permutation "update1" "merge_bal" "c2" "select1" "c1" +permutation "update1_pa" "merge_bal_pa" "c2" "select1_pa" "c1" +permutation "update1_tg" "merge_bal_tg" "c2" "select1_tg" "c1" + +# merge_bal sees row concurrently updated twice and rechecks WHEN conditions, recheck fails, so final balance = 140 +permutation "update1" "b3" "update1b" "merge_bal" "c2" "c3" "select1" "c1" +permutation "update1_pa" "b3" "update1b_pa" "merge_bal_pa" "c2" "c3" "select1_pa" "c1" +permutation "update1_tg" "b3" "update1b_tg" "merge_bal_tg" "c2" "c3" "select1_tg" "c1" + # merge_delete sees concurrently updated row and rechecks WHEN conditions, but recheck passes and row is deleted permutation "update1" "merge_delete" "c2" "select1" "c1" permutation "update1_tg" "merge_delete_tg" "c2" "select1_tg" "c1" -- 2.43.0