BRIN page type identifier

Started by Heikki Linnakangasover 11 years ago6 messageshackers
Jump to latest
#1Heikki Linnakangas
heikki.linnakangas@enterprisedb.com

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Heikki Linnakangas (#1)
Re: BRIN page type identifier

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#2)
Re: BRIN page type identifier

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Tom Lane (#3)
Re: BRIN page type identifier

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

Attachments:

brin-pagetype.patchtext/x-diff; charset=us-asciiDownload+45-41
#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#3)
Re: BRIN page type identifier

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#5)
Re: BRIN page type identifier

[ 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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers