Fix Assert failure in EXPLAIN ANALYZE MERGE with a concurrent update.HEADmaster

When instrumenting a MERGE command containing both WHEN NOT MATCHED BY
SOURCE and WHEN NOT MATCHED BY TARGET actions using EXPLAIN ANALYZE, a
concurrent update of the target relation could lead to an Assert
failure in show_modifytable_info(). In a non-assert build, this would
lead to an incorrect value for "skipped" tuples in the EXPLAIN output,
rather than a crash.

This could happen if the concurrent update caused a matched row to no
longer match, in which case ExecMerge() treats the single originally
matched row as a pair of not matched rows, and potentially executes 2
not-matched actions for the single source row. This could then lead to
a state where the number of rows processed by the ModifyTable node
exceeds the number of rows produced by its source node, causing
"skipped_path" in show_modifytable_info() to be negative, triggering
the Assert.

Fix this in ExecMergeMatched() by incrementing the instrumentation
tuple count on the source node whenever a concurrent update of this
kind is detected, if both kinds of merge actions exist, so that the
number of source rows matches the number of actions potentially
executed, and the "skipped" tuple count is correct.

Back-patch to v17, where support for WHEN NOT MATCHED BY SOURCE
actions was introduced.

Bug: #19111
Reported-by: Dilip Kumar <[email protected]>
Author: Dean Rasheed <[email protected]>
Reviewed-by: Dilip Kumar <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 17

3 files changed, 217 insertions, 0 deletions

diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 4c5647ac38a..00429326c34 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -3467,7 +3467,28 @@ lmerge_matched:
 
 								/* Switch lists, if necessary */
 								if (!*matched)
