Fix concurrent update issue with MERGE. REL_16_STABLE github/REL_16_STABLE
authorDean Rasheed <dean.a.rasheed@gmail.com>
Fri, 5 Sep 2025 07:23:57 +0000 (08:23 +0100)
committerDean Rasheed <dean.a.rasheed@gmail.com>
Fri, 5 Sep 2025 07:23:57 +0000 (08:23 +0100)
When executing a MERGE UPDATE action, if there is more than one
concurrent update of the target row, the lock-and-retry code would
sometimes incorrectly identify the latest version of the target tuple,
leading to incorrect results.

This was caused by using the ctid field from the TM_FailureData
returned by table_tuple_lock() in a case where the result was TM_Ok,
which is unsafe because the TM_FailureData struct is not guaranteed to
be fully populated in that case. Instead, it should use the tupleid
passed to (and updated by) table_tuple_lock().

To reduce the chances of similar errors in the future, improve the
commentary for table_tuple_lock() and TM_FailureData to make it
clearer that table_tuple_lock() updates the tid passed to it, and most
fields of TM_FailureData should not be relied on in non-failure cases.
An exception to this is the "traversed" field, which is set in both
success and failure cases.

Reported-by: Dmitry <dsy.075@yandex.ru>
Author: Yugo Nagata <nagata@sraoss.co.jp>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/1570d30e-2b95-4239-b9c3-f7bf2f2f8556@yandex.ru
Backpatch-through: 15

src/backend/executor/nodeModifyTable.c
src/include/access/tableam.h
src/test/isolation/expected/merge-match-recheck.out
src/test/isolation/specs/merge-match-recheck.spec

index e0dc62d31be83596885f3b1a5453348d5aabe877..6597356c018f914868b6fc5c33580b543674ac67 100644 (file)
@@ -3146,7 +3146,7 @@ lmerge_matched:
                             * the tuple moved, and setting our current
                             * resultRelInfo to that.
                             */
-                           if (ItemPointerIndicatesMovedPartitions(&context->tmfd.ctid))
+                           if (ItemPointerIndicatesMovedPartitions(tupleid))
                                ereport(ERROR,
                                        (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
                                         errmsg("tuple to be deleted was already moved to another partition due to concurrent update")));
@@ -3158,14 +3158,14 @@ lmerge_matched:
                             * that the first qualifying WHEN MATCHED action
                             * is executed.
                             *
-                            * Update tupleid to that of the new tuple, for
-                            * the refetch we do at the top.
+                            * tupleid has been updated to that of the new
+                            * tuple, as required for the refetch we do at the
+                            * top.
                             */
                            if (resultRelInfo->ri_needLockTagTuple)
                                UnlockTuple(resultRelInfo->ri_RelationDesc,
                                            &lockedtid,
                                            InplaceUpdateTupleLock);
-                           ItemPointerCopy(&context->tmfd.ctid, tupleid);
                            goto lmerge_matched;
 
                        case TM_Deleted:
