Skip to content

Commit ce96ce6

Browse files
alvherrealvherre
committed
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
1 parent a99f770 commit ce96ce6

File tree

12 files changed

+78
-57
lines changed

12 files changed

+78
-57
lines changed

‎contrib/pageinspect/btreefuncs.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -363,8 +363,8 @@ bt_page_items(PG_FUNCTION_ARGS)
363363
j = 0;
364364
values[j++] = psprintf("%d", uargs->offset);
365365
values[j++] = psprintf("(%u,%u)",
366-
BlockIdGetBlockNumber(&(itup->t_tid.ip_blkid)),
367-
itup->t_tid.ip_posid);
366+
ItemPointerGetBlockNumberNoCheck(&itup->t_tid),
367+
ItemPointerGetOffsetNumberNoCheck(&itup->t_tid));
368368
values[j++] = psprintf("%d", (int) IndexTupleSize(itup));
369369
values[j++] = psprintf("%c", IndexTupleHasNulls(itup) ? 't' : 'f');
370370
values[j++] = psprintf("%c", IndexTupleHasVarwidths(itup) ? 't' : 'f');

‎contrib/pgstattuple/pgstattuple.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ pgstat_heap(Relation rel, FunctionCallInfo fcinfo)
356356
* heap_getnext may find no tuples on a given page, so we cannot
357357
* simply examine the pages returned by the heap scan.
358358
*/
359-
tupblock = BlockIdGetBlockNumber(&tuple->t_self.ip_blkid);
359+
tupblock = ItemPointerGetBlockNumber(&tuple->t_self);
360360

