Thread: BRIN page type identifier
The "special" area in a BRIN page looks like this:
> /* 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
>...
> typedef struct BrinSpecialSpace
> {
> uint16 flags;
> uint16 type;
> } BrinSpecialSpace;
I believe this is supposed to follow the usual convention that the last
two bytes of a page can be used to identify the page type. SP-GiST uses
0xFF82, while GIN uses values 0x00XX.
However, because the special size is MAXALIGNed, the 'type' field are
not the last 2 bytes on the page, as intended. I'd suggest just adding
"char padding[6]" in BrinSpecialSpace, before 'flags'. That'll waste 4
bytes on 32-bit systems, but that seems acceptable.
Also, the comment in GinPageOpaqueData needs updating:
> /*
> * Page opaque data in an inverted index page.
> *
> * 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.
> */
BRIN now also uses 8-byte special space. While you're at it, might want
to move that comment somewhere else, as it's really about a convention
among all page types, not just GIN.
- Heikki
Heikki Linnakangas wrote:
> The "special" area in a BRIN page looks like this:
>
> >/* 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
> >...
> >typedef struct BrinSpecialSpace
> >{
> > uint16 flags;
> > uint16 type;
> >} BrinSpecialSpace;
>
> I believe this is supposed to follow the usual convention that the last two
> bytes of a page can be used to identify the page type. SP-GiST uses 0xFF82,
> while GIN uses values 0x00XX.
>
> However, because the special size is MAXALIGNed, the 'type' field are not
> the last 2 bytes on the page, as intended. I'd suggest just adding "char
> padding[6]" in BrinSpecialSpace, before 'flags'. That'll waste 4 bytes on
> 32-bit systems, but that seems acceptable.
Ouch. You're right. I don't understand why you suggest to use 6 bytes,
though -- that would make the struct size be 10 bytes, which maxaligns
to 16, and so we're back where we started. Using 4 bytes does the
trick.
I wonder if this is permissible and whether it will do the right thing
on 32-bit systems:
/** Special area of BRIN pages.** We add some padding bytes to ensure that 'type' ends up in the last two* bytes of the
page,for consumption by pg_filedump and similar utilities.* (Special space is MAXALIGN'ed).*/
typedef struct BrinSpecialSpace
{char padding[MAXALIGN(1) - 2 * sizeof(uint16)];uint16 flags;uint16 type;
} BrinSpecialSpace;
It's a bit ugly, but it seems to work for me on x86-64 ...
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> I wonder if this is permissible and whether it will do the right thing
> on 32-bit systems:
> /*
> * Special area of BRIN pages.
> *
> * We add some padding bytes to ensure that 'type' ends up in the last two
> * bytes of the page, for consumption by pg_filedump and similar utilities.
> * (Special space is MAXALIGN'ed).
> */
> typedef struct BrinSpecialSpace
> {
> char padding[MAXALIGN(1) - 2 * sizeof(uint16)];
> uint16 flags;
> uint16 type;
> } BrinSpecialSpace;
I should expect that to fail altogether on 32-bit machines, because
the declared array size will be zero.
You could try something like
typedef struct BrinSpecialSpace
{uint16 vector[MAXALIGN(1) / sizeof(uint16)];
} BrinSpecialSpace;
and then some access macros to use the last and next-to-last
elements of that array.
regards, tom lane
Tom Lane wrote:
> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > typedef struct BrinSpecialSpace
> > {
> > char padding[MAXALIGN(1) - 2 * sizeof(uint16)];
> > uint16 flags;
> > uint16 type;
> > } BrinSpecialSpace;
>
> I should expect that to fail altogether on 32-bit machines, because
> the declared array size will be zero.
Hah, of course.
> You could try something like
>
> typedef struct BrinSpecialSpace
> {
> uint16 vector[MAXALIGN(1) / sizeof(uint16)];
> } BrinSpecialSpace;
>
> and then some access macros to use the last and next-to-last
> elements of that array.
Ah, thanks, that works fine on x86-64. Here's a patch I intend to push
tomorrow.
Heikki suggested that the comment above GinPageOpaqueData be moved to
some better place, but I couldn't find any such. If there are other
ideas, I'm all ears.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
I wrote:
> You could try something like
> typedef struct BrinSpecialSpace
> {
> uint16 vector[MAXALIGN(1) / sizeof(uint16)];
> } BrinSpecialSpace;
> and then some access macros to use the last and next-to-last
> elements of that array.
On second thought, consider
typedef struct BrinSpecialSpace
{uint16 flags;uint16 vector[MAXALIGN(1) / sizeof(uint16) - 1];
} BrinSpecialSpace;
This way, accesses to "flags" require no source code changes.
You still need a macro to map "type" onto the last element of
the vector, but there's probably about one reference to "type"
in the source code so it shouldn't be too painful.
regards, tom lane
[ paths crossed in mail ]
I wrote:
> This way, accesses to "flags" require no source code changes.
> You still need a macro to map "type" onto the last element of
> the vector, but there's probably about one reference to "type"
> in the source code so it shouldn't be too painful.
Ah, nevermind, I see you already did the work to do it the other
way.
regards, tom lane