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

diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c
index 6f35e288fd16b22560badf1d17f0df35333a1f87..3d69aa9d37baf9ae55e4a63d18bfa45834d081b4 100644 (file)
--- a/contrib/pageinspect/btreefuncs.c
+++ b/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');

diff --git a/contrib/pgstattuple/pgstattuple.c b/contrib/pgstattuple/pgstattuple.c
index 1e0de5d660ad703afb04a1592ace4165fbdd2fc6..44f90cd0d371c66403ff5d326e27b6f707ca61d2 100644 (file)
--- a/contrib/pgstattuple/pgstattuple.c
+++ b/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)
                {

diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c
index 87cd9eaa9f55c9170a3201542d5218396637de1e..610d386ff8a99c022aacff6650386237f5e23187 100644 (file)
--- 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 598069d06fffe7e974f7d71690ae8183bccae7bc..8d2d31ac7236de5e2c62c6fa745af45a2b895b2c 100644 (file)
--- 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 b437799c5fde245b35d1349c1086b9cdcec9e328..12ebadca815097d9d138b82146b777d6374390f6 100644 (file)
--- 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 703cbb9c39cc98062b21827a1fa1a39b98923318..b872928f2f63b9378ee21c12aa268271863285ad 100644 (file)
--- 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 49a5a157b94e24b5781c0f9bb6cb09c56c03891a..8453b65e78aaed5002e261ed1f66e5adf046d53e 100644 (file)
--- 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));
 }
 

diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h
index 824cc1c9d89055f4fd9a82e4620c6223f97ab2b4..f2e9c4ddec833231c530f180484a7f4bc0fc4ce6 100644 (file)
--- a/src/include/access/gin_private.h
+++ b/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 */

diff --git a/src/include/access/ginblock.h b/src/include/access/ginblock.h
index a3fb0560ddfa8eb7eb8cea8b3ee12cf33211beef..438912c6a0d6238d2e7e4b14bb93e0010907ea06 100644 (file)
--- a/src/include/access/ginblock.h
+++ b/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.

diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h
index a6c7e319a6e2274fddf8ca5b0a05e887f1e1d976..7b6285df136914ad3d618d644b2f71ab0f84341f 100644 (file)
--- a/src/include/access/htup_details.h
+++ b/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) \

diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index 6289ffa9bd44fb75a5aa7313dfe8817893c9851a..f9304dbe1d595797535d64a43c835acfcf87bcbb 100644 (file)
--- a/src/include/access/nbtree.h
+++ b/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)
 

diff --git a/src/include/storage/itemptr.h b/src/include/storage/itemptr.h
index 576aaa890b81346862c9dca54456816d8bf07d47..c21d2adfcbf4726a68af77528a5b9c9aeb2fa982 100644 (file)
--- a/src/include/storage/itemptr.h
+++ b/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) \
 )
 
 /*