"Kevin Grittner" <kevin.gritt...@wicourts.gov> wrote: > Attached is a patch based on these thoughts. OK, really attached this time. -Kevin
*** a/src/backend/executor/execMain.c --- b/src/backend/executor/execMain.c *************** *** 1921,1928 **** EvalPlanQualFetch(EState *estate, Relation relation, int lockmode, switch (test) { case HeapTupleSelfUpdated: - /* treat it as deleted; do not process */ ReleaseBuffer(buffer); return NULL; case HeapTupleMayBeUpdated: --- 1921,1936 ---- switch (test) { case HeapTupleSelfUpdated: ReleaseBuffer(buffer); + if (!ItemPointerEquals(&update_ctid, &tuple.t_self)) + { + /* it was updated, so look at the updated version */ + tuple.t_self = update_ctid; + /* updated row should have xmin matching this xmax */ + priorXmax = update_xmax; + continue; + } + /* treat it as deleted; do not process */ return NULL; case HeapTupleMayBeUpdated: *** a/src/backend/executor/nodeModifyTable.c --- b/src/backend/executor/nodeModifyTable.c *************** *** 354,359 **** ldelete:; --- 354,398 ---- switch (result) { case HeapTupleSelfUpdated: + /* + * There is no sensible action to take if the BEFORE DELETE + * trigger for a row issues an UPDATE for the same row, either + * directly or by performing DML which fires other triggers + * which do the update. We don't want to discard the original + * DELETE while keeping the triggered actions based on its + * deletion; and it would be no better to allow the original + * DELETE while discarding some of its triggered actions. + * Updating the row which is being deleted risks losing some + * information which might be important according to business + * rules; so throwing an error is the only safe course. + * + * We want to be careful not to trigger this for join/updates + * which join to the same row more than once, so we check + * whether the tuple was expired by a command other than the + * one which initially fired the trigger. If it is, the + * current command ID must have changed, so do that check + * first, because it is fast. If it has changed, chase down + * to the last version of the row to see if it was changed by + * a subsequent command. + */ + if (GetCurrentCommandId(false) != estate->es_output_cid) + { + HeapTuple copyTuple; + + copyTuple = EvalPlanQualFetch(estate, + resultRelationDesc, + LockTupleExclusive, + &update_ctid, + update_xmax); + if (copyTuple == NULL + || HeapTupleHeaderGetCmax(copyTuple->t_data) != estate->es_output_cid) + { + ereport(ERROR, + (errcode(ERRCODE_TRIGGERED_DATA_CHANGE_VIOLATION), + errmsg("cannot update or delete a row from its BEFORE DELETE trigger"), + errhint("Consider moving code to an AFTER DELETE trigger."))); + } + } /* already deleted by self; nothing to do */ return NULL; *************** *** 570,575 **** lreplace:; --- 609,651 ---- switch (result) { case HeapTupleSelfUpdated: + /* + * There is no sensible action to take if the BEFORE UPDATE + * trigger for a row issues another UPDATE for the same row, + * either directly or by performing DML which fires other + * triggers which do the update. We don't want to discard the + * original UPDATE while keeping the triggered actions based + * on its update; and it would be no better to allow the + * original UPDATE while discarding some of its triggered + * actions. + * + * We want to be careful not to trigger this for join/updates + * which join to the same row more than once, so we check + * whether the tuple was expired by a command other than the + * one which initially fired the trigger. If it is, the + * current command ID must have changed, so do that check + * first, because it is fast. If it has changed, chase down + * to the last version of the row to see if it was changed by + * a subsequent command. + */ + if (GetCurrentCommandId(false) != estate->es_output_cid) + { + HeapTuple copyTuple; + + copyTuple = EvalPlanQualFetch(estate, + resultRelationDesc, + LockTupleExclusive, + &update_ctid, + update_xmax); + if (copyTuple == NULL + || HeapTupleHeaderGetCmax(copyTuple->t_data) != estate->es_output_cid) + { + ereport(ERROR, + (errcode(ERRCODE_TRIGGERED_DATA_CHANGE_VIOLATION), + errmsg("cannot update or delete a row from its BEFORE UPDATE trigger"), + errhint("Consider moving code to an AFTER UPDATE trigger."))); + } + } /* already deleted by self; nothing to do */ return NULL; *** a/src/test/regress/expected/triggers.out --- b/src/test/regress/expected/triggers.out *************** *** 1443,1445 **** NOTICE: drop cascades to 2 other objects --- 1443,1566 ---- DETAIL: drop cascades to view city_view drop cascades to view european_city_view DROP TABLE country_table; + -- + -- Test updates to rows during firing of BEFORE ROW triggers. + -- + create table parent (aid int not null primary key, + val1 text, + val2 text, + val3 text, + val4 text, + bcnt int not null default 0); + NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "parent_pkey" for table "parent" + create table child (bid int not null primary key, + aid int not null, + val1 text); + NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "child_pkey" for table "child" + create function parent_upd_func() + returns trigger language plpgsql as + $$ + begin + if old.val1 <> new.val1 then + new.val2 = new.val1; + delete from child where child.aid = new.aid and child.val1 = new.val1; + end if; + return new; + end; + $$; + create trigger parent_upd_trig before update On parent + for each row execute procedure parent_upd_func(); + create function parent_del_func() + returns trigger language plpgsql as + $$ + begin + delete from child where aid = old.aid; + return old; + end; + $$; + create trigger parent_del_trig before delete On parent + for each row execute procedure parent_del_func(); + create function child_ins_func() + returns trigger language plpgsql as + $$ + begin + update parent set bcnt = bcnt + 1 where aid = new.aid; + return new; + end; + $$; + create trigger child_ins_trig after insert on child + for each row execute procedure child_ins_func(); + create function child_del_func() + returns trigger language plpgsql as + $$ + begin + update parent set bcnt = bcnt - 1 where aid = old.aid; + return old; + end; + $$; + create trigger child_del_trig after delete on child + for each row execute procedure child_del_func(); + insert into parent values (1, 'a', 'a', 'a', 'a', 0); + insert into child values (10, 1, 'b'); + select * from parent; select * from child; + aid | val1 | val2 | val3 | val4 | bcnt + -----+------+------+------+------+------ + 1 | a | a | a | a | 1 + (1 row) + + bid | aid | val1 + -----+-----+------ + 10 | 1 | b + (1 row) + + update parent set val1 = 'b' where aid = 1; + ERROR: cannot update or delete a row from its BEFORE UPDATE trigger + HINT: Consider moving code to an AFTER UPDATE trigger. + select * from parent; select * from child; + aid | val1 | val2 | val3 | val4 | bcnt + -----+------+------+------+------+------ + 1 | a | a | a | a | 1 + (1 row) + + bid | aid | val1 + -----+-----+------ + 10 | 1 | b + (1 row) + + delete from parent where aid = 1; + ERROR: cannot update or delete a row from its BEFORE DELETE trigger + HINT: Consider moving code to an AFTER DELETE trigger. + select * from parent; select * from child; + aid | val1 | val2 | val3 | val4 | bcnt + -----+------+------+------+------+------ + 1 | a | a | a | a | 1 + (1 row) + + bid | aid | val1 + -----+-----+------ + 10 | 1 | b + (1 row) + + create or replace function parent_del_func() + returns trigger language plpgsql as + $$ + begin + delete from child where aid = old.aid; + if found then + delete from parent where aid = old.aid; + return null; + end if; + return old; + end; + $$; + delete from parent where aid = 1; + select * from parent; select * from child; + aid | val1 | val2 | val3 | val4 | bcnt + -----+------+------+------+------+------ + (0 rows) + + bid | aid | val1 + -----+-----+------ + (0 rows) + + drop table parent, child; *** a/src/test/regress/sql/triggers.sql --- b/src/test/regress/sql/triggers.sql *************** *** 961,963 **** SELECT * FROM city_view; --- 961,1050 ---- DROP TABLE city_table CASCADE; DROP TABLE country_table; + + -- + -- Test updates to rows during firing of BEFORE ROW triggers. + -- + + create table parent (aid int not null primary key, + val1 text, + val2 text, + val3 text, + val4 text, + bcnt int not null default 0); + create table child (bid int not null primary key, + aid int not null, + val1 text); + + create function parent_upd_func() + returns trigger language plpgsql as + $$ + begin + if old.val1 <> new.val1 then + new.val2 = new.val1; + delete from child where child.aid = new.aid and child.val1 = new.val1; + end if; + return new; + end; + $$; + create trigger parent_upd_trig before update On parent + for each row execute procedure parent_upd_func(); + + create function parent_del_func() + returns trigger language plpgsql as + $$ + begin + delete from child where aid = old.aid; + return old; + end; + $$; + create trigger parent_del_trig before delete On parent + for each row execute procedure parent_del_func(); + + create function child_ins_func() + returns trigger language plpgsql as + $$ + begin + update parent set bcnt = bcnt + 1 where aid = new.aid; + return new; + end; + $$; + create trigger child_ins_trig after insert on child + for each row execute procedure child_ins_func(); + + create function child_del_func() + returns trigger language plpgsql as + $$ + begin + update parent set bcnt = bcnt - 1 where aid = old.aid; + return old; + end; + $$; + create trigger child_del_trig after delete on child + for each row execute procedure child_del_func(); + + insert into parent values (1, 'a', 'a', 'a', 'a', 0); + insert into child values (10, 1, 'b'); + select * from parent; select * from child; + update parent set val1 = 'b' where aid = 1; + select * from parent; select * from child; + delete from parent where aid = 1; + select * from parent; select * from child; + + create or replace function parent_del_func() + returns trigger language plpgsql as + $$ + begin + delete from child where aid = old.aid; + if found then + delete from parent where aid = old.aid; + return null; + end if; + return old; + end; + $$; + + delete from parent where aid = 1; + select * from parent; select * from child; + + drop table parent, child;
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers