While working on the MERGE RETURNING patch, I noticed a pre-existing
MERGE bug --- ExecMergeMatched() should not call ExecUpdateEpilogue()
if ExecUpdateAct() indicates that it did a cross-partition update.

The immediate consequence is that it incorrectly tries (and fails) to
fire AFTER UPDATE ROW triggers, which it should not do if the UPDATE
has been turned into a DELETE and an INSERT:

DROP TABLE IF EXISTS foo CASCADE;

CREATE TABLE foo (a int) PARTITION BY LIST (a);
CREATE TABLE foo_p1 PARTITION OF foo FOR VALUES IN (1);
CREATE TABLE foo_p2 PARTITION OF foo FOR VALUES IN (2);
INSERT INTO foo VALUES (1);

CREATE OR REPLACE FUNCTION foo_trig_fn() RETURNS trigger AS
$$
BEGIN
  RAISE NOTICE 'Trigger: % %', TG_WHEN, TG_OP;
  IF TG_OP = 'DELETE' THEN RETURN OLD; END IF;
  RETURN NEW;
END
$$ LANGUAGE plpgsql;

CREATE TRIGGER foo_b_trig BEFORE INSERT OR UPDATE OR DELETE ON foo
  FOR EACH ROW EXECUTE FUNCTION foo_trig_fn();
CREATE TRIGGER foo_a_trig AFTER INSERT OR UPDATE OR DELETE ON foo
  FOR EACH ROW EXECUTE FUNCTION foo_trig_fn();

UPDATE foo SET a = 2 WHERE a = 1;

NOTICE:  Trigger: BEFORE UPDATE
NOTICE:  Trigger: BEFORE DELETE
NOTICE:  Trigger: BEFORE INSERT
NOTICE:  Trigger: AFTER DELETE
NOTICE:  Trigger: AFTER INSERT
UPDATE 1

MERGE INTO foo USING (VALUES (1)) AS v(a) ON true
  WHEN MATCHED THEN UPDATE SET a = v.a;

NOTICE:  Trigger: BEFORE UPDATE
NOTICE:  Trigger: BEFORE DELETE
NOTICE:  Trigger: BEFORE INSERT
NOTICE:  Trigger: AFTER DELETE
NOTICE:  Trigger: AFTER INSERT
ERROR:  failed to fetch tuple2 for AFTER trigger

The attached patch fixes that, making the UPDATE path in
ExecMergeMatched() consistent with ExecUpdate().

(If there were no AFTER ROW triggers, the old code would go on to do
other unnecessary things, like WCO/RLS checks, which I didn't really
look into. This patch will stop any of that too.)

Regards,
Dean
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
new file mode 100644
index 299c2c7..b16fbe9
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -2899,6 +2899,22 @@ lmerge_matched:
 				}
 				result = ExecUpdateAct(context, resultRelInfo, tupleid, NULL,
 									   newslot, false, &updateCxt);
