From 9ee4d06f3fdde37b063b8a0f0fa0a2113ac12303 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Thu, 17 Jan 2013 16:35:46 +0200 Subject: [PATCH] Make GiST indexes on-disk compatible with 9.2 again. The patch that turned XLogRecPtr into a uint64 inadvertently changed the on-disk format of GiST indexes, because the NSN field in the GiST page opaque is an XLogRecPtr. That breaks pg_upgrade. Revert the format of that field back to the two-field struct that XLogRecPtr was before. This is the same we did to LSNs in the page header to avoid changing on-disk format. Bump catversion, as this invalidates any existing GiST indexes built on 9.3devel. --- src/backend/access/gist/gist.c | 13 ++++++------ src/backend/access/gist/gistget.c | 2 +- src/backend/access/gist/gistvacuum.c | 2 +- src/backend/access/gist/gistxlog.c | 6 +++--- src/include/access/gist.h | 10 +++++++++- src/include/catalog/catversion.h | 2 +- src/include/storage/bufpage.h | 30 ++++++++++++++++++---------- 7 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c index 95d33c8945..4686242802 100644 --- a/src/backend/access/gist/gist.c +++ b/src/backend/access/gist/gist.c @@ -240,7 +240,7 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, { /* save old rightlink and NSN */ oldrlink = GistPageGetOpaque(page)->rightlink; - oldnsn = GistPageGetOpaque(page)->nsn; + oldnsn = GistPageGetNSN(page); dist->buffer = buffer; dist->block.blkno = BufferGetBlockNumber(buffer); @@ -364,7 +364,7 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, * F_FOLLOW_RIGHT flags ensure that scans will follow the * rightlinks until the downlinks are inserted. */ - GistPageGetOpaque(ptr->page)->nsn = oldnsn; + GistPageSetNSN(ptr->page, oldnsn); } START_CRIT_SECTION(); @@ -473,7 +473,7 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, { Page leftpg = BufferGetPage(leftchildbuf); - GistPageGetOpaque(leftpg)->nsn = recptr; + GistPageSetNSN(leftpg, recptr); GistClearFollowRight(leftpg); PageSetLSN(leftpg, recptr); @@ -561,7 +561,7 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace, GISTSTATE *giststate) } if (stack->blkno != GIST_ROOT_BLKNO && - stack->parent->lsn < GistPageGetOpaque(stack->page)->nsn) + stack->parent->lsn < GistPageGetNSN(stack->page)) { /* * Concurrent split detected. There's no guarantee that the @@ -707,8 +707,7 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace, GISTSTATE *giststate) */ } else if (GistFollowRight(stack->page) || - stack->parent->lsn < - GistPageGetOpaque(stack->page)->nsn) + stack->parent->lsn < GistPageGetNSN(stack->page)) { /* * The page was split while we momentarily unlocked the @@ -793,7 +792,7 @@ gistFindPath(Relation r, BlockNumber child, OffsetNumber *downlinkoffnum) if (GistFollowRight(page)) elog(ERROR, "concurrent GiST page split was incomplete"); - if (top->parent && top->parent->lsn < GistPageGetOpaque(page)->nsn && + if (top->parent && top->parent->lsn < GistPageGetNSN(page) && GistPageGetOpaque(page)->rightlink != InvalidBlockNumber /* sanity check */ ) { /* diff --git a/src/backend/access/gist/gistget.c b/src/backend/access/gist/gistget.c index 79996a235f..3300fec644 100644 --- a/src/backend/access/gist/gistget.c +++ b/src/backend/access/gist/gistget.c @@ -263,7 +263,7 @@ gistScanPage(IndexScanDesc scan, GISTSearchItem *pageItem, double *myDistances, */ if (!XLogRecPtrIsInvalid(pageItem->data.parentlsn) && (GistFollowRight(page) || - pageItem->data.parentlsn < opaque->nsn) && + pageItem->data.parentlsn < GistPageGetNSN(page)) && opaque->rightlink != InvalidBlockNumber /* sanity check */ ) { /* There was a page split, follow right link to add pages */ diff --git a/src/backend/access/gist/gistvacuum.c b/src/backend/access/gist/gistvacuum.c index dcad36ea3e..b5be6765d4 100644 --- a/src/backend/access/gist/gistvacuum.c +++ b/src/backend/access/gist/gistvacuum.c @@ -114,7 +114,7 @@ pushStackIfSplited(Page page, GistBDItem *stack) GISTPageOpaque opaque = GistPageGetOpaque(page); if (stack->blkno != GIST_ROOT_BLKNO && !XLogRecPtrIsInvalid(stack->parentlsn) && - (GistFollowRight(page) || stack->parentlsn < opaque->nsn) && + (GistFollowRight(page) || stack->parentlsn < GistPageGetNSN(page)) && opaque->rightlink != InvalidBlockNumber /* sanity check */ ) { /* split page detected, install right link to the stack */ diff --git a/src/backend/access/gist/gistxlog.c b/src/backend/access/gist/gistxlog.c index e3a213e7da..c18065a780 100644 --- a/src/backend/access/gist/gistxlog.c +++ b/src/backend/access/gist/gistxlog.c @@ -66,7 +66,7 @@ gistRedoClearFollowRight(XLogRecPtr lsn, XLogRecord *record, int block_index, */ if (lsn >= PageGetLSN(page)) { - GistPageGetOpaque(page)->nsn = lsn; + GistPageSetNSN(page, lsn); GistClearFollowRight(page); PageSetLSN(page, lsn); @@ -271,7 +271,7 @@ gistRedoPageSplitRecord(XLogRecPtr lsn, XLogRecord *record) if (newpage->header->blkno == GIST_ROOT_BLKNO) { GistPageGetOpaque(page)->rightlink = InvalidBlockNumber; - GistPageGetOpaque(page)->nsn = xldata->orignsn; + GistPageSetNSN(page, xldata->orignsn); GistClearFollowRight(page); } else @@ -280,7 +280,7 @@ gistRedoPageSplitRecord(XLogRecPtr lsn, XLogRecord *record) GistPageGetOpaque(page)->rightlink = xlrec.page[i + 1].header->blkno; else GistPageGetOpaque(page)->rightlink = xldata->origrlink; - GistPageGetOpaque(page)->nsn = xldata->orignsn; + GistPageSetNSN(page, xldata->orignsn); if (i < xlrec.data->npage - 1 && !isrootsplit && xldata->markfollowright) GistMarkFollowRight(page); diff --git a/src/include/access/gist.h b/src/include/access/gist.h index 69e4ec03fb..a487a0be3a 100644 --- a/src/include/access/gist.h +++ b/src/include/access/gist.h @@ -64,10 +64,15 @@ #define F_FOLLOW_RIGHT (1 << 3) /* page to the right has no downlink */ typedef XLogRecPtr GistNSN; +/* + * For on-disk compatibility with pre-9.3 servers, NSN is stored as two + * 32-bit fields on disk, same as LSNs. + */ +typedef PageXLogRecPtr PageGistNSN; typedef struct GISTPageOpaqueData { - GistNSN nsn; /* this value must change on page split */ + PageGistNSN nsn; /* this value must change on page split */ BlockNumber rightlink; /* next page if any */ uint16 flags; /* see bit definitions above */ uint16 gist_page_id; /* for identification of GiST indexes */ @@ -137,6 +142,9 @@ typedef struct GISTENTRY #define GistMarkFollowRight(page) ( GistPageGetOpaque(page)->flags |= F_FOLLOW_RIGHT) #define GistClearFollowRight(page) ( GistPageGetOpaque(page)->flags &= ~F_FOLLOW_RIGHT) +#define GistPageGetNSN(page) ( PageXLogRecPtrGet(GistPageGetOpaque(page)->nsn)) +#define GistPageSetNSN(page, val) ( PageXLogRecPtrSet(GistPageGetOpaque(page)->nsn, val)) + /* * Vector of GISTENTRY structs; user-defined methods union and picksplit * take it as one of their arguments diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 1e235c6d1e..cd562ef40c 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 201212081 +#define CATALOG_VERSION_NO 201301171 #endif diff --git a/src/include/storage/bufpage.h b/src/include/storage/bufpage.h index 7758407098..8c887cab73 100644 --- a/src/include/storage/bufpage.h +++ b/src/include/storage/bufpage.h @@ -82,6 +82,21 @@ typedef Pointer Page; typedef uint16 LocationIndex; +/* + * For historical reasons, the 64-bit LSN value is stored as two 32-bit + * values. + */ +typedef struct +{ + uint32 xlogid; /* high bits */ + uint32 xrecoff; /* low bits */ +} PageXLogRecPtr; + +#define PageXLogRecPtrGet(val) \ + ((uint64) (val).xlogid << 32 | (val).xrecoff) +#define PageXLogRecPtrSet(ptr, lsn) \ + ((ptr).xlogid = (uint32) ((lsn) >> 32), (ptr).xrecoff = (uint32) (lsn)) + /* * disk page organization * @@ -120,13 +135,6 @@ typedef uint16 LocationIndex; * are 15 bits. */ -/* for historical reasons, the LSN is stored as two 32-bit values. */ -typedef struct -{ - uint32 xlogid; /* high bits */ - uint32 xrecoff; /* low bits */ -} PageXLogRecPtr; - typedef struct PageHeaderData { /* XXX LSN is member of *any* block, not only page-organized ones */ @@ -319,13 +327,13 @@ typedef PageHeaderData *PageHeader; / sizeof(ItemIdData))) /* - * Additional macros for access to page headers + * Additional macros for access to page headers. (Beware multiple evaluation + * of the arguments!) */ #define PageGetLSN(page) \ - ((uint64) ((PageHeader) (page))->pd_lsn.xlogid << 32 | ((PageHeader) (page))->pd_lsn.xrecoff) + PageXLogRecPtrGet(((PageHeader) (page))->pd_lsn) #define PageSetLSN(page, lsn) \ - (((PageHeader) (page))->pd_lsn.xlogid = (uint32) ((lsn) >> 32), \ - ((PageHeader) (page))->pd_lsn.xrecoff = (uint32) (lsn)) + PageXLogRecPtrSet(((PageHeader) (page))->pd_lsn, lsn) /* NOTE: only the 16 least significant bits are stored */ #define PageGetTLI(page) \