Skip to content

Commit e491bd2

Browse files
alvherrealvherre
committed
Move BRIN page type to page's last two bytes
... which is the usual convention among AMs, so that pg_filedump and similar utilities can tell apart pages of different AMs. It was also the intent of the original code, but I failed to realize that alignment considerations would move the whole thing to the previous-to-last word in the page. The new definition of the associated macro makes surrounding code a bit leaner, too. Per note from Heikki at http://www.postgresql.org/message-id/546A16EF.9070005@vmware.com
1 parent 865f14a commit e491bd2

File tree

6 files changed

+45
-41
lines changed

6 files changed

+45
-41
lines changed

‎contrib/pageinspect/brinfuncs.c

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,9 @@ brin_page_type(PG_FUNCTION_ARGS)
5858
{
5959
bytea *raw_page = PG_GETARG_BYTEA_P(0);
6060
Page page = VARDATA(raw_page);
61-
BrinSpecialSpace *special;
6261
char *type;
6362

64-
special = (BrinSpecialSpace *) PageGetSpecialPointer(page);
65-
66-
switch (special->type)
63+
switch (BrinPageType(page))
6764
{
6865
case BRIN_PAGETYPE_META:
6966
type = "meta";
@@ -75,7 +72,7 @@ brin_page_type(PG_FUNCTION_ARGS)
7572
type = "regular";
7673
break;
7774
default:
78-
type = psprintf("unknown (%02x)", special->type);
75+
type = psprintf("unknown (%02x)", BrinPageType(page));
7976
break;
8077
}
8178

@@ -91,7 +88,6 @@ verify_brin_page(bytea *raw_page, uint16 type, const char *strtype)
9188
{
9289
Page page;
9390
int raw_page_size;
94-
BrinSpecialSpace *special;
9591

9692
raw_page_size = VARSIZE(raw_page) - VARHDRSZ;
9793

@@ -104,13 +100,12 @@ verify_brin_page(bytea *raw_page, uint16 type, const char *strtype)
104100
page = VARDATA(raw_page);
105101

106102
/* verify the special space says this page is what we want */
107-
special = (BrinSpecialSpace *) PageGetSpecialPointer(page);
108-
if (special->type != type)
103+
if (BrinPageType(page) != type)
109104
ereport(ERROR,
110105
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
111106
errmsg("page is not a BRIN page of type \"%s\"", strtype),
112107
errdetail("Expected special type %08x, got %08x.",
113-
type, special->type)));
108+
type, BrinPageType(page))));
114109

115110
return page;
116111
}

‎src/backend/access/brin/brin_pageops.c

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
5353
BrinTuple *oldtup;
5454
Size oldsz;
5555
Buffer newbuf;
56-
BrinSpecialSpace *special;
5756
bool extended = false;
5857

5958
newsz = MAXALIGN(newsz);
@@ -113,8 +112,6 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
113112
return false;
114113
}
115114

