bug6123-v5.patch

*** a/src/backend/access/heap/heapam.c
--- b/src/backend/access/heap/heapam.c
***************
*** 2336,2343 **** simple_heap_insert(Relation relation, HeapTuple tup)
   */
  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();
--- 2336,2343 ----
   */
  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,2506 ----
  			   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);
! 		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:
--- 2632,2642 ----
  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:
***************
*** 2693,2700 **** simple_heap_delete(Relation relation, ItemPointer tid)
   */
  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();
--- 2692,2699 ----
   */
  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);
--- 2872,2880 ----
  			   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);
! 		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:
--- 3344,3354 ----
  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,50 ----
  } LockTupleMode;
  
  
+ /* return values for heap_update and heap_delete */
+ typedef struct HeapUpdateFailureData
+ {
+ 	ItemPointerData		update_ctid;
+ 	TransactionId		update_xmax;
+ 	CommandId			update_cmax;
+ }	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,
--- 109,120 ----
  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;