Fix dangling-pointer problem in before-row update trigger processing.

ExecUpdate checked for whether ExecBRUpdateTriggers had returned a new
tuple value by seeing if the returned tuple was pointer-equal to the old
one.  But the "old one" was in estate->es_junkFilter's result slot, which
would be scribbled on if we had done an EvalPlanQual update in response to
a concurrent update of the target tuple; therefore we were comparing a
dangling pointer to a live one.  Given the right set of circumstances we
could get a false match, resulting in not forcing the tuple to be stored in
the slot we thought it was stored in.  In the case reported by Maxim Boguk
in bug #5798, this led to "cannot extract system attribute from virtual
tuple" failures when trying to do "RETURNING ctid".  I believe there is a
very-low-probability chance of more serious errors, such as generating
incorrect index entries based on the original rather than the
trigger-modified version of the row.

In HEAD, change all of ExecBRInsertTriggers, ExecIRInsertTriggers,
ExecBRUpdateTriggers, and ExecIRUpdateTriggers so that they continue to
have similar APIs.  In the back branches I just changed
ExecBRUpdateTriggers, since there is no bug in the ExecBRInsertTriggers
case.

1 files changed, 14 insertions, 15 deletions

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index cac11a6c641..44f568f3964 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -1836,7 +1836,7 @@ CopyFrom(CopyState cstate)
 	ResultRelInfo *resultRelInfo;
 	EState	   *estate = CreateExecutorState(); /* for ExecConstraints() */
 	ExprContext *econtext;
-	TupleTableSlot *slot;
+	TupleTableSlot *myslot;
 	MemoryContext oldcontext = CurrentMemoryContext;
 	ErrorContextCallback errcontext;
 	CommandId	mycid = GetCurrentCommandId(true);
@@ -1932,8 +1932,10 @@ CopyFrom(CopyState cstate)
 	estate->es_result_relation_info = resultRelInfo;
 
 	/* Set up a tuple slot too */
-	slot = ExecInitExtraTupleSlot(estate);
-	ExecSetSlotDescriptor(slot, tupDesc);
+	myslot = ExecInitExtraTupleSlot(estate);
+	ExecSetSlotDescriptor(myslot, tupDesc);
+	/* Triggers might need a slot as well */
+	estate->es_trig_tuple_slot = ExecInitExtraTupleSlot(estate);
 
 	/* Prepare to catch AFTER triggers. */
 	AfterTriggerBeginQuery();
@@ -1960,6 +1962,7 @@ CopyFrom(CopyState cstate)
 
 	for (;;)
 	{
+		TupleTableSlot *slot;
 		bool		skip_tuple;
 		Oid			loaded_oid = InvalidOid;
 
@@ -1983,32 +1986,28 @@ CopyFrom(CopyState cstate)
 		/* Triggers and stuff need to be invoked in query context. */
 		MemoryContextSwitchTo(oldcontext);
 
+		/* Place tuple in tuple slot --- but slot shouldn't free it */
+		slot = myslot;
+		ExecStoreTuple(tuple, slot, InvalidBuffer, false);
+
 		skip_tuple = false;
 
 		/* BEFORE ROW INSERT Triggers */
 		if (resultRelInfo->ri_TrigDesc &&
 			resultRelInfo->ri_TrigDesc->trig_insert_before_row)
 		{
-			HeapTuple	newtuple;
-
-			newtuple = ExecBRInsertTriggers(estate, resultRelInfo, tuple);
+			slot = ExecBRInsertTriggers(estate, resultRelInfo, slot);
 
-			if (newtuple == NULL)		/* "do nothing" */
+			if (slot == NULL)		/* "do nothing" */
 				skip_tuple = true;
-			else if (newtuple != tuple) /* modified by Trigger(s) */
-			{
-				heap_freetuple(tuple);
-				tuple = newtuple;
-			}
+			else					/* trigger might have changed tuple */
+				tuple = ExecMaterializeSlot(slot);
 		}
 
 		if (!skip_tuple)
 		{
 			List	   *recheckIndexes = NIL;
 
-			/* Place tuple in tuple slot */
-			ExecStoreTuple(tuple, slot, InvalidBuffer, false);
-
 			/* Check the constraints of the tuple */
 			if (cstate->rel->rd_att->constr)
 				ExecConstraints(resultRelInfo, slot, estate);