*** a/src/backend/access/heap/heapam.c --- b/src/backend/access/heap/heapam.c *************** *** 2317,2324 **** simple_heap_insert(Relation relation, HeapTuple tup) * * relation - table to be modified (caller must hold suitable lock) * tid - TID of tuple to be deleted ! * ctid - output parameter, used only for failure case (see below) ! * update_xmax - output parameter, used only for failure case (see below) * cid - delete command ID (used for visibility test, and stored into * cmax if successful) * crosscheck - if not InvalidSnapshot, also check tuple against this --- 2317,2323 ---- * * relation - table to be modified (caller must hold suitable lock) * tid - TID of tuple to be deleted ! * hufd - output structure, used only for failure case (see below) * cid - delete command ID (used for visibility test, and stored into * cmax if successful) * crosscheck - if not InvalidSnapshot, also check tuple against this *************** *** 2330,2343 **** simple_heap_insert(Relation relation, HeapTuple tup) * (the last only possible if wait == false). * * In the failure cases, the routine returns the tuple's t_ctid and t_xmax. * If t_ctid is the same as tid, the tuple was deleted; if different, the * tuple was updated, and t_ctid is the location of the replacement tuple. * (t_xmax is needed to verify that the replacement tuple matches.) */ HTSU_Result heap_delete(Relation relation, ItemPointer tid, ! ItemPointer ctid, TransactionId *update_xmax, ! CommandId cid, Snapshot crosscheck, bool wait) { HTSU_Result result; TransactionId xid = GetCurrentTransactionId(); --- 2329,2343 ---- * (the last only possible if wait == false). * * In the failure cases, the routine returns the tuple's t_ctid and t_xmax. + * If HeapTupleSelfUpdated, it also returns the cmax from the tuple. * If t_ctid is the same as tid, the tuple was deleted; if different, the * tuple was updated, and t_ctid is the location of the replacement tuple. * (t_xmax is needed to verify that the replacement tuple matches.) */ HTSU_Result heap_delete(Relation relation, ItemPointer tid, ! HeapUpdateFailureData *hufd, CommandId cid, ! Snapshot crosscheck, bool wait) { HTSU_Result result; TransactionId xid = GetCurrentTransactionId(); *************** *** 2498,2505 **** l1: result == HeapTupleUpdated || result == HeapTupleBeingUpdated); Assert(!(tp.t_data->t_infomask & HEAP_XMAX_INVALID)); ! *ctid = tp.t_data->t_ctid; ! *update_xmax = HeapTupleHeaderGetXmax(tp.t_data); UnlockReleaseBuffer(buffer); if (have_tuple_lock) UnlockTuple(relation, &(tp.t_self), ExclusiveLock); --- 2498,2507 ---- result == HeapTupleUpdated || result == HeapTupleBeingUpdated); Assert(!(tp.t_data->t_infomask & HEAP_XMAX_INVALID)); ! hufd->update_ctid = tp.t_data->t_ctid; ! hufd->update_xmax = HeapTupleHeaderGetXmax(tp.t_data); ! if (result == HeapTupleSelfUpdated) ! hufd->update_cmax = HeapTupleHeaderGetCmax(tp.t_data); UnlockReleaseBuffer(buffer); if (have_tuple_lock) UnlockTuple(relation, &(tp.t_self), ExclusiveLock); *************** *** 2631,2643 **** void simple_heap_delete(Relation relation, ItemPointer tid) { HTSU_Result result; ! ItemPointerData update_ctid; ! TransactionId update_xmax; result = heap_delete(relation, tid, ! &update_ctid, &update_xmax, ! GetCurrentCommandId(true), InvalidSnapshot, ! true /* wait for commit */ ); switch (result) { case HeapTupleSelfUpdated: --- 2633,2643 ---- simple_heap_delete(Relation relation, ItemPointer tid) { HTSU_Result result; ! HeapUpdateFailureData hufd; result = heap_delete(relation, tid, ! &hufd, GetCurrentCommandId(true), ! InvalidSnapshot, true /* wait for commit */ ); switch (result) { case HeapTupleSelfUpdated: *************** *** 2668,2675 **** simple_heap_delete(Relation relation, ItemPointer tid) * relation - table to be modified (caller must hold suitable lock) * otid - TID of old tuple to be replaced * newtup - newly constructed tuple data to store ! * ctid - output parameter, used only for failure case (see below) ! * update_xmax - output parameter, used only for failure case (see below) * cid - update command ID (used for visibility test, and stored into * cmax/cmin if successful) * crosscheck - if not InvalidSnapshot, also check old tuple against this --- 2668,2674 ---- * relation - table to be modified (caller must hold suitable lock) * otid - TID of old tuple to be replaced * newtup - newly constructed tuple data to store ! * hufd - output structure, used only for failure case (see below) * cid - update command ID (used for visibility test, and stored into * cmax/cmin if successful) * crosscheck - if not InvalidSnapshot, also check old tuple against this *************** *** 2687,2700 **** simple_heap_delete(Relation relation, ItemPointer tid) * data are not reflected into *newtup. * * In the failure cases, the routine returns the tuple's t_ctid and t_xmax. * If t_ctid is the same as otid, the tuple was deleted; if different, the * tuple was updated, and t_ctid is the location of the replacement tuple. * (t_xmax is needed to verify that the replacement tuple matches.) */ HTSU_Result heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, ! ItemPointer ctid, TransactionId *update_xmax, ! CommandId cid, Snapshot crosscheck, bool wait) { HTSU_Result result; TransactionId xid = GetCurrentTransactionId(); --- 2686,2700 ---- * data are not reflected into *newtup. * * In the failure cases, the routine returns the tuple's t_ctid and t_xmax. + * If HeapTupleSelfUpdated, it also returns the cmax from the tuple. * If t_ctid is the same as otid, the tuple was deleted; if different, the * tuple was updated, and t_ctid is the location of the replacement tuple. * (t_xmax is needed to verify that the replacement tuple matches.) */ HTSU_Result heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, ! HeapUpdateFailureData *hufd, CommandId cid, ! Snapshot crosscheck, bool wait) { HTSU_Result result; TransactionId xid = GetCurrentTransactionId(); *************** *** 2873,2880 **** l2: result == HeapTupleUpdated || result == HeapTupleBeingUpdated); Assert(!(oldtup.t_data->t_infomask & HEAP_XMAX_INVALID)); ! *ctid = oldtup.t_data->t_ctid; ! *update_xmax = HeapTupleHeaderGetXmax(oldtup.t_data); UnlockReleaseBuffer(buffer); if (have_tuple_lock) UnlockTuple(relation, &(oldtup.t_self), ExclusiveLock); --- 2873,2882 ---- result == HeapTupleUpdated || result == HeapTupleBeingUpdated); Assert(!(oldtup.t_data->t_infomask & HEAP_XMAX_INVALID)); ! hufd->update_ctid = oldtup.t_data->t_ctid; ! hufd->update_xmax = HeapTupleHeaderGetXmax(oldtup.t_data); ! if (result == HeapTupleSelfUpdated) ! hufd->update_cmax = HeapTupleHeaderGetCmax(oldtup.t_data); UnlockReleaseBuffer(buffer); if (have_tuple_lock) UnlockTuple(relation, &(oldtup.t_self), ExclusiveLock); *************** *** 3344,3356 **** void simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup) { HTSU_Result result; ! ItemPointerData update_ctid; ! TransactionId update_xmax; result = heap_update(relation, otid, tup, ! &update_ctid, &update_xmax, ! GetCurrentCommandId(true), InvalidSnapshot, ! true /* wait for commit */ ); switch (result) { case HeapTupleSelfUpdated: --- 3346,3356 ---- simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup) { HTSU_Result result; ! HeapUpdateFailureData hufd; result = heap_update(relation, otid, tup, ! &hufd, GetCurrentCommandId(true), ! InvalidSnapshot, true /* wait for commit */ ); switch (result) { case HeapTupleSelfUpdated: *** 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 *************** *** 294,301 **** ExecDelete(ItemPointer tupleid, ResultRelInfo *resultRelInfo; Relation resultRelationDesc; HTSU_Result result; ! ItemPointerData update_ctid; ! TransactionId update_xmax; /* * get information on the (current) result relation --- 294,300 ---- ResultRelInfo *resultRelInfo; Relation resultRelationDesc; HTSU_Result result; ! HeapUpdateFailureData hufd; /* * get information on the (current) result relation *************** *** 347,359 **** ExecDelete(ItemPointer tupleid, */ ldelete:; result = heap_delete(resultRelationDesc, tupleid, ! &update_ctid, &update_xmax, ! estate->es_output_cid, estate->es_crosscheck_snapshot, true /* wait for commit */ ); switch (result) { case HeapTupleSelfUpdated: /* already deleted by self; nothing to do */ return NULL; --- 346,381 ---- */ ldelete:; result = heap_delete(resultRelationDesc, tupleid, ! &hufd, estate->es_output_cid, estate->es_crosscheck_snapshot, true /* wait for commit */ ); 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 (hufd.update_cmax != 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; *************** *** 365,371 **** ldelete:; ereport(ERROR, (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), errmsg("could not serialize access due to concurrent update"))); ! if (!ItemPointerEquals(tupleid, &update_ctid)) { TupleTableSlot *epqslot; --- 387,393 ---- ereport(ERROR, (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), errmsg("could not serialize access due to concurrent update"))); ! if (!ItemPointerEquals(tupleid, &(hufd.update_ctid))) { TupleTableSlot *epqslot; *************** *** 373,383 **** ldelete:; epqstate, resultRelationDesc, resultRelInfo->ri_RangeTableIndex, ! &update_ctid, ! update_xmax); if (!TupIsNull(epqslot)) { ! *tupleid = update_ctid; goto ldelete; } } --- 395,405 ---- epqstate, resultRelationDesc, resultRelInfo->ri_RangeTableIndex, ! &(hufd.update_ctid), ! hufd.update_xmax); if (!TupIsNull(epqslot)) { ! *tupleid = hufd.update_ctid; goto ldelete; } } *************** *** 481,488 **** ExecUpdate(ItemPointer tupleid, ResultRelInfo *resultRelInfo; Relation resultRelationDesc; HTSU_Result result; ! ItemPointerData update_ctid; ! TransactionId update_xmax; List *recheckIndexes = NIL; /* --- 503,509 ---- ResultRelInfo *resultRelInfo; Relation resultRelationDesc; HTSU_Result result; ! HeapUpdateFailureData hufd; List *recheckIndexes = NIL; /* *************** *** 563,575 **** lreplace:; * mode transactions. */ result = heap_update(resultRelationDesc, tupleid, tuple, ! &update_ctid, &update_xmax, ! estate->es_output_cid, estate->es_crosscheck_snapshot, true /* wait for commit */ ); switch (result) { case HeapTupleSelfUpdated: /* already deleted by self; nothing to do */ return NULL; --- 584,617 ---- * mode transactions. */ result = heap_update(resultRelationDesc, tupleid, tuple, ! &hufd, estate->es_output_cid, estate->es_crosscheck_snapshot, true /* wait for commit */ ); 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 (hufd.update_cmax != 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; *************** *** 581,587 **** lreplace:; ereport(ERROR, (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), errmsg("could not serialize access due to concurrent update"))); ! if (!ItemPointerEquals(tupleid, &update_ctid)) { TupleTableSlot *epqslot; --- 623,629 ---- ereport(ERROR, (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), errmsg("could not serialize access due to concurrent update"))); ! if (!ItemPointerEquals(tupleid, &(hufd.update_ctid))) { TupleTableSlot *epqslot; *************** *** 589,599 **** lreplace:; epqstate, resultRelationDesc, resultRelInfo->ri_RangeTableIndex, ! &update_ctid, ! update_xmax); if (!TupIsNull(epqslot)) { ! *tupleid = update_ctid; slot = ExecFilterJunk(resultRelInfo->ri_junkFilter, epqslot); tuple = ExecMaterializeSlot(slot); goto lreplace; --- 631,641 ---- epqstate, resultRelationDesc, resultRelInfo->ri_RangeTableIndex, ! &(hufd.update_ctid), ! hufd.update_xmax); if (!TupIsNull(epqslot)) { ! *tupleid = hufd.update_ctid; slot = ExecFilterJunk(resultRelInfo->ri_junkFilter, epqslot); tuple = ExecMaterializeSlot(slot); goto lreplace; *** a/src/include/access/heapam.h --- b/src/include/access/heapam.h *************** *** 36,41 **** typedef enum --- 36,53 ---- } LockTupleMode; + /* + * return values for heap_update and heap_delete, filled only on failure; + * otherwise the contents are undefined. + */ + typedef struct HeapUpdateFailureData + { + ItemPointerData update_ctid; + TransactionId update_xmax; + CommandId update_cmax; /* filled only for SelfUpdated */ + } HeapUpdateFailureData; + + /* ---------------- * function prototypes for heap access method * *************** *** 100,111 **** extern Oid heap_insert(Relation relation, HeapTuple tup, CommandId cid, extern void heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples, CommandId cid, int options, BulkInsertState bistate); extern HTSU_Result heap_delete(Relation relation, ItemPointer tid, ! ItemPointer ctid, TransactionId *update_xmax, ! CommandId cid, Snapshot crosscheck, bool wait); extern HTSU_Result heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, ! ItemPointer ctid, TransactionId *update_xmax, ! CommandId cid, Snapshot crosscheck, bool wait); extern HTSU_Result heap_lock_tuple(Relation relation, HeapTuple tuple, Buffer *buffer, ItemPointer ctid, TransactionId *update_xmax, CommandId cid, --- 112,123 ---- extern void heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples, CommandId cid, int options, BulkInsertState bistate); extern HTSU_Result heap_delete(Relation relation, ItemPointer tid, ! HeapUpdateFailureData *hufd, CommandId cid, ! Snapshot crosscheck, bool wait); extern HTSU_Result heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, ! HeapUpdateFailureData *hufd, CommandId cid, ! Snapshot crosscheck, bool wait); extern HTSU_Result heap_lock_tuple(Relation relation, HeapTuple tuple, Buffer *buffer, ItemPointer ctid, TransactionId *update_xmax, CommandId cid, *** 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;