Hi,
While working on the fix for MERGE on a partitioned table with a
subplan in the UPDATE action, I noticed that the bug dated back to
the original MERGE implementation. That prompted me to use an LLM
to look for similar omissions in the MERGE executor code. It
identified the following issue, which I reproduced.
CREATE TABLE mx_t (i int, v int) PARTITION BY LIST (i);
CREATE TABLE mx_t1 PARTITION OF mx_t FOR VALUES IN (1);
CREATE TABLE mx_t2 PARTITION OF mx_t FOR VALUES IN (2);
INSERT INTO mx_t VALUES (2, 20);
CREATE FUNCTION mx_dump_update() RETURNS trigger LANGUAGE plpgsql AS $$
BEGIN
RAISE NOTICE 'old: %, new: %',
(SELECT string_agg(ot::text, ', ') FROM ot),
(SELECT string_agg(nt::text, ', ') FROM nt);
RETURN NULL;
END;
$$;
CREATE TRIGGER mx_upd AFTER UPDATE ON mx_t
REFERENCING OLD TABLE AS ot NEW TABLE AS nt
FOR EACH STATEMENT EXECUTE FUNCTION mx_dump_update();
MERGE INTO mx_t USING (VALUES (2)) s(i) ON mx_t.i = s.i
WHEN MATCHED THEN UPDATE SET i = 1, v = 999;
The MERGE updates one row, but the UPDATE trigger reports both
transition tables as empty. A plain UPDATE mx_t SET i = 1, v = 999
WHERE i = 2, run separately on the same initial data, reports
(2,20) in the old table and (1,999) in the new table.
A cross-partition UPDATE moves the row via a DELETE and an INSERT.
The transition capture code in ExecDeleteEpilogue() and ExecInsert()
checks for CMD_UPDATE, but the statement operation is CMD_MERGE
when a MERGE action performs the move. As a result, the row versions
are omitted from the UPDATE transition tables. If the MERGE also has
DELETE and INSERT actions with transition tables, the moved row
versions can be captured in those tables instead.
The attached patch recognizes both halves of the move as part of
the UPDATE action. It also keeps moved rows out of the DELETE and
INSERT transition tables when no UPDATE transition table is requested.
The patch includes regression tests for both cases.
--
Thanks,
Tender Wang
Subject: [PATCH] Fix transition tables for cross-partition MERGE updates
A MERGE UPDATE that moves a row executes a physical DELETE and INSERT.
Capture both halves in the UPDATE transition tables and exclude them
from the DELETE and INSERT transition tables, even when the UPDATE
transition tables are not requested.
diff --git a/src/backend/executor/nodeModifyTable.c
b/src/backend/executor/nodeModifyTable.c
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1285,22 +1285,21 @@
* because they happen on different tables.
*/
ar_insert_trig_tcs = mtstate->mt_transition_capture;
- if (mtstate->operation == CMD_UPDATE && mtstate->mt_transition_capture
- && mtstate->mt_transition_capture->tcs_update_new_table)
- {
- ExecARUpdateTriggers(estate, resultRelInfo,
- NULL, NULL,
- NULL,
- NULL,
- slot,
- NULL,
-
mtstate->mt_transition_capture,
- false);
-
- /*
- * We've already captured the NEW TABLE row, so make sure any AR
- * INSERT trigger fired below doesn't capture it again.
- */
+ if (mtstate->operation == CMD_UPDATE ||
+ (mtstate->operation == CMD_MERGE &&
+ mtstate->mt_merge_action->mas_action->commandType ==
CMD_UPDATE))
+ {
+ if (ar_insert_trig_tcs &&
ar_insert_trig_tcs->tcs_update_new_table)
+ ExecARUpdateTriggers(estate, resultRelInfo,
+ NULL, NULL,
+ NULL,
+ NULL,
+ slot,
+ NULL,
+
mtstate->mt_transition_capture,
+ false);
+
+ /* A moved row does not belong in the INSERT transition table.
*/
ar_insert_trig_tcs = NULL;
}
@@ -1560,19 +1559,16 @@
* because they happen on different tables.
*/
ar_delete_trig_tcs = mtstate->mt_transition_capture;
- if (mtstate->operation == CMD_UPDATE && mtstate->mt_transition_capture
&&
- mtstate->mt_transition_capture->tcs_update_old_table)
- {
- ExecARUpdateTriggers(estate, resultRelInfo,
- NULL, NULL,
- tupleid, oldtuple,
- NULL, NULL,
mtstate->mt_transition_capture,
- false);
-
- /*
- * We've already captured the OLD TABLE row, so make sure any AR
- * DELETE trigger fired below doesn't capture it again.
- */
+ if (changingPart)
+ {
+ if (ar_delete_trig_tcs &&
ar_delete_trig_tcs->tcs_update_old_table)
+ ExecARUpdateTriggers(estate, resultRelInfo,
+ NULL, NULL,
+ tupleid,
oldtuple,
+ NULL, NULL,
mtstate->mt_transition_capture,
+ false);
+
+ /* A moved row does not belong in the DELETE transition table.
*/
ar_delete_trig_tcs = NULL;
}
diff --git a/src/test/regress/sql/triggers.sql
b/src/test/regress/sql/triggers.sql
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -2499,6 +2499,36 @@
drop table merge_source_table, merge_target_table;
+-- A MERGE UPDATE that moves a row must capture it as an update.
+create table merge_move (a int, b text) partition by list (a);
+create table merge_move_p1 partition of merge_move for values in (1, 2);
+create table merge_move_p2 partition of merge_move for values in (3, 4);
+insert into merge_move values (1, 'move'), (2, 'delete');
+create trigger move_ins after insert on merge_move
+ referencing new table as new_table
+ for each statement execute function dump_insert();
+create trigger move_upd after update on merge_move
+ referencing old table as old_table new table as new_table
+ for each statement execute function dump_update();
+create trigger move_del after delete on merge_move
+ referencing old table as old_table
+ for each statement execute function dump_delete();
+
+merge into merge_move t using (values (1), (2), (4)) s(a) on t.a = s.a
+when matched and s.a = 1 then update set a = 3
+when matched then delete
+when not matched then insert values (s.a, 'insert');
+
+-- The move must stay out of INSERT and DELETE tables even without an
+-- UPDATE transition table.
+drop trigger move_upd on merge_move;
+merge into merge_move t using (values (3), (4), (2)) s(a) on t.a = s.a
+when matched and s.a = 3 then update set a = 1
+when matched then delete
+when not matched then insert values (s.a, 'insert');
+
+drop table merge_move;
+
-- cleanup
drop function dump_insert();
drop function dump_update();
diff --git a/src/test/regress/expected/triggers.out
b/src/test/regress/expected/triggers.out
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -3336,6 +3336,37 @@
NOTICE: trigger = merge_target_table_update_trig, old table = (1,"initial1
updated by merge"), (2,"initial2 updated by merge"), new table = (1,"initial1
updated by merge updated again by merge"), (2,"initial2 updated by merge
updated again by merge")
NOTICE: trigger = merge_target_table_insert_trig, new table = (3,initial3),
(4,initial4)
drop table merge_source_table, merge_target_table;
+-- A MERGE UPDATE that moves a row must capture it as an update.
+create table merge_move (a int, b text) partition by list (a);
+create table merge_move_p1 partition of merge_move for values in (1, 2);
+create table merge_move_p2 partition of merge_move for values in (3, 4);
+insert into merge_move values (1, 'move'), (2, 'delete');
+create trigger move_ins after insert on merge_move
+ referencing new table as new_table
+ for each statement execute function dump_insert();
+create trigger move_upd after update on merge_move
+ referencing old table as old_table new table as new_table
+ for each statement execute function dump_update();
+create trigger move_del after delete on merge_move
+ referencing old table as old_table
+ for each statement execute function dump_delete();
+merge into merge_move t using (values (1), (2), (4)) s(a) on t.a = s.a
+when matched and s.a = 1 then update set a = 3
+when matched then delete
+when not matched then insert values (s.a, 'insert');
+NOTICE: trigger = move_del, old table = (2,delete)
+NOTICE: trigger = move_upd, old table = (1,move), new table = (3,move)
+NOTICE: trigger = move_ins, new table = (4,insert)
+-- The move must stay out of INSERT and DELETE tables even without an
+-- UPDATE transition table.
+drop trigger move_upd on merge_move;
+merge into merge_move t using (values (3), (4), (2)) s(a) on t.a = s.a
+when matched and s.a = 3 then update set a = 1
+when matched then delete
+when not matched then insert values (s.a, 'insert');
+NOTICE: trigger = move_del, old table = (4,insert)
+NOTICE: trigger = move_ins, new table = (2,insert)
+drop table merge_move;
-- cleanup
drop function dump_insert();
drop function dump_update();
