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

5 files changed, 38 insertions, 41 deletions

diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c
index 87cd9eaa9f5..610d386ff8a 100644
--- a/src/backend/access/gin/ginget.c
+++ b/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)));
 			}
 
 			/*
diff --git a/src/backend/access/gin/ginpostinglist.c b/src/backend/access/gin/ginpostinglist.c
index 598069d06ff..8d2d31ac723 100644
--- a/src/backend/access/gin/ginpostinglist.c
+++ b/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));
 }
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index b437799c5fd..12ebadca815 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/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
 			);
diff --git a/src/backend/storage/page/itemptr.c b/src/backend/storage/page/itemptr.c
index 703cbb9c39c..b872928f2f6 100644
--- a/src/backend/storage/page/itemptr.c
+++ b/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;
diff --git a/src/backend/utils/adt/tid.c b/src/backend/utils/adt/tid.c
index 49a5a157b94..8453b65e78a 100644
--- a/src/backend/utils/adt/tid.c
+++ b/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));
 }