index 2e919d569af3136e6bf9b69ea1d1aeb661a2af29..1a399e244601a58d1bb44518b77b49fad35d7d1c 100644 (file)
@@ -121,7 +121,9 @@ typedef enum TU_UpdateIndexes
 /*
  * When table_tuple_update, table_tuple_delete, or table_tuple_lock fail
  * because the target tuple is already outdated, they fill in this struct to
- * provide information to the caller about what happened.
+ * provide information to the caller about what happened. When those functions
+ * succeed, the contents of this struct should not be relied upon, except for
+ * `traversed`, which may be set in both success and failure cases.
  *
  * ctid is the target's ctid link: it is the same as the target's TID if the
  * target was deleted, or the location of the replacement tuple if the target
@@ -137,6 +139,9 @@ typedef enum TU_UpdateIndexes
  * tuple); otherwise cmax is zero.  (We make this restriction because
  * HeapTupleHeaderGetCmax doesn't work for tuples outdated in other
  * transactions.)
+ *
+ * traversed indicates if an update chain was followed in order to try to lock
+ * the target tuple.  (This may be set in both success and failure cases.)
  */
 typedef struct TM_FailureData
 {
@@ -1544,7 +1549,7 @@ table_tuple_update(Relation rel, ItemPointer otid, TupleTableSlot *slot,
  *
  * Input parameters:
  * relation: relation containing tuple (caller must hold suitable lock)
- * tid: TID of tuple to lock
+ * tid: TID of tuple to lock (updated if an update chain was followed)
  * snapshot: snapshot to use for visibility determinations
  * cid: current command ID (used for visibility test, and stored into
  *     tuple's cmax if lock is successful)
@@ -1569,8 +1574,10 @@ table_tuple_update(Relation rel, ItemPointer otid, TupleTableSlot *slot,
  * TM_WouldBlock: lock couldn't be acquired and wait_policy is skip
  *
  * In the failure cases other than TM_Invisible and TM_Deleted, the routine
- * fills *tmfd with the tuple's t_ctid, t_xmax, and, if possible, t_cmax.  See
- * comments for struct TM_FailureData for additional info.
+ * fills *tmfd with the tuple's t_ctid, t_xmax, and, if possible, t_cmax.
+ * Additionally, in both success and failure cases, tmfd->traversed is set if
+ * an update chain was followed.  See comments for struct TM_FailureData for
+ * additional info.
  */
 static inline TM_Result
 table_tuple_lock(Relation rel, ItemPointer tid, Snapshot snapshot,
index 9a44a5959270b796aa4b1661ab06ee833c695cc9..717f12b3b90ebfb8be47ab084a3ebe5021f8a569 100644 (file)
@@ -262,6 +262,142 @@ key|balance|status|val
 
 step c1: COMMIT;
 
+starting permutation: update1 update6 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 update6: UPDATE target t SET balance = balance - 100, val = t.val || ' updated by update6' 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';
+ <waiting ...>
+step c2: COMMIT;
+step merge_bal: <... completed>
+step select1: SELECT * FROM target;
+key|balance|status|val                                              
+---+-------+------+-------------------------------------------------
+  1|    140|s1    |setup updated by update1 updated by update6 when1
+(1 row)
+
+step c1: COMMIT;
+
+starting permutation: update1_pa update6_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 update6_pa: UPDATE target_pa t SET balance = balance - 100, val = t.val || ' updated by update6_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';
+ <waiting ...>
+step c2: 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 update6_pa when1
+(1 row)
+
+step c1: COMMIT;
+
+starting permutation: update1_tg update6_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;
+s2: NOTICE:  Update: (1,170,s1,"setup updated by update1_tg") -> (1,70,s1,"setup updated by update1_tg updated by update6_tg")
+step update6_tg: UPDATE target_tg t SET balance = balance - 100, val = t.val || ' updated by update6_tg' WHERE t.key = 1;
+step merge_bal_tg: 
+  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';
+ <waiting ...>
+step c2: COMMIT;
+s1: NOTICE:  Update: (1,70,s1,"setup updated by update1_tg updated by update6_tg") -> (1,140,s1,"setup updated by update1_tg updated by update6_tg when1")
+step merge_bal_tg: <... completed>
+step select1_tg: SELECT * FROM target_tg;
+key|balance|status|val                                                    
+---+-------+------+-------------------------------------------------------
+  1|    140|s1    |setup updated by update1_tg updated by update6_tg when1
+(1 row)
+
+step c1: COMMIT;
+
+starting permutation: update7 update6 merge_bal c2 select1 c1
+step update7: UPDATE target t SET balance = 350, val = t.val || ' updated by update7' WHERE t.key = 1;
+step update6: UPDATE target t SET balance = balance - 100, val = t.val || ' updated by update6' 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';
+ <waiting ...>
+step c2: COMMIT;
+step merge_bal: <... completed>
+step select1: SELECT * FROM target;
+key|balance|status|val                                              
+---+-------+------+-------------------------------------------------
+  1|   2000|s1    |setup updated by update7 updated by update6 when3
+(1 row)
+
+step c1: COMMIT;
+
+starting permutation: update1_pa_move merge_bal_pa c2 c1
+step update1_pa_move: UPDATE target_pa t SET balance = 210, val = t.val || ' updated by update1_pa_move' 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';
+ <waiting ...>
+step c2: COMMIT;
+step merge_bal_pa: <... completed>
+ERROR:  tuple to be locked was already moved to another partition due to concurrent update
+step c1: COMMIT;
+
+starting permutation: update1_pa update1_pa_move merge_bal_pa c2 c1
+step update1_pa: UPDATE target_pa t SET balance = balance + 10, val = t.val || ' updated by update1_pa' WHERE t.key = 1;
+step update1_pa_move: UPDATE target_pa t SET balance = 210, val = t.val || ' updated by update1_pa_move' 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';
+ <waiting ...>
+step c2: COMMIT;
+step merge_bal_pa: <... completed>
+ERROR:  tuple to be locked was already moved to another partition due to concurrent update
+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: 
index 298b2bfdcd6099179b3e904881b5f25a3bbc67a1..57e32d7f80f21272c08dfec6ab66caae12ac4d7f 100644 (file)
@@ -142,6 +142,8 @@ 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_pa_move" { UPDATE target_pa t SET balance = 210, val = t.val || ' updated by update1_pa_move' 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; }
@@ -149,6 +151,10 @@ step "update3" { UPDATE target t SET status = 's3', val = t.val || ' updated by
 step "update3_tg" { UPDATE target_tg t SET status = 's3', val = t.val || ' updated by update3_tg' WHERE t.key = 1; }
 step "update5" { UPDATE target t SET status = 's5', val = t.val || ' updated by update5' WHERE t.key = 1; }
 step "update5_tg" { UPDATE target_tg t SET status = 's5', val = t.val || ' updated by update5_tg' WHERE t.key = 1; }
+step "update6" { UPDATE target t SET balance = balance - 100, val = t.val || ' updated by update6' WHERE t.key = 1; }
+step "update6_pa" { UPDATE target_pa t SET balance = balance - 100, val = t.val || ' updated by update6_pa' WHERE t.key = 1; }
+step "update6_tg" { UPDATE target_tg t SET balance = balance - 100, val = t.val || ' updated by update6_tg' WHERE t.key = 1; }
+step "update7" { UPDATE target t SET balance = 350, val = t.val || ' updated by update7' WHERE t.key = 1; }
 step "update_bal1" { UPDATE target t SET balance = 50, val = t.val || ' updated by update_bal1' WHERE t.key = 1; }
 step "update_bal1_pa" { UPDATE target_pa t SET balance = 50, val = t.val || ' updated by update_bal1_pa' WHERE t.key = 1; }
 step "update_bal1_tg" { UPDATE target_tg t SET balance = 50, val = t.val || ' updated by update_bal1_tg' WHERE t.key = 1; }
@@ -175,6 +181,18 @@ 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 row concurrently updated twice and rechecks WHEN conditions, different check passes, so final balance = 140
+permutation "update1" "update6" "merge_bal" "c2" "select1" "c1"
+permutation "update1_pa" "update6_pa" "merge_bal_pa" "c2" "select1_pa" "c1"
+permutation "update1_tg" "update6_tg" "merge_bal_tg" "c2" "select1_tg" "c1"
+
+# merge_bal sees row concurrently updated twice, first update would cause all checks to fail, second update causes different check to pass, so final balance = 2000
+permutation "update7" "update6" "merge_bal" "c2" "select1" "c1"
+
+# merge_bal sees concurrently updated row moved to new partition, so fails
+permutation "update1_pa_move" "merge_bal_pa" "c2" "c1"
+permutation "update1_pa" "update1_pa_move" "merge_bal_pa" "c2" "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"