From f73b0061795f0c320f96ecfed0c0602ae318d73e Mon Sep 17 00:00:00 2001
From: Ashwin Agrawal <aagrawal@pivotal.io>
Date: Thu, 11 Jul 2019 16:58:50 -0700
Subject: [PATCH v1] Remove IndexBuildCallback's dependency on HeapTuple.

With IndexBuildCallback taking input as HeapTuple, all table AMs
irrespective of storing the tuples in HeapTuple form or not, are
forced to construct HeapTuple, to insert the tuple in Index. Since,
only thing required by the index callbacks is TID and not really the
full tuple, modify callback to only take ItemPointer.
---
 src/backend/access/brin/brin.c           |  4 ++--
 src/backend/access/gin/gininsert.c       |  4 ++--
 src/backend/access/gist/gistbuild.c      |  6 +++---
 src/backend/access/hash/hash.c           |  8 ++++----
 src/backend/access/heap/heapam_handler.c | 11 +++++------
 src/backend/access/nbtree/nbtsort.c      |  8 ++++----
 src/backend/access/spgist/spginsert.c    |  4 ++--
 src/include/access/tableam.h             |  2 +-
 8 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index ae7b729edd9..d27d503c7f1 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -597,7 +597,7 @@ brinendscan(IndexScanDesc scan)
  */
 static void
 brinbuildCallback(Relation index,
-				  HeapTuple htup,
+				  ItemPointer tid,
 				  Datum *values,
 				  bool *isnull,
 				  bool tupleIsAlive,
@@ -607,7 +607,7 @@ brinbuildCallback(Relation index,
 	BlockNumber thisblock;
 	int			i;
 
-	thisblock = ItemPointerGetBlockNumber(&htup->t_self);
+	thisblock = ItemPointerGetBlockNumber(tid);
 
 	/*
 	 * If we're in a block that belongs to a future range, summarize what
diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c
index 55eab146173..d19cbcb2f90 100644
--- a/src/backend/access/gin/gininsert.c
+++ b/src/backend/access/gin/gininsert.c
@@ -273,7 +273,7 @@ ginHeapTupleBulkInsert(GinBuildState *buildstate, OffsetNumber attnum,
 }
 
 static void
-ginBuildCallback(Relation index, HeapTuple htup, Datum *values,
+ginBuildCallback(Relation index, ItemPointer tid, Datum *values,
 				 bool *isnull, bool tupleIsAlive, void *state)
 {
 	GinBuildState *buildstate = (GinBuildState *) state;
@@ -285,7 +285,7 @@ ginBuildCallback(Relation index, HeapTuple htup, Datum *values,
 	for (i = 0; i < buildstate->ginstate.origTupdesc->natts; i++)
 		ginHeapTupleBulkInsert(buildstate, (OffsetNumber) (i + 1),
 							   values[i], isnull[i],
-							   &htup->t_self);
+							   tid);
 
 	/* If we've maxed out our available memory, dump everything to the index */
 	if (buildstate->accum.allocatedMemory >= (Size) maintenance_work_mem * 1024L)
diff --git a/src/backend/access/gist/gistbuild.c b/src/backend/access/gist/gistbuild.c
index ecef0ff0724..2a004937078 100644
--- a/src/backend/access/gist/gistbuild.c
+++ b/src/backend/access/gist/gistbuild.c
@@ -80,7 +80,7 @@ typedef struct
 static void gistInitBuffering(GISTBuildState *buildstate);
 static int	calculatePagesPerBuffer(GISTBuildState *buildstate, int levelStep);
 static void gistBuildCallback(Relation index,
-							  HeapTuple htup,
+							  ItemPointer tid,
 							  Datum *values,
 							  bool *isnull,
 							  bool tupleIsAlive,
@@ -459,7 +459,7 @@ calculatePagesPerBuffer(GISTBuildState *buildstate, int levelStep)
  */
 static void
 gistBuildCallback(Relation index,
-				  HeapTuple htup,
+				  ItemPointer tid,
 				  Datum *values,
 				  bool *isnull,
 				  bool tupleIsAlive,
@@ -473,7 +473,7 @@ gistBuildCallback(Relation index,
 
 	/* form an index tuple and point it at the heap tuple */
 	itup = gistFormTuple(buildstate->giststate, index, values, isnull, true);
-	itup->t_tid = htup->t_self;
+	itup->t_tid = *tid;
 
 	if (buildstate->bufferingMode == GIST_BUFFERING_ACTIVE)
 	{
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index 5cc30dac429..30f994edb50 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -43,7 +43,7 @@ typedef struct
 } HashBuildState;
 
 static void hashbuildCallback(Relation index,
-							  HeapTuple htup,
+							  ItemPointer tid,
 							  Datum *values,
 							  bool *isnull,
 							  bool tupleIsAlive,
@@ -201,7 +201,7 @@ hashbuildempty(Relation index)
  */
 static void
 hashbuildCallback(Relation index,
-				  HeapTuple htup,
+				  ItemPointer tid,
 				  Datum *values,
 				  bool *isnull,
 				  bool tupleIsAlive,
@@ -220,14 +220,14 @@ hashbuildCallback(Relation index,
 
 	/* Either spool the tuple for sorting, or just put it into the index */
 	if (buildstate->spool)
-		_h_spool(buildstate->spool, &htup->t_self,
+		_h_spool(buildstate->spool, tid,
 				 index_values, index_isnull);
 	else
 	{
 		/* form an index tuple and point it at the heap tuple */
 		itup = index_form_tuple(RelationGetDescr(index),
 								index_values, index_isnull);
-		itup->t_tid = htup->t_self;
+		itup->t_tid = *tid;
 		_hash_doinsert(index, itup, buildstate->heapRel);
 		pfree(itup);
 	}
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 09bc6fe98a7..7e18538455b 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -1634,10 +1634,9 @@ heapam_index_build_range_scan(Relation heapRelation,
 			 * For a heap-only tuple, pretend its TID is that of the root. See
 			 * src/backend/access/heap/README.HOT for discussion.
 			 */
-			HeapTupleData rootTuple;
+			ItemPointerData tid;
 			OffsetNumber offnum;
 
-			rootTuple = *heapTuple;
 			offnum = ItemPointerGetOffsetNumber(&heapTuple->t_self);
 
 			if (!OffsetNumberIsValid(root_offsets[offnum - 1]))
@@ -1648,17 +1647,17 @@ heapam_index_build_range_scan(Relation heapRelation,
 										 offnum,
 										 RelationGetRelationName(heapRelation))));
 
-			ItemPointerSetOffsetNumber(&rootTuple.t_self,
-									   root_offsets[offnum - 1]);
+			ItemPointerSet(&tid, ItemPointerGetBlockNumber(&heapTuple->t_self),
+						   root_offsets[offnum - 1]);
 
 			/* Call the AM's callback routine to process the tuple */
-			callback(indexRelation, &rootTuple, values, isnull, tupleIsAlive,
+			callback(indexRelation, &tid, values, isnull, tupleIsAlive,
 					 callback_state);
 		}
 		else
 		{
 			/* Call the AM's callback routine to process the tuple */
-			callback(indexRelation, heapTuple, values, isnull, tupleIsAlive,
+			callback(indexRelation, &heapTuple->t_self, values, isnull, tupleIsAlive,
 					 callback_state);
 		}
 	}
diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index d0b9013caf4..7943d471c1f 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -278,7 +278,7 @@ static void _bt_spooldestroy(BTSpool *btspool);
 static void _bt_spool(BTSpool *btspool, ItemPointer self,
 					  Datum *values, bool *isnull);
 static void _bt_leafbuild(BTSpool *btspool, BTSpool *btspool2);
-static void _bt_build_callback(Relation index, HeapTuple htup, Datum *values,
+static void _bt_build_callback(Relation index, ItemPointer tid, Datum *values,
 							   bool *isnull, bool tupleIsAlive, void *state);
 static Page _bt_blnewpage(uint32 level);
 static BTPageState *_bt_pagestate(BTWriteState *wstate, uint32 level);
@@ -594,7 +594,7 @@ _bt_leafbuild(BTSpool *btspool, BTSpool *btspool2)
  */
 static void
 _bt_build_callback(Relation index,
-				   HeapTuple htup,
+				   ItemPointer tid,
 				   Datum *values,
 				   bool *isnull,
 				   bool tupleIsAlive,
@@ -607,12 +607,12 @@ _bt_build_callback(Relation index,
 	 * processing
 	 */
 	if (tupleIsAlive || buildstate->spool2 == NULL)
-		_bt_spool(buildstate->spool, &htup->t_self, values, isnull);
+		_bt_spool(buildstate->spool, tid, values, isnull);
 	else
 	{
 		/* dead tuples are put into spool2 */
 		buildstate->havedead = true;
-		_bt_spool(buildstate->spool2, &htup->t_self, values, isnull);
+		_bt_spool(buildstate->spool2, tid, values, isnull);
 	}
 
 	buildstate->indtuples += 1;
diff --git a/src/backend/access/spgist/spginsert.c b/src/backend/access/spgist/spginsert.c
index b40bd440cf0..dd9088741cf 100644
--- a/src/backend/access/spgist/spginsert.c
+++ b/src/backend/access/spgist/spginsert.c
@@ -40,7 +40,7 @@ typedef struct
 
 /* Callback to process one heap tuple during table_index_build_scan */
 static void
-spgistBuildCallback(Relation index, HeapTuple htup, Datum *values,
+spgistBuildCallback(Relation index, ItemPointer tid, Datum *values,
 					bool *isnull, bool tupleIsAlive, void *state)
 {
 	SpGistBuildState *buildstate = (SpGistBuildState *) state;
@@ -55,7 +55,7 @@ spgistBuildCallback(Relation index, HeapTuple htup, Datum *values,
 	 * lock on some buffer.  So we need to be willing to retry.  We can flush
 	 * any temp data when retrying.
 	 */
-	while (!spgdoinsert(index, &buildstate->spgstate, &htup->t_self,
+	while (!spgdoinsert(index, &buildstate->spgstate, tid,
 						*values, *isnull))
 	{
 		MemoryContextReset(buildstate->tmpCtx);
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index c2b0481e7e5..14f23362357 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -141,7 +141,7 @@ typedef struct TM_FailureData
 
 /* Typedef for callback function for table_index_build_scan */
 typedef void (*IndexBuildCallback) (Relation index,
-									HeapTuple htup,
+									ItemPointer tid,
 									Datum *values,
 									bool *isnull,
 									bool tupleIsAlive,
-- 
2.19.1