+
+				/*
+				 * As in ExecUpdate(), if ExecUpdateAct() reports that a
+				 * cross-partition update was done, then there's nothing else
+				 * for us to do --- the UPDATE has been turned into a DELETE
+				 * and an INSERT, and we must not perform any of the usual
+				 * post-update tasks.
+				 */
+				if (updateCxt.crossPartUpdate)
+				{
+					mtstate->mt_merge_updated += 1;
+					if (canSetTag)
+						(estate->es_processed)++;
+					return true;
+				}
+
 				if (result == TM_Ok && updateCxt.updated)
 				{
 					ExecUpdateEpilogue(context, &updateCxt, resultRelInfo,
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
new file mode 100644
index e8de916..6cbfbbe
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -2309,6 +2309,51 @@ NOTICE:  trigger zzz on parted_trig_1_1
 NOTICE:  trigger bbb on parted_trig_2 AFTER INSERT for ROW
 NOTICE:  trigger zzz on parted_trig_2 AFTER INSERT for ROW
 drop table parted_trig;
+-- Verify that the correct triggers fire for cross-partition updates
+create table parted_trig (a int) partition by list (a);
+create table parted_trig1 partition of parted_trig for values in (1);
+create table parted_trig2 partition of parted_trig for values in (2);
+insert into parted_trig values (1);
+create or replace function trigger_notice() returns trigger as $$
+  begin
+    raise notice 'trigger % on % % % for %', TG_NAME, TG_TABLE_NAME, TG_WHEN, TG_OP, TG_LEVEL;
+    if TG_LEVEL = 'ROW' then
+      if TG_OP = 'DELETE' then
+        return OLD;
+      else
+        return NEW;
+      end if;
+    end if;
+    return null;
+  end;
+  $$ language plpgsql;
+create trigger parted_trig_before_stmt before insert or update or delete on parted_trig
+   for each statement execute procedure trigger_notice();
+create trigger parted_trig_before_row before insert or update or delete on parted_trig
+   for each row execute procedure trigger_notice();
+create trigger parted_trig_after_row after insert or update or delete on parted_trig
+   for each row execute procedure trigger_notice();
+create trigger parted_trig_after_stmt after insert or update or delete on parted_trig
+   for each statement execute procedure trigger_notice();
+update parted_trig set a = 2 where a = 1;
+NOTICE:  trigger parted_trig_before_stmt on parted_trig BEFORE UPDATE for STATEMENT
+NOTICE:  trigger parted_trig_before_row on parted_trig1 BEFORE UPDATE for ROW
+NOTICE:  trigger parted_trig_before_row on parted_trig1 BEFORE DELETE for ROW
+NOTICE:  trigger parted_trig_before_row on parted_trig2 BEFORE INSERT for ROW
+NOTICE:  trigger parted_trig_after_row on parted_trig1 AFTER DELETE for ROW
+NOTICE:  trigger parted_trig_after_row on parted_trig2 AFTER INSERT for ROW
+NOTICE:  trigger parted_trig_after_stmt on parted_trig AFTER UPDATE for STATEMENT
+-- update action in merge should behave the same
+merge into parted_trig using (select 1) on true
+  when matched and a = 2 then update set a = 1;
+NOTICE:  trigger parted_trig_before_stmt on parted_trig BEFORE UPDATE for STATEMENT
+NOTICE:  trigger parted_trig_before_row on parted_trig2 BEFORE UPDATE for ROW
+NOTICE:  trigger parted_trig_before_row on parted_trig2 BEFORE DELETE for ROW
+NOTICE:  trigger parted_trig_before_row on parted_trig1 BEFORE INSERT for ROW
+NOTICE:  trigger parted_trig_after_row on parted_trig2 AFTER DELETE for ROW
+NOTICE:  trigger parted_trig_after_row on parted_trig1 AFTER INSERT for ROW
+NOTICE:  trigger parted_trig_after_stmt on parted_trig AFTER UPDATE for STATEMENT
+drop table parted_trig;
 -- Verify propagation of trigger arguments to partitions
 create table parted_trig (a int) partition by list (a);
 create table parted_trig1 partition of parted_trig for values in (1);
diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
new file mode 100644
index d29e98d..e5f30bd
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -1583,6 +1583,42 @@ create trigger qqq after insert on parte
 insert into parted_trig values (50), (1500);
 drop table parted_trig;
 
+-- Verify that the correct triggers fire for cross-partition updates
+create table parted_trig (a int) partition by list (a);
+create table parted_trig1 partition of parted_trig for values in (1);
+create table parted_trig2 partition of parted_trig for values in (2);
+insert into parted_trig values (1);
+
+create or replace function trigger_notice() returns trigger as $$
+  begin
+    raise notice 'trigger % on % % % for %', TG_NAME, TG_TABLE_NAME, TG_WHEN, TG_OP, TG_LEVEL;
+    if TG_LEVEL = 'ROW' then
+      if TG_OP = 'DELETE' then
+        return OLD;
+      else
+        return NEW;
+      end if;
+    end if;
+    return null;
+  end;
+  $$ language plpgsql;
+create trigger parted_trig_before_stmt before insert or update or delete on parted_trig
+   for each statement execute procedure trigger_notice();
+create trigger parted_trig_before_row before insert or update or delete on parted_trig
+   for each row execute procedure trigger_notice();
+create trigger parted_trig_after_row after insert or update or delete on parted_trig
+   for each row execute procedure trigger_notice();
+create trigger parted_trig_after_stmt after insert or update or delete on parted_trig
+   for each statement execute procedure trigger_notice();
+
+update parted_trig set a = 2 where a = 1;
+
+-- update action in merge should behave the same
+merge into parted_trig using (select 1) on true
+  when matched and a = 2 then update set a = 1;
+
+drop table parted_trig;
+
 -- Verify propagation of trigger arguments to partitions
 create table parted_trig (a int) partition by list (a);
 create table parted_trig1 partition of parted_trig for values in (1);

Reply via email to