361361
while (block <= tupblock)
362362
{

‎src/backend/access/gin/ginget.c

+17-12
Original file line numberDiff line numberDiff line change
@@ -626,8 +626,9 @@ entryLoadMoreItems(GinState *ginstate, GinScanEntry entry,
626626
}
627627
else
628628
{
629-
entry->btree.itemptr = advancePast;
630-
entry->btree.itemptr.ip_posid++;
629+
ItemPointerSet(&entry->btree.itemptr,
630+
GinItemPointerGetBlockNumber(&advancePast),
631+
OffsetNumberNext(GinItemPointerGetOffsetNumber(&advancePast)));
631632
}
632633
entry->btree.fullScan = false;
633634
stack = ginFindLeafPage(&entry->btree, true, snapshot);
@@ -979,15 +980,17 @@ keyGetItem(GinState *ginstate, MemoryContext tempCtx, GinScanKey key,
979980
if (GinItemPointerGetBlockNumber(&advancePast) <
980981
GinItemPointerGetBlockNumber(&minItem))
981982
{
982-
advancePast.ip_blkid = minItem.ip_blkid;
983-
advancePast.ip_posid = 0;
983+
ItemPointerSet(&advancePast,
984+
GinItemPointerGetBlockNumber(&minItem),
985+
InvalidOffsetNumber);
984986
}
985987
}
986988
else
987989
{
988-
Assert(minItem.ip_posid > 0);
989-
advancePast = minItem;
990-
advancePast.ip_posid--;
990+
Assert(GinItemPointerGetOffsetNumber(&minItem) > 0);
991+
ItemPointerSet(&advancePast,
992+
GinItemPointerGetBlockNumber(&minItem),
993+
OffsetNumberPrev(GinItemPointerGetOffsetNumber(&minItem)));
991994
}
992995

993996
/*
@@ -1245,15 +1248,17 @@ scanGetItem(IndexScanDesc scan, ItemPointerData advancePast,
12451248
if (GinItemPointerGetBlockNumber(&advancePast) <
12461249
GinItemPointerGetBlockNumber(&key->curItem))
12471250
{
1248-
advancePast.ip_blkid = key->curItem.ip_blkid;
1249-
advancePast.ip_posid = 0;
1251+
ItemPointerSet(&advancePast,
1252+
GinItemPointerGetBlockNumber(&key->curItem),
1253+
InvalidOffsetNumber);
12501254
}
12511255
}
12521256
else
12531257
{
1254-
Assert(key->curItem.ip_posid > 0);
1255-
advancePast = key->curItem;
1256-
advancePast.ip_posid--;
1258+
Assert(GinItemPointerGetOffsetNumber(&key->curItem) > 0);
1259+
ItemPointerSet(&advancePast,
1260+
GinItemPointerGetBlockNumber(&key->curItem),
1261+
OffsetNumberPrev(GinItemPointerGetOffsetNumber(&key->curItem)));
12571262
}
12581263

12591264
/*

‎src/backend/access/gin/ginpostinglist.c

+5-9
Original file line numberDiff line numberDiff line change
@@ -79,25 +79,21 @@ itemptr_to_uint64(const ItemPointer iptr)
7979
uint64 val;
8080

8181
Assert(ItemPointerIsValid(iptr));
82-
Assert(iptr->ip_posid < (1 << MaxHeapTuplesPerPageBits));
82+
Assert(GinItemPointerGetOffsetNumber(iptr) < (1 << MaxHeapTuplesPerPageBits));
8383

84-
val = iptr->ip_blkid.bi_hi;
85-
val <<= 16;
86-
val |= iptr->ip_blkid.bi_lo;
84+
val = GinItemPointerGetBlockNumber(iptr);
8785
val <<= MaxHeapTuplesPerPageBits;
88-
val |= iptr->ip_posid;
86+
val |= GinItemPointerGetOffsetNumber(iptr);
8987

9088
return val;
9189
}
9290

9391
static inline void
9492
uint64_to_itemptr(uint64 val, ItemPointer iptr)
9593
{
96-
iptr->ip_posid = val & ((1 << MaxHeapTuplesPerPageBits) - 1);
94+
GinItemPointerSetOffsetNumber(iptr, val & ((1 << MaxHeapTuplesPerPageBits) - 1));
9795
val = val >> MaxHeapTuplesPerPageBits;
98-
iptr->ip_blkid.bi_lo = val & 0xFFFF;
99-
val = val >> 16;
100-
iptr->ip_blkid.bi_hi = val & 0xFFFF;
96+
GinItemPointerSetBlockNumber(iptr, val);
10197

10298
Assert(ItemPointerIsValid(iptr));
10399
}

‎src/backend/replication/logical/reorderbuffer.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -3013,8 +3013,8 @@ DisplayMapping(HTAB *tuplecid_data)
30133013
ent->key.relnode.dbNode,
30143014
ent->key.relnode.spcNode,
30153015
ent->key.relnode.relNode,
3016-
BlockIdGetBlockNumber(&ent->key.tid.ip_blkid),
3017-
ent->key.tid.ip_posid,
3016+
ItemPointerGetBlockNumber(&ent->key.tid),
3017+
ItemPointerGetOffsetNumber(&ent->key.tid),
30183018
ent->cmin,
30193019
ent->cmax
30203020
);

‎src/backend/storage/page/itemptr.c

+8-7
Original file line numberDiff line numberDiff line change
@@ -52,20 +52,21 @@ int32
5252
ItemPointerCompare(ItemPointer arg1, ItemPointer arg2)
5353
{
5454
/*
55-
* Don't use ItemPointerGetBlockNumber or ItemPointerGetOffsetNumber here,
56-
* because they assert ip_posid != 0 which might not be true for a
57-
* user-supplied TID.
55+
* Use ItemPointerGet{Offset,Block}NumberNoCheck to avoid asserting
56+
* ip_posid != 0, which may not be true for a user-supplied TID.
5857
*/
59-
BlockNumber b1 = BlockIdGetBlockNumber(&(arg1->ip_blkid));
60-
BlockNumber b2 = BlockIdGetBlockNumber(&(arg2->ip_blkid));
58+
BlockNumber b1 = ItemPointerGetBlockNumberNoCheck(arg1);
59+
BlockNumber b2 = ItemPointerGetBlockNumberNoCheck(arg2);
6160

6261
if (b1 < b2)
6362
return -1;
6463
else if (b1 > b2)
6564
return 1;
66-
else if (arg1->ip_posid < arg2->ip_posid)
65+
else if (ItemPointerGetOffsetNumberNoCheck(arg1) <
66+
ItemPointerGetOffsetNumberNoCheck(arg2))
6767
return -1;
68-
else if (arg1->ip_posid > arg2->ip_posid)
68+
else if (ItemPointerGetOffsetNumberNoCheck(arg1) >
69+
ItemPointerGetOffsetNumberNoCheck(arg2))
6970
return 1;
7071
else
7172
return 0;

‎src/backend/utils/adt/tid.c

+6-11
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@ tidout(PG_FUNCTION_ARGS)
109109
OffsetNumber offsetNumber;
110110
char buf[32];
111111

112-
blockNumber = BlockIdGetBlockNumber(&(itemPtr->ip_blkid));
113-
offsetNumber = itemPtr->ip_posid;
112+
blockNumber = ItemPointerGetBlockNumberNoCheck(itemPtr);
113+
offsetNumber = ItemPointerGetOffsetNumberNoCheck(itemPtr);
114114

115115
/* Perhaps someday we should output this as a record. */
116116
snprintf(buf, sizeof(buf), "(%u,%u)", blockNumber, offsetNumber);
@@ -146,18 +146,13 @@ Datum
146146
tidsend(PG_FUNCTION_ARGS)
147147
{
148148
ItemPointer itemPtr = PG_GETARG_ITEMPOINTER(0);
149-
BlockId blockId;
150-
BlockNumber blockNumber;
151-
OffsetNumber offsetNumber;
152149
StringInfoData buf;
153150

154-
blockId = &(itemPtr->ip_blkid);
155-
blockNumber = BlockIdGetBlockNumber(blockId);
156-
offsetNumber = itemPtr->ip_posid;
157-
158151
pq_begintypsend(&buf);
159-
pq_sendint(&buf, blockNumber, sizeof(blockNumber));
160-
pq_sendint(&buf, offsetNumber, sizeof(offsetNumber));
152+
pq_sendint(&buf, ItemPointerGetBlockNumberNoCheck(itemPtr),
153+
sizeof(BlockNumber));
154+
pq_sendint(&buf, ItemPointerGetOffsetNumberNoCheck(itemPtr),
155+
sizeof(OffsetNumber));
161156
PG_RETURN_BYTEA_P(pq_endtypsend(&buf));
162157
}
163158

‎src/include/access/gin_private.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -460,8 +460,8 @@ extern ItemPointer ginMergeItemPointers(ItemPointerData *a, uint32 na,
460460
static inline int
461461
ginCompareItemPointers(ItemPointer a, ItemPointer b)
462462
{
463-
uint64 ia = (uint64) a->ip_blkid.bi_hi << 32 | (uint64) a->ip_blkid.bi_lo << 16 | a->ip_posid;
464-
uint64 ib = (uint64) b->ip_blkid.bi_hi << 32 | (uint64) b->ip_blkid.bi_lo << 16 | b->ip_posid;
463+
uint64 ia = (uint64) GinItemPointerGetBlockNumber(a) << 32 | GinItemPointerGetOffsetNumber(a);
464+
uint64 ib = (uint64) GinItemPointerGetBlockNumber(b) << 32 | GinItemPointerGetOffsetNumber(b);
465465

466466
if (ia == ib)
467467
return 0;
@@ -471,6 +471,6 @@ ginCompareItemPointers(ItemPointer a, ItemPointer b)
471471
return -1;
472472
}
473473

474-
extern int ginTraverseLock(Buffer buffer, bool searchMode);
474+
extern int ginTraverseLock(Buffer buffer, bool searchMode);
475475

476476
#endif /* GIN_PRIVATE_H */

‎src/include/access/ginblock.h

+9-2
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,17 @@ typedef struct GinMetaPageData
132132
* to avoid Asserts, since sometimes the ip_posid isn't "valid"
133133
*/
134134
#define GinItemPointerGetBlockNumber(pointer) \
135-
BlockIdGetBlockNumber(&(pointer)->ip_blkid)
135+
(ItemPointerGetBlockNumberNoCheck(pointer))
136136

137137
#define GinItemPointerGetOffsetNumber(pointer) \
138-
((pointer)->ip_posid)
138+
(ItemPointerGetOffsetNumberNoCheck(pointer))
139+
140+
#define GinItemPointerSetBlockNumber(pointer, blkno) \
141+
(ItemPointerSetBlockNumber((pointer), (blkno)))
142+
143+
#define GinItemPointerSetOffsetNumber(pointer, offnum) \
144+
(ItemPointerSetOffsetNumber((pointer), (offnum)))
145+
139146

140147
/*
141148
* Special-case item pointer values needed by the GIN search logic.

‎src/include/access/htup_details.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ do { \
422422

423423
#define HeapTupleHeaderIsSpeculative(tup) \
424424
( \
425-
(tup)->t_ctid.ip_posid == SpecTokenOffsetNumber \
425+
(ItemPointerGetOffsetNumberNoCheck(&(tup)->t_ctid) == SpecTokenOffsetNumber) \
426426
)
427427

428428
#define HeapTupleHeaderGetSpeculativeToken(tup) \

‎src/include/access/nbtree.h

+2-3
Original file line numberDiff line numberDiff line change
@@ -151,9 +151,8 @@ typedef struct BTMetaPageData
151151
* within a level). - vadim 04/09/97
152152
*/
153153
#define BTTidSame(i1, i2) \
154-
( (i1).ip_blkid.bi_hi == (i2).ip_blkid.bi_hi && \
155-
(i1).ip_blkid.bi_lo == (i2).ip_blkid.bi_lo && \
156-
(i1).ip_posid == (i2).ip_posid )
154+
((ItemPointerGetBlockNumber(&(i1)) == ItemPointerGetBlockNumber(&(i2))) && \
155+
(ItemPointerGetOffsetNumber(&(i1)) == ItemPointerGetOffsetNumber(&(i2))))
157156
#define BTEntrySame(i1, i2) \
158157
BTTidSame((i1)->t_tid, (i2)->t_tid)
159158

‎src/include/storage/itemptr.h

+22-4
Original file line numberDiff line numberDiff line change
@@ -60,23 +60,41 @@ typedef ItemPointerData *ItemPointer;
6060
((bool) (PointerIsValid(pointer) && ((pointer)->ip_posid != 0)))
6161

6262
/*
63-
* ItemPointerGetBlockNumber
63+
* ItemPointerGetBlockNumberNoCheck
6464
* Returns the block number of a disk item pointer.
6565
*/
66+
#define ItemPointerGetBlockNumberNoCheck(pointer) \
67+
( \
68+
BlockIdGetBlockNumber(&(pointer)->ip_blkid) \
69+
)
70+
71+
/*
72+
* ItemPointerGetBlockNumber
73+
* As above, but verifies that the item pointer looks valid.
74+
*/
6675
#define ItemPointerGetBlockNumber(pointer) \
6776
( \
6877
AssertMacro(ItemPointerIsValid(pointer)), \
69-
BlockIdGetBlockNumber(&(pointer)->ip_blkid) \
78+
ItemPointerGetBlockNumberNoCheck(pointer) \
7079
)
7180

7281
/*
73-
* ItemPointerGetOffsetNumber
82+
* ItemPointerGetOffsetNumberNoCheck
7483
* Returns the offset number of a disk item pointer.
7584
*/
85+
#define ItemPointerGetOffsetNumberNoCheck(pointer) \
86+
( \
87+
(pointer)->ip_posid \
88+
)
89+
90+
/*
91+
* ItemPointerGetOffsetNumber
92+
* As above, but verifies that the item pointer looks valid.
93+
*/
7694
#define ItemPointerGetOffsetNumber(pointer) \
7795
( \
7896
AssertMacro(ItemPointerIsValid(pointer)), \
79-
(pointer)->ip_posid \
97+
ItemPointerGetOffsetNumberNoCheck(pointer) \
8098
)
8199

82100
/*

0 commit comments

Comments
 (0)