Remove direct uses of ItemPointer.{ip_blkid,ip_posid}

There are no functional changes here; this simply encapsulates knowledge
of the ItemPointerData struct so that a future patch can change things
without more breakage.

All direct users of ip_blkid and ip_posid are changed to use existing
macros ItemPointerGetBlockNumber and ItemPointerGetOffsetNumber
respectively.  For callers where that's inappropriate (because they
Assert that the itempointer is is valid-looking), add
ItemPointerGetBlockNumberNoCheck and ItemPointerGetOffsetNumberNoCheck,
which lack the assertion but are otherwise identical.

Author: Pavan Deolasee
Discussion: https://postgr.es/m/CABOikdNnFon4cJiL=h1mZH3bgUeU+sWHuU4Yr8AB=j3A2p1GiA@mail.gmail.com

contrib/pageinspect/btreefuncs.c

@@ -363,8 +363,8 @@ bt_page_items(PG_FUNCTION_ARGS)
 		j = 0;
 		values[j++] = psprintf("%d", uargs->offset);
 		values[j++] = psprintf("(%u,%u)",
-							   BlockIdGetBlockNumber(&(itup->t_tid.ip_blkid)),
-							   itup->t_tid.ip_posid);
+							   ItemPointerGetBlockNumberNoCheck(&itup->t_tid),
+							ItemPointerGetOffsetNumberNoCheck(&itup->t_tid));
 		values[j++] = psprintf("%d", (int) IndexTupleSize(itup));
 		values[j++] = psprintf("%c", IndexTupleHasNulls(itup) ? 't' : 'f');
 		values[j++] = psprintf("%c", IndexTupleHasVarwidths(itup) ? 't' : 'f');

contrib/pgstattuple/pgstattuple.c

@@ -356,7 +356,7 @@ pgstat_heap(Relation rel, FunctionCallInfo fcinfo)
 		 * heap_getnext may find no tuples on a given page, so we cannot
 		 * simply examine the pages returned by the heap scan.
 		 */
