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
This commit is contained in:
Alvaro Herrera 2015-03-10 12:26:34 -03:00
parent 865f14a2d3
commit e491bd2ee3
6 changed files with 45 additions and 41 deletions

View File

@ -58,12 +58,9 @@ brin_page_type(PG_FUNCTION_ARGS)
{
bytea *raw_page = PG_GETARG_BYTEA_P(0);
Page page = VARDATA(raw_page);
BrinSpecialSpace *special;
char *type;
special = (BrinSpecialSpace *) PageGetSpecialPointer(page);
switch (special->type)
switch (BrinPageType(page))
{
case BRIN_PAGETYPE_META:
type = "meta";
@ -75,7 +72,7 @@ brin_page_type(PG_FUNCTION_ARGS)
type = "regular";
break;
default:
type = psprintf("unknown (%02x)", special->type);
type = psprintf("unknown (%02x)", BrinPageType(page));
break;
}
@ -91,7 +88,6 @@ verify_brin_page(bytea *raw_page, uint16 type, const char *strtype)
{
Page page;
int raw_page_size;
BrinSpecialSpace *special;
raw_page_size = VARSIZE(raw_page) - VARHDRSZ;
@ -104,13 +100,12 @@ verify_brin_page(bytea *raw_page, uint16 type, const char *strtype)
page = VARDATA(raw_page);
/* verify the special space says this page is what we want */
special = (BrinSpecialSpace *) PageGetSpecialPointer(page);
if (special->type != type)
if (BrinPageType(page) != type)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("page is not a BRIN page of type \"%s\"", strtype),
errdetail("Expected special type %08x, got %08x.",
type, special->type)));
type, BrinPageType(page))));
return page;
}

View File

