From 5f41738b3c7dc7bf3d849539d16a568b52cedc55 Mon Sep 17 00:00:00 2001 From: jian he Date: Mon, 15 Jan 2024 16:20:28 +0800 Subject: [PATCH v5 1/1] allow INSTEAD OF DELETE triggers to modify the OLD tuple and use that for RETURNING instead of returning the tuple in planSlot. Currently the tuple returned by INSTEAD OF triggers on DELETEs is only used to determine whether to pretend that the DELETE happened or not. now we can modified the old value returned by INSTEAD OF DELETE trigger. --- doc/src/sgml/trigger.sgml | 7 ++++-- src/backend/commands/trigger.c | 34 ++++++++++++++++++-------- src/backend/executor/nodeModifyTable.c | 14 +++++++++-- src/include/commands/trigger.h | 2 +- src/test/regress/expected/triggers.out | 27 ++++++++++++++++++++ src/test/regress/sql/triggers.sql | 20 +++++++++++++++ 6 files changed, 89 insertions(+), 15 deletions(-) diff --git a/doc/src/sgml/trigger.sgml b/doc/src/sgml/trigger.sgml index a5390ff6..9813e323 100644 --- a/doc/src/sgml/trigger.sgml +++ b/doc/src/sgml/trigger.sgml @@ -261,9 +261,12 @@ modifications in the view. This will cause the count of the number of rows affected by the command to be incremented. For INSERT and UPDATE operations only, the trigger - may modify the NEW row before returning it. This will + may modify the NEW row before returning it. + For DELETE operations, the trigger + may modify the OLD row before returning it. + This will change the data returned by - INSERT RETURNING or UPDATE RETURNING, + INSERT RETURNING, UPDATE RETURNING, DELETE RETURNING and is useful when the view will not show exactly the same data that was provided. diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 0880ca51..d8e95286 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -2815,10 +2815,11 @@ ExecARDeleteTriggers(EState *estate, bool ExecIRDeleteTriggers(EState *estate, ResultRelInfo *relinfo, - HeapTuple trigtuple) + TupleTableSlot *slot) { TriggerDesc *trigdesc = relinfo->ri_TrigDesc; - TupleTableSlot *slot = ExecGetTriggerOldSlot(estate, relinfo); + HeapTuple newtuple = NULL; + bool should_free; TriggerData LocTriggerData = {0}; int i; @@ -2828,12 +2829,10 @@ ExecIRDeleteTriggers(EState *estate, ResultRelInfo *relinfo, TRIGGER_EVENT_INSTEAD; LocTriggerData.tg_relation = relinfo->ri_RelationDesc; - ExecForceStoreHeapTuple(trigtuple, slot, false); - for (i = 0; i < trigdesc->numtriggers; i++) { - HeapTuple rettuple; Trigger *trigger = &trigdesc->triggers[i]; + HeapTuple oldtuple; if (!TRIGGER_TYPE_MATCHES(trigger->tgtype, TRIGGER_TYPE_ROW, @@ -2844,18 +2843,33 @@ ExecIRDeleteTriggers(EState *estate, ResultRelInfo *relinfo, NULL, slot, NULL)) continue; + if (!newtuple) + newtuple = ExecFetchSlotHeapTuple(slot, true, &should_free); + LocTriggerData.tg_trigslot = slot; - LocTriggerData.tg_trigtuple = trigtuple; + LocTriggerData.tg_trigtuple = oldtuple = newtuple; LocTriggerData.tg_trigger = trigger; - rettuple = ExecCallTriggerFunc(&LocTriggerData, + newtuple = ExecCallTriggerFunc(&LocTriggerData, i, relinfo->ri_TrigFunctions, relinfo->ri_TrigInstrument, GetPerTupleMemoryContext(estate)); - if (rettuple == NULL) + if (newtuple == NULL) + { + if (should_free) + heap_freetuple(oldtuple); return false; /* Delete was suppressed */ - if (rettuple != trigtuple) - heap_freetuple(rettuple); + } + else if (newtuple != oldtuple) + { + ExecForceStoreHeapTuple(newtuple, slot, false); + + if (should_free) + heap_freetuple(oldtuple); + heap_freetuple(newtuple); + /* signal tuple should be re-fetched if used */ + newtuple = NULL; + } } return true; } diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 9fc5abff..103553b2 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1452,7 +1452,11 @@ ExecDelete(ModifyTableContext *context, bool dodelete; Assert(oldtuple != NULL); - dodelete = ExecIRDeleteTriggers(estate, resultRelInfo, oldtuple); + + slot = ExecGetReturningSlot(estate, resultRelInfo); + ExecForceStoreHeapTuple(oldtuple, slot, false); + + dodelete = ExecIRDeleteTriggers(estate, resultRelInfo, slot); if (!dodelete) /* "do nothing" */ return NULL; @@ -1676,7 +1680,13 @@ ldelete: */ TupleTableSlot *rslot; - if (resultRelInfo->ri_FdwRoutine) + if (resultRelInfo->ri_TrigDesc && + resultRelInfo->ri_TrigDesc->trig_delete_instead_row) + { + /* INSTEAD OF trigger handling should have provided a slot */ + Assert(!TupIsNull(slot)); + } + else if (resultRelInfo->ri_FdwRoutine) { /* FDW must have provided a slot containing the deleted row */ Assert(!TupIsNull(slot)); diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h index 8a5a9fe6..e853da57 100644 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -222,7 +222,7 @@ extern void ExecARDeleteTriggers(EState *estate, bool is_crosspart_update); extern bool ExecIRDeleteTriggers(EState *estate, ResultRelInfo *relinfo, - HeapTuple trigtuple); + TupleTableSlot *slot); extern void ExecBSUpdateTriggers(EState *estate, ResultRelInfo *relinfo); extern void ExecASUpdateTriggers(EState *estate, diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out index 78e90309..df557f5b 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -1385,6 +1385,22 @@ end; $$; CREATE TRIGGER city_update_trig INSTEAD OF UPDATE ON city_view FOR EACH ROW EXECUTE PROCEDURE city_update(); +-- DELETE .. RETURNING setup. +CREATE TABLE test_ins_del(a int, b text); +INSERT INTO test_ins_del VALUES (1,repeat('x', 1000)); +CREATE VIEW test_ins_del_view AS SELECT a as a,b as b from test_ins_del; +CREATE OR REPLACE FUNCTION view_ins_del_trig_trig() RETURNS trigger LANGUAGE plpgsql AS $$ +BEGIN + if (TG_OP = 'DELETE') then + OLD.a := OLD.a + 3; + raise notice 'old.a %',old.a; + RETURN OLD; + end if; +end +$$; +CREATE OR REPLACE TRIGGER tv_delete_trig +INSTEAD OF DELETE ON test_ins_del_view +FOR EACH ROW EXECUTE PROCEDURE view_ins_del_trig_trig(); \set QUIET false -- INSERT .. RETURNING INSERT INTO city_view(city_name) VALUES('Tokyo') RETURNING *; @@ -1478,8 +1494,19 @@ DELETE FROM city_view WHERE city_name = 'Birmingham' RETURNING *; 234567 | Birmingham | 1016800 | UK | Europe (1 row) +DELETE 1 +DELETE FROM test_ins_del_view where a = 1 returning a; +NOTICE: old.a 4 + a +--- + 4 +(1 row) + DELETE 1 \set QUIET true +DROP TABLE test_ins_del cascade; +NOTICE: drop cascades to view test_ins_del_view +DROP FUNCTION view_ins_del_trig_trig; -- read-only view with WHERE clause CREATE VIEW european_city_view AS SELECT * FROM city_view WHERE continent = 'Europe'; diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql index 46795a9c..e2dd702d 100644 --- a/src/test/regress/sql/triggers.sql +++ b/src/test/regress/sql/triggers.sql @@ -960,6 +960,23 @@ $$; CREATE TRIGGER city_update_trig INSTEAD OF UPDATE ON city_view FOR EACH ROW EXECUTE PROCEDURE city_update(); +-- DELETE .. RETURNING setup. +CREATE TABLE test_ins_del(a int, b text); +INSERT INTO test_ins_del VALUES (1,repeat('x', 1000)); +CREATE VIEW test_ins_del_view AS SELECT a as a,b as b from test_ins_del; +CREATE OR REPLACE FUNCTION view_ins_del_trig_trig() RETURNS trigger LANGUAGE plpgsql AS $$ +BEGIN + if (TG_OP = 'DELETE') then + OLD.a := OLD.a + 3; + raise notice 'old.a %',old.a; + RETURN OLD; + end if; +end +$$; + +CREATE OR REPLACE TRIGGER tv_delete_trig +INSTEAD OF DELETE ON test_ins_del_view +FOR EACH ROW EXECUTE PROCEDURE view_ins_del_trig_trig(); \set QUIET false -- INSERT .. RETURNING @@ -983,9 +1000,12 @@ UPDATE city_view v1 SET country_name = v2.country_name FROM city_view v2 -- DELETE .. RETURNING DELETE FROM city_view WHERE city_name = 'Birmingham' RETURNING *; +DELETE FROM test_ins_del_view where a = 1 returning a; \set QUIET true +DROP TABLE test_ins_del cascade; +DROP FUNCTION view_ins_del_trig_trig; -- read-only view with WHERE clause CREATE VIEW european_city_view AS SELECT * FROM city_view WHERE continent = 'Europe'; -- 2.34.1