Use slots in trigger infrastructure, except for the actual invocation.

In preparation for abstracting table storage, convert trigger.c to
track tuples in slots. Which also happens to make code calling
triggers simpler.

As the calling interface for triggers themselves is not changed in
this patch, HeapTuples still are extracted from the slot at that
time. But that's handled solely inside trigger.c, not visible to
callers. It's quite likely that we'll want to revise the external
trigger interface, but that's a separate large project.

As part of this work the slots used for old/new/return tuples are
moved from EState into ResultRelInfo, as different updated tables
might need different slots. The slots are now also now created
on-demand, which is good both from an efficiency POV, but also makes
the modifying code simpler.

Author: Andres Freund, Amit Khandekar and Ashutosh Bapat
Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de

1 files changed, 60 insertions, 106 deletions

diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index fe62da06ead..76175aaa6be 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -255,7 +255,6 @@ ExecInsert(ModifyTableState *mtstate,
 		   EState *estate,
 		   bool canSetTag)
 {
-	HeapTuple	tuple;
 	ResultRelInfo *resultRelInfo;
 	Relation	resultRelationDesc;
 	List	   *recheckIndexes = NIL;
@@ -264,11 +263,7 @@ ExecInsert(ModifyTableState *mtstate,
 	ModifyTable *node = (ModifyTable *) mtstate->ps.plan;
 	OnConflictAction onconflict = node->onConflictAction;
 
-	/*
-	 * get the heap tuple out of the tuple table slot, making sure we have a
-	 * writable copy
-	 */
-	tuple = ExecFetchSlotHeapTuple(slot, true, NULL);
+	ExecMaterializeSlot(slot);
 
 	/*
 	 * get information on the (current) result relation
@@ -288,26 +283,16 @@ ExecInsert(ModifyTableState *mtstate,
 	if (resultRelInfo->ri_TrigDesc &&
 		resultRelInfo->ri_TrigDesc->trig_insert_before_row)
 	{
-		slot = ExecBRInsertTriggers(estate, resultRelInfo, slot);
-
-		if (slot == NULL)		/* "do nothing" */
-			return NULL;
-
-		/* trigger might have changed tuple */
-		tuple = ExecFetchSlotHeapTuple(slot, true, NULL);
+		if (!ExecBRInsertTriggers(estate, resultRelInfo, slot))
+			return NULL;		/* "do nothing" */
 	}
 
 	/* INSTEAD OF ROW INSERT Triggers */
 	if (resultRelInfo->ri_TrigDesc &&
 		resultRelInfo->ri_TrigDesc->trig_insert_instead_row)
 	{
-		slot = ExecIRInsertTriggers(estate, resultRelInfo, slot);
-
-		if (slot == NULL)		/* "do nothing" */
-			return NULL;
-
-		/* trigger might have changed tuple */
-		tuple = ExecFetchSlotHeapTuple(slot, true, NULL);
+		if (!ExecIRInsertTriggers(estate, resultRelInfo, slot))
+			return NULL;		/* "do nothing" */
 	}
 	else if (resultRelInfo->ri_FdwRoutine)
 	{
@@ -322,9 +307,6 @@ ExecInsert(ModifyTableState *mtstate,
 		if (slot == NULL)		/* "do nothing" */
 			return NULL;
 
-		/* FDW might have changed tuple */
-		tuple = ExecFetchSlotHeapTuple(slot, true, NULL);
-
 		/*
 		 * AFTER ROW Triggers or RETURNING expressions might reference the
 		 * tableoid column, so (re-)initialize tts_tableOid before evaluating
@@ -336,6 +318,7 @@ ExecInsert(ModifyTableState *mtstate,
 	else
 	{
 		WCOKind		wco_kind;
+		HeapTuple	inserttuple;
 
 		/*
 		 * Constraints might reference the tableoid column, so (re-)initialize
@@ -441,6 +424,8 @@ ExecInsert(ModifyTableState *mtstate,
 				}
 			}
 
+			inserttuple = ExecFetchSlotHeapTuple(slot, true, NULL);
+
 			/*
 			 * Before we start insertion proper, acquire our "speculative
 			 * insertion lock".  Others can use that to wait for us to decide
@@ -448,26 +433,26 @@ ExecInsert(ModifyTableState *mtstate,
 			 * waiting for the whole transaction to complete.
 			 */
 			specToken = SpeculativeInsertionLockAcquire(GetCurrentTransactionId());
-			HeapTupleHeaderSetSpeculativeToken(tuple->t_data, specToken);
+			HeapTupleHeaderSetSpeculativeToken(inserttuple->t_data, specToken);
 
 			/* insert the tuple, with the speculative token */
-			heap_insert(resultRelationDesc, tuple,
+			heap_insert(resultRelationDesc, inserttuple,
 						estate->es_output_cid,
 						HEAP_INSERT_SPECULATIVE,
 						NULL);
 			slot->tts_tableOid = RelationGetRelid(resultRelationDesc);
-			ItemPointerCopy(&tuple->t_self, &slot->tts_tid);
+			ItemPointerCopy(&inserttuple->t_self, &slot->tts_tid);
 
 			/* insert index entries for tuple */
-			recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self),
+			recheckIndexes = ExecInsertIndexTuples(slot, &(inserttuple->t_self),
 												   estate, true, &specConflict,
 												   arbiterIndexes);
 
 			/* adjust the tuple's state accordingly */
 			if (!specConflict)
-				heap_finish_speculative(resultRelationDesc, tuple);
+				heap_finish_speculative(resultRelationDesc, inserttuple);
 			else
-				heap_abort_speculative(resultRelationDesc, tuple);
+				heap_abort_speculative(resultRelationDesc, inserttuple);
 
 			/*
 			 * Wake up anyone waiting for our decision.  They will re-check
@@ -499,15 +484,16 @@ ExecInsert(ModifyTableState *mtstate,
 			 * Note: heap_insert returns the tid (location) of the new tuple
 			 * in the t_self field.
 			 */
-			heap_insert(resultRelationDesc, tuple,
+			inserttuple = ExecFetchSlotHeapTuple(slot, true, NULL);
+			heap_insert(resultRelationDesc, inserttuple,
 						estate->es_output_cid,
 						0, NULL);
 			slot->tts_tableOid = RelationGetRelid(resultRelationDesc);
-			ItemPointerCopy(&tuple->t_self, &slot->tts_tid);
+			ItemPointerCopy(&inserttuple->t_self, &slot->tts_tid);
 
 			/* insert index entries for tuple */
 			if (resultRelInfo->ri_NumIndices > 0)
-				recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self),
+				recheckIndexes = ExecInsertIndexTuples(slot, &(inserttuple->t_self),
 													   estate, false, NULL,
 													   NIL);
 		}
@@ -531,7 +517,7 @@ ExecInsert(ModifyTableState *mtstate,
 	{
 		ExecARUpdateTriggers(estate, resultRelInfo, NULL,
 							 NULL,
-							 tuple,
+							 slot,
 							 NULL,
 							 mtstate->mt_transition_capture);
 
@@ -543,7 +529,7 @@ ExecInsert(ModifyTableState *mtstate,
 	}
 
 	/* AFTER ROW INSERT Triggers */
-	ExecARInsertTriggers(estate, resultRelInfo, tuple, recheckIndexes,
+	ExecARInsertTriggers(estate, resultRelInfo, slot, recheckIndexes,
 						 ar_insert_trig_tcs);
 
 	list_free(recheckIndexes);
@@ -603,7 +589,7 @@ ExecDelete(ModifyTableState *mtstate,
 		   bool canSetTag,
 		   bool changingPart,
 		   bool *tupleDeleted,
-		   TupleTableSlot **epqslot)
+		   TupleTableSlot **epqreturnslot)
 {
 	ResultRelInfo *resultRelInfo;
 	Relation	resultRelationDesc;
@@ -628,7 +614,7 @@ ExecDelete(ModifyTableState *mtstate,
 		bool		dodelete;
 
 		dodelete = ExecBRDeleteTriggers(estate, epqstate, resultRelInfo,
-										tupleid, oldtuple, epqslot);
+										tupleid, oldtuple, epqreturnslot);
 
 		if (!dodelete)			/* "do nothing" */
 			return NULL;
@@ -651,14 +637,10 @@ ExecDelete(ModifyTableState *mtstate,
 		/*
 		 * delete from foreign table: let the FDW do it
 		 *
-		 * We offer the trigger tuple slot as a place to store RETURNING data,
-		 * although the FDW can return some other slot if it wants.  Set up
-		 * the slot's tupdesc so the FDW doesn't need to do that for itself.
+		 * We offer the returning slot as a place to store RETURNING data,
+		 * although the FDW can return some other slot if it wants.
 		 */
-		slot = estate->es_trig_tuple_slot;
-		if (slot->tts_tupleDescriptor != RelationGetDescr(resultRelationDesc))
-			ExecSetSlotDescriptor(slot, RelationGetDescr(resultRelationDesc));
-
+		slot = ExecGetReturningSlot(estate, resultRelInfo);
 		slot = resultRelInfo->ri_FdwRoutine->ExecForeignDelete(estate,
 															   resultRelInfo,
 															   slot,
@@ -673,6 +655,8 @@ ExecDelete(ModifyTableState *mtstate,
 		 */
 		if (TTS_EMPTY(slot))
 			ExecStoreAllNullTuple(slot);
+		ExecMaterializeSlot(slot);
+
 		slot->tts_tableOid = RelationGetRelid(resultRelationDesc);
 	}
 	else
@@ -762,9 +746,9 @@ ldelete:;
 						 * If requested, skip delete and pass back the updated
 						 * row.
 						 */
-						if (epqslot)
+						if (epqreturnslot)
 						{
-							*epqslot = my_epqslot;
+							*epqreturnslot = my_epqslot;
 							return NULL;
 						}
 						else
@@ -832,34 +816,37 @@ ldelete:;
 		 * gotta fetch it.  We can use the trigger tuple slot.
 		 */
 		TupleTableSlot *rslot;
-		HeapTupleData deltuple;
-		Buffer		delbuffer;
 
 		if (resultRelInfo->ri_FdwRoutine)
 		{
 			/* FDW must have provided a slot containing the deleted row */
 			Assert(!TupIsNull(slot));
-			delbuffer = InvalidBuffer;
 		}
 		else
 		{
-			slot = estate->es_trig_tuple_slot;
+			slot = ExecGetReturningSlot(estate, resultRelInfo);
 			if (oldtuple != NULL)
 			{
-				deltuple = *oldtuple;
-				delbuffer = InvalidBuffer;
+				ExecForceStoreHeapTuple(oldtuple, slot);
 			}
 			else
 			{
-				deltuple.t_self = *tupleid;
+				BufferHeapTupleTableSlot *bslot;
+				HeapTuple deltuple;
+				Buffer buffer;
+
+				Assert(TTS_IS_BUFFERTUPLE(slot));
+				ExecClearTuple(slot);
+				bslot = (BufferHeapTupleTableSlot *) slot;
+				deltuple = &bslot->base.tupdata;
+
+				deltuple->t_self = *tupleid;
 				if (!heap_fetch(resultRelationDesc, SnapshotAny,
-								&deltuple, &delbuffer, false, NULL))
+								deltuple, &buffer, false, NULL))
 					elog(ERROR, "failed to fetch deleted tuple for DELETE RETURNING");
-			}
 
-			if (slot->tts_tupleDescriptor != RelationGetDescr(resultRelationDesc))
-				ExecSetSlotDescriptor(slot, RelationGetDescr(resultRelationDesc));
-			ExecStoreHeapTuple(&deltuple, slot, false);
+				ExecStorePinnedBufferHeapTuple(deltuple, slot, buffer);
+			}
 		}
 
 		rslot = ExecProcessReturning(resultRelInfo, slot, planSlot);
@@ -871,8 +858,6 @@ ldelete:;
 		ExecMaterializeSlot(rslot);
 
 		ExecClearTuple(slot);
-		if (BufferIsValid(delbuffer))
-			ReleaseBuffer(delbuffer);
 
 		return rslot;
 	}
@@ -912,7 +897,7 @@ ExecUpdate(ModifyTableState *mtstate,
 		   EState *estate,
 		   bool canSetTag)
 {
-	HeapTuple	tuple;
+	HeapTuple	updatetuple;
 	ResultRelInfo *resultRelInfo;
 	Relation	resultRelationDesc;
 	HTSU_Result result;
@@ -926,11 +911,7 @@ ExecUpdate(ModifyTableState *mtstate,
 	if (IsBootstrapProcessingMode())
 		elog(ERROR, "cannot UPDATE during bootstrap");
 
-	/*
-	 * get the heap tuple out of the tuple table slot, making sure we have a
-	 * writable copy
-	 */
-	tuple = ExecFetchSlotHeapTuple(slot, true, NULL);
+	ExecMaterializeSlot(slot);
 
 	/*
 	 * get information on the (current) result relation
@@ -942,28 +923,18 @@ ExecUpdate(ModifyTableState *mtstate,
 	if (resultRelInfo->ri_TrigDesc &&
 		resultRelInfo->ri_TrigDesc->trig_update_before_row)
 	{
-		slot = ExecBRUpdateTriggers(estate, epqstate, resultRelInfo,
-									tupleid, oldtuple, slot);
-
-		if (slot == NULL)		/* "do nothing" */
-			return NULL;
-
-		/* trigger might have changed tuple */
-		tuple = ExecFetchSlotHeapTuple(slot, true, NULL);
+		if (!ExecBRUpdateTriggers(estate, epqstate, resultRelInfo,
+								  tupleid, oldtuple, slot))
+			return NULL;        /* "do nothing" */
 	}
 
 	/* INSTEAD OF ROW UPDATE Triggers */
 	if (resultRelInfo->ri_TrigDesc &&
 		resultRelInfo->ri_TrigDesc->trig_update_instead_row)
 	{
-		slot = ExecIRUpdateTriggers(estate, resultRelInfo,
-									oldtuple, slot);
-
-		if (slot == NULL)		/* "do nothing" */
-			return NULL;
-
-		/* trigger might have changed tuple */
-		tuple = ExecFetchSlotHeapTuple(slot, true, NULL);
+		if (!ExecIRUpdateTriggers(estate, resultRelInfo,
+								  oldtuple, slot))
+			return NULL;        /* "do nothing" */
 	}
 	else if (resultRelInfo->ri_FdwRoutine)
 	{
@@ -978,9 +949,6 @@ ExecUpdate(ModifyTableState *mtstate,
 		if (slot == NULL)		/* "do nothing" */
 			return NULL;
 
-		/* FDW might have changed tuple */
-		tuple = ExecFetchSlotHeapTuple(slot, true, NULL);
-
 		/*
 		 * AFTER ROW Triggers or RETURNING expressions might reference the
 		 * tableoid column, so (re-)initialize tts_tableOid before evaluating
@@ -1107,7 +1075,6 @@ lreplace:;
 				else
 				{
 					slot = ExecFilterJunk(resultRelInfo->ri_junkFilter, epqslot);
-					tuple = ExecFetchSlotHeapTuple(slot, true, NULL);
 					goto lreplace;
 				}
 			}
@@ -1178,12 +1145,14 @@ lreplace:;
 		 * needed for referential integrity updates in transaction-snapshot
 		 * mode transactions.
 		 */
-		result = heap_update(resultRelationDesc, tupleid, tuple,
+		updatetuple = ExecFetchSlotHeapTuple(slot, true, NULL);
+		result = heap_update(resultRelationDesc, tupleid,
+							 updatetuple,
 							 estate->es_output_cid,
 							 estate->es_crosscheck_snapshot,
 							 true /* wait for commit */ ,
 							 &hufd, &lockmode);
-		ItemPointerCopy(&tuple->t_self, &slot->tts_tid);
+		ItemPointerCopy(&updatetuple->t_self, &slot->tts_tid);
 
 		switch (result)
 		{
@@ -1249,7 +1218,6 @@ lreplace:;
 					{
 						*tupleid = hufd.ctid;
 						slot = ExecFilterJunk(resultRelInfo->ri_junkFilter, epqslot);
-						tuple = ExecFetchSlotHeapTuple(slot, true, NULL);
 						goto lreplace;
 					}
 				}
@@ -1277,8 +1245,8 @@ lreplace:;
 		 *
 		 * If it's a HOT update, we mustn't insert new index entries.
 		 */
-		if (resultRelInfo->ri_NumIndices > 0 && !HeapTupleIsHeapOnly(tuple))
-			recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self),
+		if (resultRelInfo->ri_NumIndices > 0 && !HeapTupleIsHeapOnly(updatetuple))
+			recheckIndexes = ExecInsertIndexTuples(slot, &(updatetuple->t_self),
 												   estate, false, NULL, NIL);
 	}
 
@@ -1286,7 +1254,7 @@ lreplace:;
 		(estate->es_processed)++;
 
 	/* AFTER ROW UPDATE Triggers */
-	ExecARUpdateTriggers(estate, resultRelInfo, tupleid, oldtuple, tuple,
+	ExecARUpdateTriggers(estate, resultRelInfo, tupleid, oldtuple, slot,
 						 recheckIndexes,
 						 mtstate->operation == CMD_INSERT ?
 						 mtstate->mt_oc_transition_capture :
@@ -1669,7 +1637,6 @@ ExecPrepareTupleRouting(ModifyTableState *mtstate,
 	ModifyTable *node;
 	ResultRelInfo *partrel;
 	PartitionRoutingInfo *partrouteinfo;
-	HeapTuple	tuple;
 	TupleConversionMap *map;
 
 	/*
@@ -1688,9 +1655,6 @@ ExecPrepareTupleRouting(ModifyTableState *mtstate,
 	 */
 	estate->es_result_relation_info = partrel;
 
-	/* Get the heap tuple out of the given slot. */
-	tuple = ExecFetchSlotHeapTuple(slot, true, NULL);
-
 	/*
 	 * If we're capturing transition tuples, we might need to convert from the
 	 * partition rowtype to root partitioned table's rowtype.
@@ -1714,7 +1678,7 @@ ExecPrepareTupleRouting(ModifyTableState *mtstate,
 			 * Otherwise, just remember the original unconverted tuple, to
 			 * avoid a needless round trip conversion.
 			 */
-			mtstate->mt_transition_capture->tcs_original_insert_tuple = tuple;
+			mtstate->mt_transition_capture->tcs_original_insert_tuple = slot;
 			mtstate->mt_transition_capture->tcs_map = NULL;
 		}
 	}
@@ -2542,16 +2506,6 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 	}
 
 	/*
-	 * Set up a tuple table slot for use for trigger output tuples. In a plan
-	 * containing multiple ModifyTable nodes, all can share one such slot, so
-	 * we keep it in the estate. The tuple being inserted doesn't come from a
-	 * buffer.
-	 */
-	if (estate->es_trig_tuple_slot == NULL)
-		estate->es_trig_tuple_slot = ExecInitExtraTupleSlot(estate, NULL,
-															&TTSOpsHeapTuple);
-
-	/*
 	 * Lastly, if this is not the primary (canSetTag) ModifyTable node, add it
 	 * to estate->es_auxmodifytables so that it will be run to completion by
 	 * ExecPostprocessPlan.  (It'd actually work fine to add the primary