Rejigger materializing and fetching a HeapTuple from a slot.

Previously materializing a slot always returned a HeapTuple. As
current work aims to reduce the reliance on HeapTuples (so other
storage systems can work efficiently), that needs to change. Thus
split the tasks of materializing a slot (i.e. making it independent
from the underlying storage / other memory contexts) from fetching a
HeapTuple from the slot.  For brevity, allow to fetch a HeapTuple from
a slot and materializing the slot at the same time, controlled by a
parameter.

For now some callers of ExecFetchSlotHeapTuple, with materialize =
true, expect that changes to the heap tuple will be reflected in the
underlying slot.  Those places will be adapted in due course, so while
not pretty, that's OK for now.

Also rename ExecFetchSlotTuple to ExecFetchSlotHeapTupleDatum and
ExecFetchSlotTupleDatum to ExecFetchSlotHeapTupleDatum, as it's likely
that future storage methods will need similar methods. There already
is ExecFetchSlotMinimalTuple, so the new names make the naming scheme
more coherent.

Author: Ashutosh Bapat and Andres Freund, with changes by Amit Khandekar
Discussion: https://postgr.es/m/20181105210039.hh4vvi4vwoq5ba2q@alap3.anarazel.de

1 files changed, 31 insertions, 15 deletions

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index ccb5706c162..d6f33ecbd04 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2517,7 +2517,8 @@ ExecBRInsertTriggers(EState *estate, ResultRelInfo *relinfo,
 					 TupleTableSlot *slot)
 {
 	TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
-	HeapTuple	slottuple = ExecMaterializeSlot(slot);
+	bool		should_free;
+	HeapTuple	slottuple = ExecFetchSlotHeapTuple(slot, true, &should_free);
 	HeapTuple	newtuple = slottuple;
 	HeapTuple	oldtuple;
 	TriggerData LocTriggerData;
@@ -2556,7 +2557,11 @@ ExecBRInsertTriggers(EState *estate, ResultRelInfo *relinfo,
 		if (oldtuple != newtuple && oldtuple != slottuple)
 			heap_freetuple(oldtuple);
 		if (newtuple == NULL)
+		{
+			if (should_free)
+				heap_freetuple(slottuple);
 			return NULL;		/* "do nothing" */
+		}
 	}
 
 	if (newtuple != slottuple)
@@ -2575,6 +2580,9 @@ ExecBRInsertTriggers(EState *estate, ResultRelInfo *relinfo,
 		ExecStoreHeapTuple(newtuple, newslot, false);
 		slot = newslot;
 	}
+
+	if (should_free)
+		heap_freetuple(slottuple);
 	return slot;
 }
 
@@ -2598,7 +2606,8 @@ ExecIRInsertTriggers(EState *estate, ResultRelInfo *relinfo,
 					 TupleTableSlot *slot)
 {
 	TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
-	HeapTuple	slottuple = ExecMaterializeSlot(slot);
+	bool		should_free;
+	HeapTuple	slottuple = ExecFetchSlotHeapTuple(slot, true, &should_free);
 	HeapTuple	newtuple = slottuple;
 	HeapTuple	oldtuple;
 	TriggerData LocTriggerData;
@@ -2637,7 +2646,11 @@ ExecIRInsertTriggers(EState *estate, ResultRelInfo *relinfo,
 		if (oldtuple != newtuple && oldtuple != slottuple)
 			heap_freetuple(oldtuple);
 		if (newtuple == NULL)
+		{
+			if (should_free)
+				heap_freetuple(slottuple);
 			return NULL;		/* "do nothing" */
+		}
 	}
 
 	if (newtuple != slottuple)
@@ -2656,6 +2669,9 @@ ExecIRInsertTriggers(EState *estate, ResultRelInfo *relinfo,
 		ExecStoreHeapTuple(newtuple, newslot, false);
 		slot = newslot;
 	}
+
+	if (should_free)
+		heap_freetuple(slottuple);
 	return slot;
 }
 
@@ -2976,7 +2992,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 					 TupleTableSlot *slot)
 {
 	TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
-	HeapTuple	slottuple = ExecMaterializeSlot(slot);
+	HeapTuple	slottuple = ExecFetchSlotHeapTuple(slot, true, NULL);
 	HeapTuple	newtuple = slottuple;
 	TriggerData LocTriggerData;
 	HeapTuple	trigtuple;
@@ -3018,7 +3034,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
 	if (newSlot != NULL)
 	{
 		slot = ExecFilterJunk(relinfo->ri_junkFilter, newSlot);
-		slottuple = ExecMaterializeSlot(slot);
+		slottuple = ExecFetchSlotHeapTuple(slot, true, NULL);
 		newtuple = slottuple;
 	}
 
@@ -3132,7 +3148,7 @@ ExecIRUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
 					 HeapTuple trigtuple, TupleTableSlot *slot)
 {
 	TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
-	HeapTuple	slottuple = ExecMaterializeSlot(slot);
+	HeapTuple	slottuple = ExecFetchSlotHeapTuple(slot, true, NULL);
 	HeapTuple	newtuple = slottuple;
 	TriggerData LocTriggerData;
 	HeapTuple	oldtuple;
@@ -4262,22 +4278,22 @@ AfterTriggerExecute(AfterTriggerEvent event,
 		case AFTER_TRIGGER_FDW_REUSE:
 
 			/*
-			 * Using ExecMaterializeSlot() rather than ExecFetchSlotTuple()
-			 * ensures that tg_trigtuple does not reference tuplestore memory.
-			 * (It is formally possible for the trigger function to queue
-			 * trigger events that add to the same tuplestore, which can push
-			 * other tuples out of memory.)  The distinction is academic,
-			 * because we start with a minimal tuple that ExecFetchSlotTuple()
-			 * must materialize anyway.
+			 * Materialize tuple in the slot so that tg_trigtuple does not
+			 * reference tuplestore memory.  (It is formally possible for the
+			 * trigger function to queue trigger events that add to the same
+			 * tuplestore, which can push other tuples out of memory.)  The
+			 * distinction is academic, because we start with a minimal tuple
+			 * that is stored as a heap tuple, constructed in different memory
+			 * context, in the slot anyway.
 			 */
-			LocTriggerData.tg_trigtuple =
-				ExecMaterializeSlot(trig_tuple_slot1);
+			LocTriggerData.tg_trigtuple = ExecFetchSlotHeapTuple(trig_tuple_slot1,
+																	true, NULL);
 			LocTriggerData.tg_trigtuplebuf = InvalidBuffer;
 
 			LocTriggerData.tg_newtuple =
 				((evtshared->ats_event & TRIGGER_EVENT_OPMASK) ==
 				 TRIGGER_EVENT_UPDATE) ?
-				ExecMaterializeSlot(trig_tuple_slot2) : NULL;
+				ExecFetchSlotHeapTuple(trig_tuple_slot2, true, NULL) : NULL;
 			LocTriggerData.tg_newtuplebuf = InvalidBuffer;
 
 			break;