+								{
 									actionStates = mergeActions[MERGE_WHEN_NOT_MATCHED_BY_SOURCE];
+
+									/*
+									 * If we have both NOT MATCHED BY SOURCE
+									 * and NOT MATCHED BY TARGET actions (a
+									 * full join between the source and target
+									 * relations), the single previously
+									 * matched tuple from the outer plan node
+									 * is treated as two not matched tuples,
+									 * in the same way as if they had not
+									 * matched to start with.  Therefore, we
+									 * must adjust the outer plan node's tuple
+									 * count, if we're instrumenting the
+									 * query, to get the correct "skipped" row
+									 * count --- see show_modifytable_info().
+									 */
+									if (outerPlanState(mtstate)->instrument &&
+										mergeActions[MERGE_WHEN_NOT_MATCHED_BY_SOURCE] &&
+										mergeActions[MERGE_WHEN_NOT_MATCHED_BY_TARGET])
+										InstrUpdateTupleCount(outerPlanState(mtstate)->instrument, 1.0);
+								}
 							}
 
 							/*
diff --git a/src/test/isolation/expected/merge-update.out b/src/test/isolation/expected/merge-update.out
index 677263d1ec1..feceacf4818 100644
--- a/src/test/isolation/expected/merge-update.out
+++ b/src/test/isolation/expected/merge-update.out
@@ -57,6 +57,52 @@ key|val
 
 step c2: COMMIT;
 
+starting permutation: merge1 c1 explain_merge2a select2 c2
+step merge1: 
+  MERGE INTO target t
+  USING (SELECT 1 as key, 'merge1' as val) s
+  ON s.key = t.key
+  WHEN NOT MATCHED THEN
+	INSERT VALUES (s.key, s.val)
+  WHEN MATCHED THEN
+    UPDATE set key = t.key + 1, val = t.val || ' updated by ' || s.val;
+
+step c1: COMMIT;
+step explain_merge2a: 
+  SELECT explain_filter($$
+  EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF, BUFFERS OFF)
+  MERGE INTO target t
+  USING (SELECT 1 as key, 'merge2a' as val) s
+  ON s.key = t.key
+  WHEN NOT MATCHED THEN
+	INSERT VALUES (s.key, s.val)
+  WHEN MATCHED THEN
+	UPDATE set key = t.key + 1, val = t.val || ' updated by ' || s.val
+  WHEN NOT MATCHED BY SOURCE THEN
+	UPDATE set key = t.key + 1, val = t.val || ' source not matched by merge2a'
+  RETURNING merge_action(), old, new, t.*
+  $$);
+
+explain_filter                                             
+-----------------------------------------------------------
+Merge on target t (actual rows=2.00 loops=1)               
+  Tuples: inserted=1 updated=1                             
+  ->  Hash Full Join (actual rows=2.00 loops=1)            
+        Hash Cond: (t.key = (1))                           
+        ->  Seq Scan on target t (actual rows=1.00 loops=1)
+        ->  Hash (actual rows=1.00 loops=1)                
+              ->  Result (actual rows=1.00 loops=1)        
+(7 rows)
+
+step select2: SELECT * FROM target;
+key|val                                                   
+---+------------------------------------------------------
+  3|setup1 updated by merge1 source not matched by merge2a
+  1|merge2a                                               
+(2 rows)
+
+step c2: COMMIT;
+
 starting permutation: pa_merge1 c1 pa_merge2c_dup a2
 step pa_merge1: 
   MERGE INTO pa_target t
@@ -117,6 +163,53 @@ key|val
 
 step c2: COMMIT;
 
+starting permutation: merge1 explain_merge2a c1 select2 c2
+step merge1: 
+  MERGE INTO target t
+  USING (SELECT 1 as key, 'merge1' as val) s
+  ON s.key = t.key
+  WHEN NOT MATCHED THEN
+	INSERT VALUES (s.key, s.val)
+  WHEN MATCHED THEN
+    UPDATE set key = t.key + 1, val = t.val || ' updated by ' || s.val;
+
+step explain_merge2a: 
+  SELECT explain_filter($$
+  EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF, BUFFERS OFF)
+  MERGE INTO target t
+  USING (SELECT 1 as key, 'merge2a' as val) s
+  ON s.key = t.key
+  WHEN NOT MATCHED THEN
+	INSERT VALUES (s.key, s.val)
+  WHEN MATCHED THEN
+	UPDATE set key = t.key + 1, val = t.val || ' updated by ' || s.val
+  WHEN NOT MATCHED BY SOURCE THEN
+	UPDATE set key = t.key + 1, val = t.val || ' source not matched by merge2a'
+  RETURNING merge_action(), old, new, t.*
+  $$);
+ <waiting ...>
+step c1: COMMIT;
+step explain_merge2a: <... completed>
+explain_filter                                             
+-----------------------------------------------------------
+Merge on target t (actual rows=2.00 loops=1)               
+  Tuples: inserted=1 updated=1                             
+  ->  Hash Full Join (actual rows=2.00 loops=1)            
+        Hash Cond: (t.key = (1))                           
+        ->  Seq Scan on target t (actual rows=1.00 loops=1)
+        ->  Hash (actual rows=1.00 loops=1)                
+              ->  Result (actual rows=1.00 loops=1)        
+(7 rows)
+
+step select2: SELECT * FROM target;
+key|val                                                   
+---+------------------------------------------------------
+  3|setup1 updated by merge1 source not matched by merge2a
+  1|merge2a                                               
+(2 rows)
+
+step c2: COMMIT;
+
 starting permutation: merge1 merge2a a1 select2 c2
 step merge1: 
   MERGE INTO target t
@@ -253,6 +346,59 @@ key|val
 
 step c2: COMMIT;
 
+starting permutation: pa_merge1 explain_pa_merge2a c1 pa_select2 c2
+step pa_merge1: 
+  MERGE INTO pa_target t
+  USING (SELECT 1 as key, 'pa_merge1' as val) s
+  ON s.key = t.key
+  WHEN NOT MATCHED THEN
+	INSERT VALUES (s.key, s.val)
+  WHEN MATCHED THEN
+    UPDATE set val = t.val || ' updated by ' || s.val;
+
+step explain_pa_merge2a: 
+  SELECT explain_filter($$
+  EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF, BUFFERS OFF)
+  MERGE INTO pa_target t
+  USING (SELECT 1 as key, 'pa_merge2a' as val) s
+  ON s.key = t.key
+  WHEN NOT MATCHED THEN
+	INSERT VALUES (s.key, s.val)
+  WHEN MATCHED THEN
+	UPDATE set key = t.key + 1, val = t.val || ' updated by ' || s.val
+  WHEN NOT MATCHED BY SOURCE THEN
+	UPDATE set key = t.key + 1, val = t.val || ' source not matched by pa_merge2a'
+  RETURNING merge_action(), old, new, t.*
+  $$);
+ <waiting ...>
+step c1: COMMIT;
+step explain_pa_merge2a: <... completed>
+explain_filter                                                    
+------------------------------------------------------------------
+Merge on pa_target t (actual rows=2.00 loops=1)                   
+  Merge on part1 t_1                                              
+  Merge on part2 t_2                                              
+  Merge on part3 t_3                                              
+  Tuples: updated=2                                               
+  ->  Hash Full Join (actual rows=2.00 loops=1)                   
+        Hash Cond: (t.key = (1))                                  
+        ->  Append (actual rows=2.00 loops=1)                     
+              ->  Seq Scan on part1 t_1 (actual rows=1.00 loops=1)
+              ->  Seq Scan on part2 t_2 (actual rows=1.00 loops=1)
+              ->  Seq Scan on part3 t_3 (actual rows=0.00 loops=1)
+        ->  Hash (actual rows=1.00 loops=1)                       
+              ->  Result (actual rows=1.00 loops=1)               
+(13 rows)
+
+step pa_select2: SELECT * FROM pa_target;
+key|val                                               
+---+--------------------------------------------------
+  2|initial updated by pa_merge1 updated by pa_merge2a
+  3|initial source not matched by pa_merge2a          
+(2 rows)
+
+step c2: COMMIT;
+
 starting permutation: pa_merge2 pa_merge2a c1 pa_select2 c2
 step pa_merge2: 
   MERGE INTO pa_target t
diff --git a/src/test/isolation/specs/merge-update.spec b/src/test/isolation/specs/merge-update.spec
index c718ff646b2..771ee5b70cf 100644
--- a/src/test/isolation/specs/merge-update.spec
+++ b/src/test/isolation/specs/merge-update.spec
@@ -23,12 +23,27 @@ setup
 
   INSERT INTO pa_target VALUES (1, 'initial');
   INSERT INTO pa_target VALUES (2, 'initial');
+
+  CREATE FUNCTION explain_filter(text) RETURNS SETOF text
+  LANGUAGE plpgsql AS
+  $$
+  DECLARE
+    ln text;
+  BEGIN
+    FOR ln IN EXECUTE $1 LOOP
+      -- Ignore hash memory usage because it varies depending on the system
+      CONTINUE WHEN (ln ~ 'Memory Usage');
+      RETURN NEXT ln;
+    END LOOP;
+  END;
+  $$;
 }
 
 teardown
 {
   DROP TABLE target;
   DROP TABLE pa_target CASCADE;
+  DROP FUNCTION explain_filter;
 }
 
 session "s1"
@@ -97,6 +112,22 @@ step "merge2a"
 	UPDATE set key = t.key + 1, val = t.val || ' source not matched by merge2a'
   RETURNING merge_action(), old, new, t.*;
 }
+step "explain_merge2a"
+{
+  SELECT explain_filter($$
+  EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF, BUFFERS OFF)
+  MERGE INTO target t
+  USING (SELECT 1 as key, 'merge2a' as val) s
+  ON s.key = t.key
+  WHEN NOT MATCHED THEN
+	INSERT VALUES (s.key, s.val)
+  WHEN MATCHED THEN
+	UPDATE set key = t.key + 1, val = t.val || ' updated by ' || s.val
+  WHEN NOT MATCHED BY SOURCE THEN
+	UPDATE set key = t.key + 1, val = t.val || ' source not matched by merge2a'
+  RETURNING merge_action(), old, new, t.*
+  $$);
+}
 step "merge2b"
 {
   MERGE INTO target t
@@ -130,6 +161,22 @@ step "pa_merge2a"
 	UPDATE set key = t.key + 1, val = t.val || ' source not matched by pa_merge2a'
   RETURNING merge_action(), old, new, t.*;
 }
+step "explain_pa_merge2a"
+{
+  SELECT explain_filter($$
+  EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF, BUFFERS OFF)
+  MERGE INTO pa_target t
+  USING (SELECT 1 as key, 'pa_merge2a' as val) s
+  ON s.key = t.key
+  WHEN NOT MATCHED THEN
+	INSERT VALUES (s.key, s.val)
+  WHEN MATCHED THEN
+	UPDATE set key = t.key + 1, val = t.val || ' updated by ' || s.val
+  WHEN NOT MATCHED BY SOURCE THEN
+	UPDATE set key = t.key + 1, val = t.val || ' source not matched by pa_merge2a'
+  RETURNING merge_action(), old, new, t.*
+  $$);
+}
 # MERGE proceeds only if 'val' unchanged
 step "pa_merge2b_when"
 {
@@ -160,14 +207,17 @@ permutation "merge1" "c1" "select2" "c2"
 
 # One after the other, no concurrency
 permutation "merge1" "c1" "merge2a" "select2" "c2"
+permutation "merge1" "c1" "explain_merge2a" "select2" "c2"
 permutation "pa_merge1" "c1" "pa_merge2c_dup" "a2"
 
 # Now with concurrency
 permutation "merge1" "merge2a" "c1" "select2" "c2"
+permutation "merge1" "explain_merge2a" "c1" "select2" "c2"
 permutation "merge1" "merge2a" "a1" "select2" "c2"
 permutation "merge1" "merge2b" "c1" "select2" "c2"
 permutation "merge1" "merge2c" "c1" "select2" "c2"
 permutation "pa_merge1" "pa_merge2a" "c1" "pa_select2" "c2"
+permutation "pa_merge1" "explain_pa_merge2a" "c1" "pa_select2" "c2"
 permutation "pa_merge2" "pa_merge2a" "c1" "pa_select2" "c2" # fails
 permutation "pa_merge2" "c1" "pa_merge2a" "pa_select2" "c2" # succeeds
 permutation "pa_merge3" "pa_merge2b_when" "c1" "pa_select2" "c2" # WHEN not satisfied by updated tuple