Subject: [PATCH v18 04/18] tableam: Add fetch_row_version.

From: Andres Freund <[email-masked]>
Date: Thu, 7 Mar 2019 16:30:12 -0800

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/access/heap/heapam_handler.c | 28 ++++++++++
 src/backend/commands/trigger.c           | 70 ++++--------------------
 src/backend/executor/execMain.c          | 12 +---
 src/backend/executor/nodeLockRows.c      | 10 +---
 src/backend/executor/nodeModifyTable.c   | 23 ++------
 src/backend/executor/nodeTidscan.c       | 15 +----
 src/include/access/tableam.h             | 25 +++++++++
 7 files changed, 73 insertions(+), 110 deletions(-)

diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 3285197c558..318e393dbde 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -148,6 +148,33 @@ heapam_index_fetch_tuple(struct IndexFetchTableData *scan,
  * ------------------------------------------------------------------------
  */
 
+static bool
+heapam_fetch_row_version(Relation relation,
+						 ItemPointer tid,
+						 Snapshot snapshot,
+						 TupleTableSlot *slot,
+						 Relation stats_relation)
+{
+	BufferHeapTupleTableSlot *bslot = (BufferHeapTupleTableSlot *) slot;
+	Buffer		buffer;
+
+	Assert(TTS_IS_BUFFERTUPLE(slot));
+
+	if (heap_fetch(relation, tid, snapshot, &bslot->base.tupdata, &buffer, stats_relation))
+	{
+		/* store in slot, transferring existing pin */
+		ExecStorePinnedBufferHeapTuple(&bslot->base.tupdata, slot, buffer);
+
+		slot->tts_tableOid = RelationGetRelid(relation);
+
+		return true;
+	}
+
+	slot->tts_tableOid = RelationGetRelid(relation);
+
+	return false;
+}
+
 static bool
 heapam_tuple_satisfies_snapshot(Relation rel, TupleTableSlot *slot,
 								Snapshot snapshot)
@@ -546,6 +573,7 @@ static const TableAmRoutine heapam_methods = {
 	.tuple_update = heapam_heap_update,
 	.tuple_lock = heapam_lock_tuple,
 
+	.tuple_fetch_row_version = heapam_fetch_row_version,
 	.tuple_satisfies_snapshot = heapam_tuple_satisfies_snapshot,
 };
 
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 5cc15bcfef0..6fbf0c2b81e 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -14,10 +14,11 @@
 #include "postgres.h"
 
 #include "access/genam.h"
-#include "access/heapam.h"
-#include "access/tableam.h"
-#include "access/sysattr.h"
 #include "access/htup_details.h"
+#include "access/relation.h"
+#include "access/sysattr.h"
+#include "access/table.h"
+#include "access/tableam.h"
 #include "access/xact.h"
 #include "catalog/catalog.h"
 #include "catalog/dependency.h"
@@ -3382,43 +3383,8 @@ GetTupleForTrigger(EState *estate,
 	}
 	else
 	{
-
-		Page		page;
-		ItemId		lp;
-		Buffer		buffer;
-		BufferHeapTupleTableSlot *boldslot;
-		HeapTuple tuple;
-
-		Assert(TTS_IS_BUFFERTUPLE(oldslot));
-		ExecClearTuple(oldslot);
-		boldslot = (BufferHeapTupleTableSlot *) oldslot;
-		tuple = &boldslot->base.tupdata;
-
-		buffer = ReadBuffer(relation, ItemPointerGetBlockNumber(tid));
-
-		/*
-		 * Although we already know this tuple is valid, we must lock the
-		 * buffer to ensure that no one has a buffer cleanup lock; otherwise
-		 * they might move the tuple while we try to copy it.  But we can
-		 * release the lock before actually doing the heap_copytuple call,
-		 * since holding pin is sufficient to prevent anyone from getting a
-		 * cleanup lock they don't already hold.
-		 */
-		LockBuffer(buffer, BUFFER_LOCK_SHARE);
-
-		page = BufferGetPage(buffer);
-		lp = PageGetItemId(page, ItemPointerGetOffsetNumber(tid));
-
-		Assert(ItemIdIsNormal(lp));
-
-		tuple->t_data = (HeapTupleHeader) PageGetItem(page, lp);
-		tuple->t_len = ItemIdGetLength(lp);
-		tuple->t_self = *tid;
-		tuple->t_tableOid = RelationGetRelid(relation);
-
-		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
-
-		ExecStorePinnedBufferHeapTuple(tuple, oldslot, buffer);
+		if (!table_fetch_row_version(relation, tid, SnapshotAny, oldslot, NULL))
+			elog(ERROR, "couldn't fetch tuple");
 	}
 
 	return true;
@@ -4197,8 +4163,6 @@ AfterTriggerExecute(EState *estate,
 	AfterTriggerShared evtshared = GetTriggerSharedData(event);
 	Oid			tgoid = evtshared->ats_tgoid;
 	TriggerData LocTriggerData;
-	HeapTupleData tuple1;
-	HeapTupleData tuple2;
 	HeapTuple	rettuple;
 	int			tgindx;
 	bool		should_free_trig = false;
@@ -4275,19 +4239,12 @@ AfterTriggerExecute(EState *estate,
 		default:
 			if (ItemPointerIsValid(&(event->ate_ctid1)))
 			{
-				Buffer buffer;
-
 				LocTriggerData.tg_trigslot = ExecGetTriggerOldSlot(estate, relInfo);
 
-				ItemPointerCopy(&(event->ate_ctid1), &(tuple1.t_self));
-				if (!heap_fetch(rel, &(tuple1.t_self), SnapshotAny, &tuple1, &buffer, NULL))
+				if (!table_fetch_row_version(rel, &(event->ate_ctid1), SnapshotAny, LocTriggerData.tg_trigslot, NULL))
 					elog(ERROR, "failed to fetch tuple1 for AFTER trigger");
-				ExecStorePinnedBufferHeapTuple(&tuple1,
-											   LocTriggerData.tg_trigslot,
-											   buffer);
 				LocTriggerData.tg_trigtuple =
-					ExecFetchSlotHeapTuple(LocTriggerData.tg_trigslot, false,
-										   &should_free_trig);
+					ExecFetchSlotHeapTuple(LocTriggerData.tg_trigslot, false, &should_free_trig);
 			}
 			else
 			{
@@ -4299,19 +4256,12 @@ AfterTriggerExecute(EState *estate,
 				AFTER_TRIGGER_2CTID &&
 				ItemPointerIsValid(&(event->ate_ctid2)))
 			{
-				Buffer buffer;
-
 				LocTriggerData.tg_newslot = ExecGetTriggerNewSlot(estate, relInfo);
 
-				ItemPointerCopy(&(event->ate_ctid2), &(tuple2.t_self));
-				if (!heap_fetch(rel, &(tuple2.t_self), SnapshotAny, &tuple2, &buffer, NULL))
+				if (!table_fetch_row_version(rel, &(event->ate_ctid2), SnapshotAny, LocTriggerData.tg_newslot, NULL))
 					elog(ERROR, "failed to fetch tuple2 for AFTER trigger");
-				ExecStorePinnedBufferHeapTuple(&tuple2,
-											   LocTriggerData.tg_newslot,
-											   buffer);
 				LocTriggerData.tg_newtuple =
-					ExecFetchSlotHeapTuple(LocTriggerData.tg_newslot, false,
-										   &should_free_new);
+					ExecFetchSlotHeapTuple(LocTriggerData.tg_newslot, false, &should_free_new);
 			}
 			else
 			{
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 8723e32dd58..e70e9f08e42 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -2638,17 +2638,9 @@ EvalPlanQualFetchRowMarks(EPQState *epqstate)
 			else
 			{
 				/* ordinary table, fetch the tuple */
-				HeapTupleData tuple;
-				Buffer		buffer;
-
-				tuple.t_self = *((ItemPointer) DatumGetPointer(datum));
-				if (!heap_fetch(erm->relation, &tuple.t_self, SnapshotAny,
-								&tuple, &buffer, NULL))
+				if (!table_fetch_row_version(erm->relation, (ItemPointer) DatumGetPointer(datum),
+											 SnapshotAny, slot, NULL))
 					elog(ERROR, "failed to fetch tuple for EvalPlanQual recheck");
-
-				/* successful, store tuple */
-				ExecStorePinnedBufferHeapTuple(&tuple, slot, buffer);
-				ExecMaterializeSlot(slot);
 			}
 		}
 		else
diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c
index 91f46b88ed8..aedc5297e3b 100644
--- a/src/backend/executor/nodeLockRows.c
+++ b/src/backend/executor/nodeLockRows.c
@@ -21,7 +21,6 @@
 
 #include "postgres.h"
 
-#include "access/heapam.h"
 #include "access/htup_details.h"
 #include "access/tableam.h"
 #include "access/xact.h"
@@ -275,8 +274,6 @@ lnext:
 			ExecAuxRowMark *aerm = (ExecAuxRowMark *) lfirst(lc);
 			ExecRowMark *erm = aerm->rowmark;
 			TupleTableSlot *markSlot;
-			HeapTupleData tuple;
-			Buffer buffer;
 
 			markSlot = EvalPlanQualSlot(&node->lr_epqstate, erm->relation, erm->rti);
 
@@ -297,12 +294,9 @@ lnext:
 			Assert(ItemPointerIsValid(&(erm->curCtid)));
 
 			/* okay, fetch the tuple */
-			tuple.t_self = erm->curCtid;
-			if (!heap_fetch(erm->relation, &tuple.t_self, SnapshotAny, &tuple, &buffer,
-							NULL))
+			if (!table_fetch_row_version(erm->relation, &erm->curCtid, SnapshotAny, markSlot,
+							   NULL))
 				elog(ERROR, "failed to fetch tuple for EvalPlanQual recheck");
-			ExecStorePinnedBufferHeapTuple(&tuple, markSlot, buffer);
-			ExecMaterializeSlot(markSlot);
 			/* successful, use tuple in slot */
 		}
 
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 5b079d8302a..b4247ec33b4 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -230,18 +230,15 @@ ExecCheckTIDVisible(EState *estate,
 					TupleTableSlot *tempSlot)
 {
 	Relation	rel = relinfo->ri_RelationDesc;
-	Buffer		buffer;
-	HeapTupleData tuple;
 
 	/* Redundantly check isolation level */
 	if (!IsolationUsesXactSnapshot())
 		return;
 
-	tuple.t_self = *tid;
-	if (!heap_fetch(rel, &tuple.t_self, SnapshotAny, &tuple, &buffer, NULL))
+	if (!table_fetch_row_version(rel, tid, SnapshotAny, tempSlot, NULL))
 		elog(ERROR, "failed to fetch conflicting tuple for ON CONFLICT");
-	ExecStorePinnedBufferHeapTuple(&tuple, tempSlot, buffer);
 	ExecCheckHeapTupleVisible(estate, rel, tempSlot);
+	ExecClearTuple(tempSlot);
 }
 
 /* ----------------------------------------------------------------
@@ -851,21 +848,9 @@ ldelete:;
 			}
 			else
 			{
-				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, &deltuple->t_self, SnapshotAny,
-								deltuple, &buffer, NULL))
+				if (!table_fetch_row_version(resultRelationDesc, tupleid, SnapshotAny,
+											 slot, NULL))
 					elog(ERROR, "failed to fetch deleted tuple for DELETE RETURNING");
-
-				ExecStorePinnedBufferHeapTuple(deltuple, slot, buffer);
 			}
 		}
 
diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c
index b819cf2383e..7d496cc4105 100644
--- a/src/backend/executor/nodeTidscan.c
+++ b/src/backend/executor/nodeTidscan.c
@@ -310,7 +310,6 @@ TidNext(TidScanState *node)
 	Relation	heapRelation;
 	HeapTuple	tuple;
 	TupleTableSlot *slot;
-	Buffer		buffer = InvalidBuffer;
 	ItemPointerData *tidList;
 	int			numTids;
 	bool		bBackward;
@@ -376,19 +375,9 @@ TidNext(TidScanState *node)
 		if (node->tss_isCurrentOf)
 			heap_get_latest_tid(heapRelation, snapshot, &tuple->t_self);
 
-		if (heap_fetch(heapRelation, &tuple->t_self, snapshot, tuple, &buffer, NULL))
-		{
-			/*
-			 * Store the scanned tuple in the scan tuple slot of the scan
-			 * state, transferring the pin to the slot.
-			 */
-			ExecStorePinnedBufferHeapTuple(tuple, /* tuple to store */
-										   slot,	/* slot to store in */
-										   buffer);	/* buffer associated with
-													 * tuple */
-
+		if (table_fetch_row_version(heapRelation, &tuple->t_self, snapshot, slot, NULL))
 			return slot;
-		}
+
 		/* Bad TID or failed snapshot qual; try next */
 		if (bBackward)
 			node->tss_TidPtr--;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index b34e903d84d..abbd7f63385 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -218,6 +218,13 @@ typedef struct TableAmRoutine
 	 * ------------------------------------------------------------------------
 	 */
 
+
+	bool		(*tuple_fetch_row_version) (Relation rel,
+											ItemPointer tid,
+											Snapshot snapshot,
+											TupleTableSlot *slot,
+											Relation stats_relation);
+
 	/*
 	 * Does the tuple in `slot` satisfy `snapshot`?  The slot needs to be of
 	 * the appropriate type for the AM.
@@ -542,6 +549,24 @@ table_index_fetch_tuple(struct IndexFetchTableData *scan,
  * ------------------------------------------------------------------------
  */
 
+
+/*
+ *	table_fetch_row_version		- retrieve tuple with given tid
+ *
+ *  XXX: This shouldn't just take a tid, but tid + additional information
+ */
+static inline bool
+table_fetch_row_version(Relation rel,
+						ItemPointer tid,
+						Snapshot snapshot,
+						TupleTableSlot *slot,
+						Relation stats_relation)
+{
+	return rel->rd_tableam->tuple_fetch_row_version(rel, tid,
+													snapshot, slot,
+													stats_relation);
+}
+
 /*
  * Return true iff tuple in slot satisfies the snapshot.
  *
-- 
2.21.0.dirty

--yvn3crbc4qf4vymf
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v18-0005-tableam-Add-use-tableam_fetch_follow_check.patch"