Commit 975ad4e6 authored by Alvaro Herrera's avatar Alvaro Herrera

Fix PageAddItem BRIN bug

BRIN was relying on the ability to remove a tuple from an index page,
then putting another tuple in the same line pointer.  But PageAddItem
refuses to add a tuple beyond the first free item past the last used
item, and in particular, it rejects an attempt to add an item to an
empty page anywhere other than the first line pointer.  PageAddItem
issues a WARNING and indicates to the caller that it failed, which in
turn causes the BRIN calling code to issue a PANIC, so the whole
sequence looks like this:
	WARNING:  specified item offset is too large
	PANIC:  failed to add BRIN tuple

To fix, create a new function PageAddItemExtended which is like
PageAddItem except that the two boolean arguments become a flags bitmap;
the "overwrite" and "is_heap" boolean flags in PageAddItem become
PAI_OVERWITE and PAI_IS_HEAP flags in the new function, and a new flag
PAI_ALLOW_FAR_OFFSET enables the behavior required by BRIN.
PageAddItem() retains its original signature, for compatibility with
third-party modules (other callers in core code are not modified,
either).

Also, in the belt-and-suspenders spirit, I added a new sanity check in
brinGetTupleForHeapBlock to raise an error if an TID found in the revmap
is not marked as live by the page header.  This causes it to react with
"ERROR: corrupted BRIN index" to the bug at hand, rather than a hard
crash.

Backpatch to 9.5.