-		tupblock = BlockIdGetBlockNumber(&tuple->t_self.ip_blkid);
+		tupblock = ItemPointerGetBlockNumber(&tuple->t_self);
 
 		while (block <= tupblock)
 		{

src/backend/access/gin/ginget.c

@@ -626,8 +626,9 @@ entryLoadMoreItems(GinState *ginstate, GinScanEntry entry,
 		}
 		else
 		{
-			entry->btree.itemptr = advancePast;
-			entry->btree.itemptr.ip_posid++;
+			ItemPointerSet(&entry->btree.itemptr,
+						   GinItemPointerGetBlockNumber(&advancePast),
+			  OffsetNumberNext(GinItemPointerGetOffsetNumber(&advancePast)));
 		}
 		entry->btree.fullScan = false;
 		stack = ginFindLeafPage(&entry->btree, true, snapshot);
@@ -979,15 +980,17 @@ keyGetItem(GinState *ginstate, MemoryContext tempCtx, GinScanKey key,
 		if (GinItemPointerGetBlockNumber(&advancePast) <
 			GinItemPointerGetBlockNumber(&minItem))
 		{
-			advancePast.ip_blkid = minItem.ip_blkid;
-			advancePast.ip_posid = 0;
+			ItemPointerSet(&advancePast,
+						   GinItemPointerGetBlockNumber(&minItem),
+						   InvalidOffsetNumber);
 		}
 	}
 	else
 	{
-		Assert(minItem.ip_posid > 0);
-		advancePast = minItem;
-		advancePast.ip_posid--;
+		Assert(GinItemPointerGetOffsetNumber(&minItem) > 0);
+		ItemPointerSet(&advancePast,
+					   GinItemPointerGetBlockNumber(&minItem),
+				  OffsetNumberPrev(GinItemPointerGetOffsetNumber(&minItem)));
 	}
 
 	/*
@@ -1245,15 +1248,17 @@ scanGetItem(IndexScanDesc scan, ItemPointerData advancePast,
 				if (GinItemPointerGetBlockNumber(&advancePast) <
 					GinItemPointerGetBlockNumber(&key->curItem))
 				{
-					advancePast.ip_blkid = key->curItem.ip_blkid;
-					advancePast.ip_posid = 0;
+					ItemPointerSet(&advancePast,
+								 GinItemPointerGetBlockNumber(&key->curItem),
+								   InvalidOffsetNumber);
 				}
 			}
 			else
 			{
-				Assert(key->curItem.ip_posid > 0);
-				advancePast = key->curItem;
-				advancePast.ip_posid--;
+				Assert(GinItemPointerGetOffsetNumber(&key->curItem) > 0);
+				ItemPointerSet(&advancePast,
+							   GinItemPointerGetBlockNumber(&key->curItem),
+							   OffsetNumberPrev(GinItemPointerGetOffsetNumber(&key->curItem)));
 			}
 
 			/*

src/backend/access/gin/ginpostinglist.c

@@ -79,13 +79,11 @@ itemptr_to_uint64(const ItemPointer iptr)
 	uint64		val;
 
 	Assert(ItemPointerIsValid(iptr));
-	Assert(iptr->ip_posid < (1 << MaxHeapTuplesPerPageBits));
+	Assert(GinItemPointerGetOffsetNumber(iptr) < (1 << MaxHeapTuplesPerPageBits));
 
-	val = iptr->ip_blkid.bi_hi;
-	val <<= 16;
-	val |= iptr->ip_blkid.bi_lo;
+	val = GinItemPointerGetBlockNumber(iptr);
 	val <<= MaxHeapTuplesPerPageBits;
-	val |= iptr->ip_posid;
+	val |= GinItemPointerGetOffsetNumber(iptr);
 
 	return val;
 }
@@ -93,11 +91,9 @@ itemptr_to_uint64(const ItemPointer iptr)
 static inline void
 uint64_to_itemptr(uint64 val, ItemPointer iptr)
 {
-	iptr->ip_posid = val & ((1 << MaxHeapTuplesPerPageBits) - 1);
+	GinItemPointerSetOffsetNumber(iptr, val & ((1 << MaxHeapTuplesPerPageBits) - 1));
 	val = val >> MaxHeapTuplesPerPageBits;
-	iptr->ip_blkid.bi_lo = val & 0xFFFF;
-	val = val >> 16;
-	iptr->ip_blkid.bi_hi = val & 0xFFFF;
+	GinItemPointerSetBlockNumber(iptr, val);
 
 	Assert(ItemPointerIsValid(iptr));
 }

src/backend/replication/logical/reorderbuffer.c

@@ -3013,8 +3013,8 @@ DisplayMapping(HTAB *tuplecid_data)
 			 ent->key.relnode.dbNode,
 			 ent->key.relnode.spcNode,
 			 ent->key.relnode.relNode,
-			 BlockIdGetBlockNumber(&ent->key.tid.ip_blkid),
-			 ent->key.tid.ip_posid,
+			 ItemPointerGetBlockNumber(&ent->key.tid),
+			 ItemPointerGetOffsetNumber(&ent->key.tid),
 			 ent->cmin,
 			 ent->cmax
 			);

src/backend/storage/page/itemptr.c

@@ -52,20 +52,21 @@ int32
 ItemPointerCompare(ItemPointer arg1, ItemPointer arg2)
 {
 	/*
-	 * Don't use ItemPointerGetBlockNumber or ItemPointerGetOffsetNumber here,
-	 * because they assert ip_posid != 0 which might not be true for a
-	 * user-supplied TID.
+	 * Use ItemPointerGet{Offset,Block}NumberNoCheck to avoid asserting
+	 * ip_posid != 0, which may not be true for a user-supplied TID.
 	 */
-	BlockNumber b1 = BlockIdGetBlockNumber(&(arg1->ip_blkid));
-	BlockNumber b2 = BlockIdGetBlockNumber(&(arg2->ip_blkid));
+	BlockNumber b1 = ItemPointerGetBlockNumberNoCheck(arg1);
+	BlockNumber b2 = ItemPointerGetBlockNumberNoCheck(arg2);
 
 	if (b1 < b2)
 		return -1;
 	else if (b1 > b2)
 		return 1;
-	else if (arg1->ip_posid < arg2->ip_posid)
+	else if (ItemPointerGetOffsetNumberNoCheck(arg1) <
+			 ItemPointerGetOffsetNumberNoCheck(arg2))
 		return -1;
-	else if (arg1->ip_posid > arg2->ip_posid)
+	else if (ItemPointerGetOffsetNumberNoCheck(arg1) >
+			 ItemPointerGetOffsetNumberNoCheck(arg2))
 		return 1;
 	else
 		return 0;

src/backend/utils/adt/tid.c

@@ -109,8 +109,8 @@ tidout(PG_FUNCTION_ARGS)
 	OffsetNumber offsetNumber;
 	char		buf[32];
 
-	blockNumber = BlockIdGetBlockNumber(&(itemPtr->ip_blkid));
-	offsetNumber = itemPtr->ip_posid;
+	blockNumber = ItemPointerGetBlockNumberNoCheck(itemPtr);
+	offsetNumber = ItemPointerGetOffsetNumberNoCheck(itemPtr);
 
 	/* Perhaps someday we should output this as a record. */
 	snprintf(buf, sizeof(buf), "(%u,%u)", blockNumber, offsetNumber);
@@ -146,18 +146,13 @@ Datum
 tidsend(PG_FUNCTION_ARGS)
 {
 	ItemPointer itemPtr = PG_GETARG_ITEMPOINTER(0);
-	BlockId		blockId;
-	BlockNumber blockNumber;
-	OffsetNumber offsetNumber;
 	StringInfoData buf;
 
-	blockId = &(itemPtr->ip_blkid);
-	blockNumber = BlockIdGetBlockNumber(blockId);
-	offsetNumber = itemPtr->ip_posid;
-
 	pq_begintypsend(&buf);
-	pq_sendint(&buf, blockNumber, sizeof(blockNumber));
-	pq_sendint(&buf, offsetNumber, sizeof(offsetNumber));
+	pq_sendint(&buf, ItemPointerGetBlockNumberNoCheck(itemPtr),
+			   sizeof(BlockNumber));
+	pq_sendint(&buf, ItemPointerGetOffsetNumberNoCheck(itemPtr),
+			   sizeof(OffsetNumber));
 	PG_RETURN_BYTEA_P(pq_endtypsend(&buf));
 }
 

src/include/access/gin_private.h

@@ -460,8 +460,8 @@ extern ItemPointer ginMergeItemPointers(ItemPointerData *a, uint32 na,
 static inline int
 ginCompareItemPointers(ItemPointer a, ItemPointer b)
 {
-	uint64		ia = (uint64) a->ip_blkid.bi_hi << 32 | (uint64) a->ip_blkid.bi_lo << 16 | a->ip_posid;
-	uint64		ib = (uint64) b->ip_blkid.bi_hi << 32 | (uint64) b->ip_blkid.bi_lo << 16 | b->ip_posid;
+	uint64		ia = (uint64) GinItemPointerGetBlockNumber(a) << 32 | GinItemPointerGetOffsetNumber(a);
+	uint64		ib = (uint64) GinItemPointerGetBlockNumber(b) << 32 | GinItemPointerGetOffsetNumber(b);
 
 	if (ia == ib)
 		return 0;
@@ -471,6 +471,6 @@ ginCompareItemPointers(ItemPointer a, ItemPointer b)
 		return -1;
 }
 
-extern int ginTraverseLock(Buffer buffer, bool searchMode);
+extern int	ginTraverseLock(Buffer buffer, bool searchMode);
 
 #endif   /* GIN_PRIVATE_H */

src/include/access/ginblock.h

@@ -132,10 +132,17 @@ typedef struct GinMetaPageData
  * to avoid Asserts, since sometimes the ip_posid isn't "valid"
  */
 #define GinItemPointerGetBlockNumber(pointer) \
-	BlockIdGetBlockNumber(&(pointer)->ip_blkid)
+	(ItemPointerGetBlockNumberNoCheck(pointer))
 
 #define GinItemPointerGetOffsetNumber(pointer) \
-	((pointer)->ip_posid)
+	(ItemPointerGetOffsetNumberNoCheck(pointer))
+
+#define GinItemPointerSetBlockNumber(pointer, blkno) \
+	(ItemPointerSetBlockNumber((pointer), (blkno)))
+
+#define GinItemPointerSetOffsetNumber(pointer, offnum) \
+	(ItemPointerSetOffsetNumber((pointer), (offnum)))
+
 
 /*
  * Special-case item pointer values needed by the GIN search logic.

src/include/access/htup_details.h

@@ -422,7 +422,7 @@ do { \
 
 #define HeapTupleHeaderIsSpeculative(tup) \
 ( \
-	(tup)->t_ctid.ip_posid == SpecTokenOffsetNumber \
+	(ItemPointerGetOffsetNumberNoCheck(&(tup)->t_ctid) == SpecTokenOffsetNumber) \
 )
 
 #define HeapTupleHeaderGetSpeculativeToken(tup) \

src/include/access/nbtree.h

@@ -151,9 +151,8 @@ typedef struct BTMetaPageData
  *	within a level). - vadim 04/09/97
  */
 #define BTTidSame(i1, i2)	\
-	( (i1).ip_blkid.bi_hi == (i2).ip_blkid.bi_hi && \
-	  (i1).ip_blkid.bi_lo == (i2).ip_blkid.bi_lo && \
-	  (i1).ip_posid == (i2).ip_posid )
+	((ItemPointerGetBlockNumber(&(i1)) == ItemPointerGetBlockNumber(&(i2))) && \
+	 (ItemPointerGetOffsetNumber(&(i1)) == ItemPointerGetOffsetNumber(&(i2))))
 #define BTEntrySame(i1, i2) \
 	BTTidSame((i1)->t_tid, (i2)->t_tid)
 

src/include/storage/itemptr.h

@@ -60,23 +60,41 @@ typedef ItemPointerData *ItemPointer;
 	((bool) (PointerIsValid(pointer) && ((pointer)->ip_posid != 0)))
 
 /*
- * ItemPointerGetBlockNumber
+ * ItemPointerGetBlockNumberNoCheck
  *		Returns the block number of a disk item pointer.
  */
+#define ItemPointerGetBlockNumberNoCheck(pointer) \
+( \
+	BlockIdGetBlockNumber(&(pointer)->ip_blkid) \
+)
+
+/*
+ * ItemPointerGetBlockNumber
+ *		As above, but verifies that the item pointer looks valid.
+ */
 #define ItemPointerGetBlockNumber(pointer) \
 ( \
 	AssertMacro(ItemPointerIsValid(pointer)), \
-	BlockIdGetBlockNumber(&(pointer)->ip_blkid) \
+	ItemPointerGetBlockNumberNoCheck(pointer) \
 )
 
 /*
- * ItemPointerGetOffsetNumber
+ * ItemPointerGetOffsetNumberNoCheck
  *		Returns the offset number of a disk item pointer.
  */
+#define ItemPointerGetOffsetNumberNoCheck(pointer) \
+( \
+	(pointer)->ip_posid \
+)
+
+/*
+ * ItemPointerGetOffsetNumber
+ *		As above, but verifies that the item pointer looks valid.
+ */
 #define ItemPointerGetOffsetNumber(pointer) \
 ( \
 	AssertMacro(ItemPointerIsValid(pointer)), \
-	(pointer)->ip_posid \
+	ItemPointerGetOffsetNumberNoCheck(pointer) \
 )
 
 /*