116-
special = (BrinSpecialSpace *) PageGetSpecialPointer(oldpage);
117-
118115
/*
119116
* Great, the old tuple is intact. We can proceed with the update.
120117
*
@@ -124,7 +121,7 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
124121
* caller told us there isn't, if a concurrent update moved another tuple
125122
* elsewhere or replaced a tuple with a smaller one.
126123
*/
127-
if (((special->flags & BRIN_EVACUATE_PAGE) == 0) &&
124+
if (((BrinPageFlags(oldpage) & BRIN_EVACUATE_PAGE) == 0) &&
128125
brin_can_do_samepage_update(oldbuf, origsz, newsz))
129126
{
130127
if (BufferIsValid(newbuf))
@@ -374,12 +371,9 @@ brin_doinsert(Relation idxrel, BlockNumber pagesPerRange,
374371
void
375372
brin_page_init(Page page, uint16 type)
376373
{
377-
BrinSpecialSpace *special;
378-
379374
PageInit(page, BLCKSZ, sizeof(BrinSpecialSpace));
380375

381-
special = (BrinSpecialSpace *) PageGetSpecialPointer(page);
382-
special->type = type;
376+
BrinPageType(page) = type;
383377
}
384378

385379
/*
@@ -420,16 +414,13 @@ brin_start_evacuating_page(Relation idxRel, Buffer buf)
420414
{
421415
OffsetNumber off;
422416
OffsetNumber maxoff;
423-
BrinSpecialSpace *special;
424417
Page page;
425418

426419
page = BufferGetPage(buf);
427420

428421
if (PageIsNew(page))
429422
return false;
430423

431-
special = (BrinSpecialSpace *) PageGetSpecialPointer(page);
432-
433424
maxoff = PageGetMaxOffsetNumber(page);
434425
for (off = FirstOffsetNumber; off <= maxoff; off++)
435426
{
@@ -439,7 +430,7 @@ brin_start_evacuating_page(Relation idxRel, Buffer buf)
439430
if (ItemIdIsUsed(lp))
440431
{
441432
/* prevent other backends from adding more stuff to this page */
442-
special->flags |= BRIN_EVACUATE_PAGE;
433+
BrinPageFlags(page) |= BRIN_EVACUATE_PAGE;
443434
MarkBufferDirtyHint(buf, true);
444435

445436
return true;
@@ -463,8 +454,7 @@ brin_evacuate_page(Relation idxRel, BlockNumber pagesPerRange,
463454

464455
page = BufferGetPage(buf);
465456

466-
Assert(((BrinSpecialSpace *)
467-
PageGetSpecialPointer(page))->flags & BRIN_EVACUATE_PAGE);
457+
Assert(BrinPageFlags(page) & BRIN_EVACUATE_PAGE);
468458

469459
maxoff = PageGetMaxOffsetNumber(page);
470460
for (off = FirstOffsetNumber; off <= maxoff; off++)
@@ -677,11 +667,8 @@ brin_getinsertbuffer(Relation irel, Buffer oldbuf, Size itemsz,
677667
static Size
678668
br_page_get_freespace(Page page)
679669
{
680-
BrinSpecialSpace *special;
681-
682-
special = (BrinSpecialSpace *) PageGetSpecialPointer(page);
683670
if (!BRIN_IS_REGULAR_PAGE(page) ||
684-
(special->flags & BRIN_EVACUATE_PAGE) != 0)
671+
(BrinPageFlags(page) & BRIN_EVACUATE_PAGE) != 0)
685672
return 0;
686673
else
687674
return PageGetFreeSpace(page);

‎src/backend/access/brin/brin_revmap.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,7 @@ revmap_physical_extend(BrinRevmap *revmap)
446446
ereport(ERROR,
447447
(errcode(ERRCODE_INDEX_CORRUPTED),
448448
errmsg("unexpected page type 0x%04X in BRIN index \"%s\" block %u",
449-
BRIN_PAGE_TYPE(page),
449+
BrinPageType(page),
450450
RelationGetRelationName(irel),
451451
BufferGetBlockNumber(buf))));
452452

‎src/include/access/brin_page.h

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,24 +20,44 @@
2020
#include "storage/block.h"
2121
#include "storage/itemptr.h"
2222

23+
/*
24+
* Special area of BRIN pages.
25+
*
26+
* We define it in this odd way so that it always occupies the last
27+
* MAXALIGN-sized element of each page.
28+
*/
29+
typedef struct BrinSpecialSpace
30+
{
31+
uint16 vector[MAXALIGN(1) / sizeof(uint16)];
32+
} BrinSpecialSpace;
33+
34+
/*
35+
* Make the page type be the last half-word in the page, for consumption by
36+
* pg_filedump and similar utilities. We don't really care much about the
37+
* position of the "flags" half-word, but it's simpler to apply a consistent
38+
* rule to both.
39+
*
40+
* See comments above GinPageOpaqueData.
41+
*/
42+
#define BrinPageType(page) \
43+
(((BrinSpecialSpace *) \
44+
PageGetSpecialPointer(page))->vector[MAXALIGN(1) / sizeof(uint16) - 1])
45+
46+
#define BrinPageFlags(page) \
47+
(((BrinSpecialSpace *) \
48+
PageGetSpecialPointer(page))->vector[MAXALIGN(1) / sizeof(uint16) - 2])
49+
2350
/* special space on all BRIN pages stores a "type" identifier */
2451
#define BRIN_PAGETYPE_META 0xF091
2552
#define BRIN_PAGETYPE_REVMAP 0xF092
2653
#define BRIN_PAGETYPE_REGULAR 0xF093
2754

28-
#define BRIN_PAGE_TYPE(page) \
29-
(((BrinSpecialSpace *) PageGetSpecialPointer(page))->type)
30-
#define BRIN_IS_REVMAP_PAGE(page) (BRIN_PAGE_TYPE(page) == BRIN_PAGETYPE_REVMAP)
31-
#define BRIN_IS_REGULAR_PAGE(page) (BRIN_PAGE_TYPE(page) == BRIN_PAGETYPE_REGULAR)
55+
#define BRIN_IS_REVMAP_PAGE(page) (BrinPageType(page) == BRIN_PAGETYPE_REVMAP)
56+
#define BRIN_IS_REGULAR_PAGE(page) (BrinPageType(page) == BRIN_PAGETYPE_REGULAR)
3257

3358
/* flags for BrinSpecialSpace */
3459
#define BRIN_EVACUATE_PAGE (1 << 0)
3560

36-
typedef struct BrinSpecialSpace
37-
{
38-
uint16 flags;
39-
uint16 type;
40-
} BrinSpecialSpace;
4161

4262
/* Metapage definitions */
4363
typedef struct BrinMetaPageData

‎src/include/access/gin_private.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@
2424
* Note: GIN does not include a page ID word as do the other index types.
2525
* This is OK because the opaque data is only 8 bytes and so can be reliably
2626
* distinguished by size. Revisit this if the size ever increases.
27-
* Further note: as of 9.2, SP-GiST also uses 8-byte special space. This is
28-
* still OK, as long as GIN isn't using all of the high-order bits in its
29-
* flags word, because that way the flags word cannot match the page ID used
30-
* by SP-GiST.
27+
* Further note: as of 9.2, SP-GiST also uses 8-byte special space, as does
28+
* BRIN as of 9.5. This is still OK, as long as GIN isn't using all of the
29+
* high-order bits in its flags word, because that way the flags word cannot
30+
* match the page IDs used by SP-GiST and BRIN.
3131
*/
3232
typedef struct GinPageOpaqueData
3333
{

‎src/include/access/spgist_private.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ typedef SpGistPageOpaqueData *SpGistPageOpaque;
6464
* which otherwise would have a hard time telling pages of different index
6565
* types apart. It should be the last 2 bytes on the page. This is more or
6666
* less "free" due to alignment considerations.
67+
*
68+
* See comments above GinPageOpaqueData.
6769
*/
6870
#define SPGIST_PAGE_ID 0xFF82
6971

0 commit comments

Comments
 (0)