Bug reported by Andreas Seltenreich as detected by his handy sqlsmith
fuzzer.
Discussion: https://www.postgresql.org/message-id/87mvni77jh.fsf@elite.ansel.ydns.eu
parent 3c8aa665
...@@ -179,8 +179,8 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange, ...@@ -179,8 +179,8 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
START_CRIT_SECTION(); START_CRIT_SECTION();
PageIndexDeleteNoCompact(oldpage, &oldoff, 1); PageIndexDeleteNoCompact(oldpage, &oldoff, 1);
if (PageAddItem(oldpage, (Item) newtup, newsz, oldoff, true, if (PageAddItemExtended(oldpage, (Item) newtup, newsz, oldoff,
false) == InvalidOffsetNumber) PAI_OVERWRITE | PAI_ALLOW_FAR_OFFSET) == InvalidOffsetNumber)
elog(ERROR, "failed to add BRIN tuple"); elog(ERROR, "failed to add BRIN tuple");
MarkBufferDirty(oldbuf); MarkBufferDirty(oldbuf);
......
...@@ -272,6 +272,10 @@ brinGetTupleForHeapBlock(BrinRevmap *revmap, BlockNumber heapBlk, ...@@ -272,6 +272,10 @@ brinGetTupleForHeapBlock(BrinRevmap *revmap, BlockNumber heapBlk,
/* If we land on a revmap page, start over */ /* If we land on a revmap page, start over */
if (BRIN_IS_REGULAR_PAGE(page)) if (BRIN_IS_REGULAR_PAGE(page))
{ {
if (*off > PageGetMaxOffsetNumber(page))
ereport(ERROR,
(errcode(ERRCODE_INDEX_CORRUPTED),
errmsg_internal("corrupted BRIN index: inconsistent range map")));
lp = PageGetItemId(page, *off); lp = PageGetItemId(page, *off);
if (ItemIdIsUsed(lp)) if (ItemIdIsUsed(lp))
{ {
......
...@@ -193,7 +193,8 @@ brin_xlog_samepage_update(XLogReaderState *record) ...@@ -193,7 +193,8 @@ brin_xlog_samepage_update(XLogReaderState *record)
elog(PANIC, "brin_xlog_samepage_update: invalid max offset number"); elog(PANIC, "brin_xlog_samepage_update: invalid max offset number");
PageIndexDeleteNoCompact(page, &offnum, 1); PageIndexDeleteNoCompact(page, &offnum, 1);
offnum = PageAddItem(page, (Item) brintuple, tuplen, offnum, true, false); offnum = PageAddItemExtended(page, (Item) brintuple, tuplen, offnum,
PAI_OVERWRITE | PAI_ALLOW_FAR_OFFSET);
if (offnum == InvalidOffsetNumber) if (offnum == InvalidOffsetNumber)
elog(PANIC, "brin_xlog_samepage_update: failed to add tuple"); elog(PANIC, "brin_xlog_samepage_update: failed to add tuple");
......
...@@ -153,12 +153,13 @@ PageIsVerified(Page page, BlockNumber blkno) ...@@ -153,12 +153,13 @@ PageIsVerified(Page page, BlockNumber blkno)
/* /*
* PageAddItem * PageAddItemExtended
* *
* Add an item to a page. Return value is offset at which it was * Add an item to a page. Return value is the offset at which it was
* inserted, or InvalidOffsetNumber if there's not room to insert. * inserted, or InvalidOffsetNumber if the item is not inserted for any
* reason. A WARNING is issued indicating the reason for the refusal.
* *
* If overwrite is true, we just store the item at the specified * If flag PAI_OVERWRITE is set, we just store the item at the specified
* offsetNumber (which must be either a currently-unused item pointer, * offsetNumber (which must be either a currently-unused item pointer,
* or one past the last existing item). Otherwise, * or one past the last existing item). Otherwise,
* if offsetNumber is valid and <= current max offset in the page, * if offsetNumber is valid and <= current max offset in the page,
...@@ -167,18 +168,20 @@ PageIsVerified(Page page, BlockNumber blkno) ...@@ -167,18 +168,20 @@ PageIsVerified(Page page, BlockNumber blkno)
* If offsetNumber is not valid, then assign one by finding the first * If offsetNumber is not valid, then assign one by finding the first
* one that is both unused and deallocated. * one that is both unused and deallocated.
* *
* If is_heap is true, we enforce that there can't be more than * If flag PAI_IS_HEAP is set, we enforce that there can't be more than
* MaxHeapTuplesPerPage line pointers on the page. * MaxHeapTuplesPerPage line pointers on the page.
* *
* If flag PAI_ALLOW_FAR_OFFSET is not set, we disallow placing items
* beyond one past the last existing item.
*
* !!! EREPORT(ERROR) IS DISALLOWED HERE !!! * !!! EREPORT(ERROR) IS DISALLOWED HERE !!!
*/ */
OffsetNumber OffsetNumber
PageAddItem(Page page, PageAddItemExtended(Page page,
Item item, Item item,
Size size, Size size,
OffsetNumber offsetNumber, OffsetNumber offsetNumber,
bool overwrite, int flags)
bool is_heap)
{ {
PageHeader phdr = (PageHeader) page; PageHeader phdr = (PageHeader) page;
Size alignedSize; Size alignedSize;
...@@ -209,7 +212,7 @@ PageAddItem(Page page, ...@@ -209,7 +212,7 @@ PageAddItem(Page page,
if (OffsetNumberIsValid(offsetNumber)) if (OffsetNumberIsValid(offsetNumber))
{ {
/* yes, check it */ /* yes, check it */
if (overwrite) if ((flags & PAI_OVERWRITE) != 0)
{ {
if (offsetNumber < limit) if (offsetNumber < limit)
{ {
...@@ -257,13 +260,18 @@ PageAddItem(Page page, ...@@ -257,13 +260,18 @@ PageAddItem(Page page,
} }
} }
if (offsetNumber > limit) /*
* Reject placing items beyond the first unused line pointer, unless
* caller asked for that behavior specifically.
*/
if ((flags & PAI_ALLOW_FAR_OFFSET) == 0 && offsetNumber > limit)
{ {
elog(WARNING, "specified item offset is too large"); elog(WARNING, "specified item offset is too large");
return InvalidOffsetNumber; return InvalidOffsetNumber;
} }
if (is_heap && offsetNumber > MaxHeapTuplesPerPage) /* Reject placing items beyond heap boundary, if heap */
if ((flags & PAI_IS_HEAP) != 0 && offsetNumber > MaxHeapTuplesPerPage)
{ {
elog(WARNING, "can't put more than MaxHeapTuplesPerPage items in a heap page"); elog(WARNING, "can't put more than MaxHeapTuplesPerPage items in a heap page");
return InvalidOffsetNumber; return InvalidOffsetNumber;
...@@ -275,7 +283,10 @@ PageAddItem(Page page, ...@@ -275,7 +283,10 @@ PageAddItem(Page page,
* Note: do arithmetic as signed ints, to avoid mistakes if, say, * Note: do arithmetic as signed ints, to avoid mistakes if, say,
* alignedSize > pd_upper. * alignedSize > pd_upper.
*/ */
if (offsetNumber == limit || needshuffle) if ((flags & PAI_ALLOW_FAR_OFFSET) != 0)
lower = Max(phdr->pd_lower,
SizeOfPageHeaderData + sizeof(ItemIdData) * offsetNumber);
else if (offsetNumber == limit || needshuffle)
lower = phdr->pd_lower + sizeof(ItemIdData); lower = phdr->pd_lower + sizeof(ItemIdData);
else else
lower = phdr->pd_lower; lower = phdr->pd_lower;
...@@ -323,6 +334,27 @@ PageAddItem(Page page, ...@@ -323,6 +334,27 @@ PageAddItem(Page page,
return offsetNumber; return offsetNumber;
} }
/*
* PageAddItem
*
* Add an item to a page. Return value is offset at which it was
* inserted, or InvalidOffsetNumber if the item is not inserted for
* any reason.
*
* Passing the 'overwrite' and 'is_heap' parameters as true causes the
* PAI_OVERWRITE and PAI_IS_HEAP flags to be set, respectively.
*
* !!! EREPORT(ERROR) IS DISALLOWED HERE !!!
*/
OffsetNumber
PageAddItem(Page page, Item item, Size size, OffsetNumber offsetNumber,
bool overwrite, bool is_heap)
{
return PageAddItemExtended(page, item, size, offsetNumber,
overwrite ? PAI_OVERWRITE : 0 |
is_heap ? PAI_IS_HEAP : 0);
}
/* /*
* PageGetTempPage * PageGetTempPage
* Get a temporary page in local memory for special processing. * Get a temporary page in local memory for special processing.
......
...@@ -407,11 +407,16 @@ do { \ ...@@ -407,11 +407,16 @@ do { \
* extern declarations * extern declarations
* ---------------------------------------------------------------- * ----------------------------------------------------------------
*/ */
#define PAI_OVERWRITE (1 << 0)
#define PAI_IS_HEAP (1 << 1)
#define PAI_ALLOW_FAR_OFFSET (1 << 2)
extern void PageInit(Page page, Size pageSize, Size specialSize); extern void PageInit(Page page, Size pageSize, Size specialSize);
extern bool PageIsVerified(Page page, BlockNumber blkno); extern bool PageIsVerified(Page page, BlockNumber blkno);
extern OffsetNumber PageAddItem(Page page, Item item, Size size, extern OffsetNumber PageAddItem(Page page, Item item, Size size,
OffsetNumber offsetNumber, bool overwrite, bool is_heap); OffsetNumber offsetNumber, bool overwrite, bool is_heap);
extern OffsetNumber PageAddItemExtended(Page page, Item item, Size size,
OffsetNumber offsetNumber, int flags);
extern Page PageGetTempPage(Page page); extern Page PageGetTempPage(Page page);
extern Page PageGetTempPageCopy(Page page); extern Page PageGetTempPageCopy(Page page);
extern Page PageGetTempPageCopySpecial(Page page); extern Page PageGetTempPageCopySpecial(Page page);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment