Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 975ad4e

Browse filesBrowse files
alvherrealvherre
committed
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
1 parent 3c8aa66 commit 975ad4e
Copy full SHA for 975ad4e

File tree

Expand file treeCollapse file tree

5 files changed

+60
-18
lines changed
Filter options
Expand file treeCollapse file tree

5 files changed

+60
-18
lines changed

‎src/backend/access/brin/brin_pageops.c

Copy file name to clipboardExpand all lines: src/backend/access/brin/brin_pageops.c
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,8 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
179179

180180
START_CRIT_SECTION();
181181
PageIndexDeleteNoCompact(oldpage, &oldoff, 1);
182-
if (PageAddItem(oldpage, (Item) newtup, newsz, oldoff, true,
183-
false) == InvalidOffsetNumber)
182+
if (PageAddItemExtended(oldpage, (Item) newtup, newsz, oldoff,
183+
PAI_OVERWRITE | PAI_ALLOW_FAR_OFFSET) == InvalidOffsetNumber)
184184
elog(ERROR, "failed to add BRIN tuple");
185185
MarkBufferDirty(oldbuf);
186186

‎src/backend/access/brin/brin_revmap.c

Copy file name to clipboardExpand all lines: src/backend/access/brin/brin_revmap.c
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,10 @@ brinGetTupleForHeapBlock(BrinRevmap *revmap, BlockNumber heapBlk,
272272
/* If we land on a revmap page, start over */
273273
if (BRIN_IS_REGULAR_PAGE(page))
274274
{
275+
if (*off > PageGetMaxOffsetNumber(page))
276+
ereport(ERROR,
277+
(errcode(ERRCODE_INDEX_CORRUPTED),
278+
errmsg_internal("corrupted BRIN index: inconsistent range map")));
275279
lp = PageGetItemId(page, *off);
276280
if (ItemIdIsUsed(lp))
277281
{

‎src/backend/access/brin/brin_xlog.c

Copy file name to clipboardExpand all lines: src/backend/access/brin/brin_xlog.c
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,8 @@ brin_xlog_samepage_update(XLogReaderState *record)
193193
elog(PANIC, "brin_xlog_samepage_update: invalid max offset number");
194194

195195
PageIndexDeleteNoCompact(page, &offnum, 1);
196-
offnum = PageAddItem(page, (Item) brintuple, tuplen, offnum, true, false);
196+
offnum = PageAddItemExtended(page, (Item) brintuple, tuplen, offnum,
197+
PAI_OVERWRITE | PAI_ALLOW_FAR_OFFSET);
197198
if (offnum == InvalidOffsetNumber)
198199
elog(PANIC, "brin_xlog_samepage_update: failed to add tuple");
199200

‎src/backend/storage/page/bufpage.c

Copy file name to clipboardExpand all lines: src/backend/storage/page/bufpage.c
+47-15Lines changed: 47 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -153,12 +153,13 @@ PageIsVerified(Page page, BlockNumber blkno)
153153

154154

155155
/*
156-
* PageAddItem
156+
* PageAddItemExtended
157157
*
158-
* Add an item to a page. Return value is offset at which it was
159-
* inserted, or InvalidOffsetNumber if there's not room to insert.
158+
* Add an item to a page. Return value is the offset at which it was
159+
* inserted, or InvalidOffsetNumber if the item is not inserted for any
160+
* reason. A WARNING is issued indicating the reason for the refusal.
160161
*
161-
* If overwrite is true, we just store the item at the specified
162+
* If flag PAI_OVERWRITE is set, we just store the item at the specified
162163
* offsetNumber (which must be either a currently-unused item pointer,
163164
* or one past the last existing item). Otherwise,
164165
* if offsetNumber is valid and <= current max offset in the page,
@@ -167,18 +168,20 @@ PageIsVerified(Page page, BlockNumber blkno)
167168
* If offsetNumber is not valid, then assign one by finding the first
168169
* one that is both unused and deallocated.
169170
*
170-
* If is_heap is true, we enforce that there can't be more than
171+
* If flag PAI_IS_HEAP is set, we enforce that there can't be more than
171172
* MaxHeapTuplesPerPage line pointers on the page.
172173
*
174+
* If flag PAI_ALLOW_FAR_OFFSET is not set, we disallow placing items
175+
* beyond one past the last existing item.
176+
*
173177
* !!! EREPORT(ERROR) IS DISALLOWED HERE !!!
174178
*/
175179
OffsetNumber
176-
PageAddItem(Page page,
177-
Item item,
178-
Size size,
179-
OffsetNumber offsetNumber,
180-
bool overwrite,
181-
bool is_heap)
180+
PageAddItemExtended(Page page,
181+
Item item,
182+
Size size,
183+
OffsetNumber offsetNumber,
184+
int flags)
182185
{
183186
PageHeader phdr = (PageHeader) page;
184187
Size alignedSize;
@@ -209,7 +212,7 @@ PageAddItem(Page page,
209212
if (OffsetNumberIsValid(offsetNumber))
210213
{
211214
/* yes, check it */
212-
if (overwrite)
215+
if ((flags & PAI_OVERWRITE) != 0)
213216
{
214217
if (offsetNumber < limit)
215218
{
@@ -257,13 +260,18 @@ PageAddItem(Page page,
257260
}
258261
}
259262

260-
if (offsetNumber > limit)
263+
/*
264+
* Reject placing items beyond the first unused line pointer, unless
265+
* caller asked for that behavior specifically.
266+
*/
267+
if ((flags & PAI_ALLOW_FAR_OFFSET) == 0 && offsetNumber > limit)
261268
{
262269
elog(WARNING, "specified item offset is too large");
263270
return InvalidOffsetNumber;
264271
}
265272

266-
if (is_heap && offsetNumber > MaxHeapTuplesPerPage)
273+
/* Reject placing items beyond heap boundary, if heap */
274+
if ((flags & PAI_IS_HEAP) != 0 && offsetNumber > MaxHeapTuplesPerPage)
267275
{
268276
elog(WARNING, "can't put more than MaxHeapTuplesPerPage items in a heap page");
269277
return InvalidOffsetNumber;
@@ -275,7 +283,10 @@ PageAddItem(Page page,
275283
* Note: do arithmetic as signed ints, to avoid mistakes if, say,
276284
* alignedSize > pd_upper.
277285
*/
278-
if (offsetNumber == limit || needshuffle)
286+
if ((flags & PAI_ALLOW_FAR_OFFSET) != 0)
287+
lower = Max(phdr->pd_lower,
288+
SizeOfPageHeaderData + sizeof(ItemIdData) * offsetNumber);
289+
else if (offsetNumber == limit || needshuffle)
279290
lower = phdr->pd_lower + sizeof(ItemIdData);
280291
else
281292
lower = phdr->pd_lower;
@@ -323,6 +334,27 @@ PageAddItem(Page page,
323334
return offsetNumber;
324335
}
325336

337+
/*
338+
* PageAddItem
339+
*
340+
* Add an item to a page. Return value is offset at which it was
341+
* inserted, or InvalidOffsetNumber if the item is not inserted for
342+
* any reason.
343+
*
344+
* Passing the 'overwrite' and 'is_heap' parameters as true causes the
345+
* PAI_OVERWRITE and PAI_IS_HEAP flags to be set, respectively.
346+
*
347+
* !!! EREPORT(ERROR) IS DISALLOWED HERE !!!
348+
*/
349+
OffsetNumber
350+
PageAddItem(Page page, Item item, Size size, OffsetNumber offsetNumber,
351+
bool overwrite, bool is_heap)
352+
{
353+
return PageAddItemExtended(page, item, size, offsetNumber,
354+
overwrite ? PAI_OVERWRITE : 0 |
355+
is_heap ? PAI_IS_HEAP : 0);
356+
}
357+
326358
/*
327359
* PageGetTempPage
328360
* Get a temporary page in local memory for special processing.

‎src/include/storage/bufpage.h

Copy file name to clipboardExpand all lines: src/include/storage/bufpage.h
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,11 +407,16 @@ do { \
407407
* extern declarations
408408
* ----------------------------------------------------------------
409409
*/
410+
#define PAI_OVERWRITE (1 << 0)
411+
#define PAI_IS_HEAP (1 << 1)
412+
#define PAI_ALLOW_FAR_OFFSET (1 << 2)
410413

411414
extern void PageInit(Page page, Size pageSize, Size specialSize);
412415
extern bool PageIsVerified(Page page, BlockNumber blkno);
413416
extern OffsetNumber PageAddItem(Page page, Item item, Size size,
414417
OffsetNumber offsetNumber, bool overwrite, bool is_heap);
418+
extern OffsetNumber PageAddItemExtended(Page page, Item item, Size size,
419+
OffsetNumber offsetNumber, int flags);
415420
extern Page PageGetTempPage(Page page);
416421
extern Page PageGetTempPageCopy(Page page);
417422
extern Page PageGetTempPageCopySpecial(Page page);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.