Remove direct uses of ItemPointer.{ip_blkid,ip_posid}
authorAlvaro Herrera <[email protected]>
Tue, 28 Mar 2017 15:52:55 +0000 (12:52 -0300)
committerAlvaro Herrera <[email protected]>
Tue, 28 Mar 2017 22:02:23 +0000 (19:02 -0300)
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

12 files changed:
contrib/pageinspect/btreefuncs.c
contrib/pgstattuple/pgstattuple.c
src/backend/access/gin/ginget.c
src/backend/access/gin/ginpostinglist.c
src/backend/replication/logical/reorderbuffer.c
src/backend/storage/page/itemptr.c
src/backend/utils/adt/tid.c
src/include/access/gin_private.h
src/include/access/ginblock.h
src/include/access/htup_details.h
src/include/access/nbtree.h
src/include/storage/itemptr.h

index 6f35e288fd16b22560badf1d17f0df35333a1f87..3d69aa9d37baf9ae55e4a63d18bfa45834d081b4 100644 (file)
@@ -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');
index 1e0de5d660ad703afb04a1592ace4165fbdd2fc6..44f90cd0d371c66403ff5d326e27b6f707ca61d2 100644 (file)
@@ -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)
        {
index 87cd9eaa9f55c9170a3201542d5218396637de1e..610d386ff8a99c022aacff6650386237f5e23187 100644 (file)
@@ -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)));
            }
 
            /*
index 598069d06fffe7e974f7d71690ae8183bccae7bc..8d2d31ac7236de5e2c62c6fa745af45a2b895b2c 100644 (file)
@@ -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));
 }
index b437799c5fde245b35d1349c1086b9cdcec9e328..12ebadca815097d9d138b82146b777d6374390f6 100644 (file)
@@ -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
            );
index 703cbb9c39cc98062b21827a1fa1a39b98923318..b872928f2f63b9378ee21c12aa268271863285ad 100644 (file)
@@ -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;
index 49a5a157b94e24b5781c0f9bb6cb09c56c03891a..8453b65e78aaed5002e261ed1f66e5adf046d53e 100644 (file)
@@ -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));
 }
 
index 824cc1c9d89055f4fd9a82e4620c6223f97ab2b4..f2e9c4ddec833231c530f180484a7f4bc0fc4ce6 100644 (file)
@@ -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 */
index a3fb0560ddfa8eb7eb8cea8b3ee12cf33211beef..438912c6a0d6238d2e7e4b14bb93e0010907ea06 100644 (file)
@@ -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.
index a6c7e319a6e2274fddf8ca5b0a05e887f1e1d976..7b6285df136914ad3d618d644b2f71ab0f84341f 100644 (file)
@@ -422,7 +422,7 @@ do { \
 
 #define HeapTupleHeaderIsSpeculative(tup) \
 ( \
-   (tup)->t_ctid.ip_posid == SpecTokenOffsetNumber \
+   (ItemPointerGetOffsetNumberNoCheck(&(tup)->t_ctid) == SpecTokenOffsetNumber) \
 )
 
 #define HeapTupleHeaderGetSpeculativeToken(tup) \
index 6289ffa9bd44fb75a5aa7313dfe8817893c9851a..f9304dbe1d595797535d64a43c835acfcf87bcbb 100644 (file)
@@ -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)
 
index 576aaa890b81346862c9dca54456816d8bf07d47..c21d2adfcbf4726a68af77528a5b9c9aeb2fa982 100644 (file)
@@ -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) \
 )
 
 /*