@ -53,7 +53,6 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
BrinTuple *oldtup;
Size oldsz;
Buffer newbuf;
BrinSpecialSpace *special;
bool extended = false;
newsz = MAXALIGN(newsz);
@ -113,8 +112,6 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
return false;
}
special = (BrinSpecialSpace *) PageGetSpecialPointer(oldpage);
/*
* Great, the old tuple is intact. We can proceed with the update.
*
@ -124,7 +121,7 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
* caller told us there isn't, if a concurrent update moved another tuple
* elsewhere or replaced a tuple with a smaller one.
*/
if (((special->flags & BRIN_EVACUATE_PAGE) == 0) &&
if (((BrinPageFlags(oldpage) & BRIN_EVACUATE_PAGE) == 0) &&
brin_can_do_samepage_update(oldbuf, origsz, newsz))
{
if (BufferIsValid(newbuf))
@ -374,12 +371,9 @@ brin_doinsert(Relation idxrel, BlockNumber pagesPerRange,
void
brin_page_init(Page page, uint16 type)
{
BrinSpecialSpace *special;
PageInit(page, BLCKSZ, sizeof(BrinSpecialSpace));
special = (BrinSpecialSpace *) PageGetSpecialPointer(page);
special->type = type;
BrinPageType(page) = type;
}
/*
@ -420,7 +414,6 @@ brin_start_evacuating_page(Relation idxRel, Buffer buf)
{
OffsetNumber off;
OffsetNumber maxoff;
BrinSpecialSpace *special;
Page page;
page = BufferGetPage(buf);
@ -428,8 +421,6 @@ brin_start_evacuating_page(Relation idxRel, Buffer buf)
if (PageIsNew(page))
return false;
special = (BrinSpecialSpace *) PageGetSpecialPointer(page);
maxoff = PageGetMaxOffsetNumber(page);
for (off = FirstOffsetNumber; off <= maxoff; off++)
{
@ -439,7 +430,7 @@ brin_start_evacuating_page(Relation idxRel, Buffer buf)
if (ItemIdIsUsed(lp))
{
/* prevent other backends from adding more stuff to this page */
special->flags |= BRIN_EVACUATE_PAGE;
BrinPageFlags(page) |= BRIN_EVACUATE_PAGE;
MarkBufferDirtyHint(buf, true);
return true;
@ -463,8 +454,7 @@ brin_evacuate_page(Relation idxRel, BlockNumber pagesPerRange,
page = BufferGetPage(buf);
Assert(((BrinSpecialSpace *)
PageGetSpecialPointer(page))->flags & BRIN_EVACUATE_PAGE);
Assert(BrinPageFlags(page) & BRIN_EVACUATE_PAGE);
maxoff = PageGetMaxOffsetNumber(page);
for (off = FirstOffsetNumber; off <= maxoff; off++)
@ -677,11 +667,8 @@ brin_getinsertbuffer(Relation irel, Buffer oldbuf, Size itemsz,
static Size
br_page_get_freespace(Page page)
{
BrinSpecialSpace *special;
special = (BrinSpecialSpace *) PageGetSpecialPointer(page);
if (!BRIN_IS_REGULAR_PAGE(page) ||
(special->flags & BRIN_EVACUATE_PAGE) != 0)
(BrinPageFlags(page) & BRIN_EVACUATE_PAGE) != 0)
return 0;
else
return PageGetFreeSpace(page);

View File

@ -446,7 +446,7 @@ revmap_physical_extend(BrinRevmap *revmap)
ereport(ERROR,
(errcode(ERRCODE_INDEX_CORRUPTED),
errmsg("unexpected page type 0x%04X in BRIN index \"%s\" block %u",
BRIN_PAGE_TYPE(page),
BrinPageType(page),
RelationGetRelationName(irel),
BufferGetBlockNumber(buf))));

View File

@ -20,24 +20,44 @@
#include "storage/block.h"
#include "storage/itemptr.h"
/*
* Special area of BRIN pages.
*
* We define it in this odd way so that it always occupies the last
* MAXALIGN-sized element of each page.
*/
typedef struct BrinSpecialSpace
{
uint16 vector[MAXALIGN(1) / sizeof(uint16)];
} BrinSpecialSpace;
/*
* Make the page type be the last half-word in the page, for consumption by
* pg_filedump and similar utilities. We don't really care much about the
* position of the "flags" half-word, but it's simpler to apply a consistent
* rule to both.
*
* See comments above GinPageOpaqueData.
*/
#define BrinPageType(page) \
(((BrinSpecialSpace *) \
PageGetSpecialPointer(page))->vector[MAXALIGN(1) / sizeof(uint16) - 1])
#define BrinPageFlags(page) \
(((BrinSpecialSpace *) \
PageGetSpecialPointer(page))->vector[MAXALIGN(1) / sizeof(uint16) - 2])
/* special space on all BRIN pages stores a "type" identifier */
#define BRIN_PAGETYPE_META 0xF091
#define BRIN_PAGETYPE_REVMAP 0xF092
#define BRIN_PAGETYPE_REGULAR 0xF093
#define BRIN_PAGE_TYPE(page) \
(((BrinSpecialSpace *) PageGetSpecialPointer(page))->type)
#define BRIN_IS_REVMAP_PAGE(page) (BRIN_PAGE_TYPE(page) == BRIN_PAGETYPE_REVMAP)
#define BRIN_IS_REGULAR_PAGE(page) (BRIN_PAGE_TYPE(page) == BRIN_PAGETYPE_REGULAR)
#define BRIN_IS_REVMAP_PAGE(page) (BrinPageType(page) == BRIN_PAGETYPE_REVMAP)
#define BRIN_IS_REGULAR_PAGE(page) (BrinPageType(page) == BRIN_PAGETYPE_REGULAR)
/* flags for BrinSpecialSpace */
#define BRIN_EVACUATE_PAGE (1 << 0)
typedef struct BrinSpecialSpace
{
uint16 flags;
uint16 type;
} BrinSpecialSpace;
/* Metapage definitions */
typedef struct BrinMetaPageData

View File

@ -24,10 +24,10 @@
* Note: GIN does not include a page ID word as do the other index types.
* This is OK because the opaque data is only 8 bytes and so can be reliably
* distinguished by size. Revisit this if the size ever increases.
* Further note: as of 9.2, SP-GiST also uses 8-byte special space. This is
* still OK, as long as GIN isn't using all of the high-order bits in its
* flags word, because that way the flags word cannot match the page ID used
* by SP-GiST.
* Further note: as of 9.2, SP-GiST also uses 8-byte special space, as does
* BRIN as of 9.5. This is still OK, as long as GIN isn't using all of the
* high-order bits in its flags word, because that way the flags word cannot
* match the page IDs used by SP-GiST and BRIN.
*/
typedef struct GinPageOpaqueData
{

View File

@ -64,6 +64,8 @@ typedef SpGistPageOpaqueData *SpGistPageOpaque;
* which otherwise would have a hard time telling pages of different index
* types apart. It should be the last 2 bytes on the page. This is more or
* less "free" due to alignment considerations.
*
* See comments above GinPageOpaqueData.
*/
#define SPGIST_PAGE_ID 0xFF82