aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorAlvaro Herrera <alvherre@alvh.no-ip.org>2015-10-27 18:17:55 -0300
committerAlvaro Herrera <alvherre@alvh.no-ip.org>2015-10-27 18:17:55 -0300
commit21a4e4a4c9fe417e2462b6f90f6b0e49e32ceba6 (patch)
tree49dae19887e79f3b91c90200a82a3594c26c2f78 /src
parent531d21b75ff6b18ea8638c736a05326ebd519f49 (diff)
downloadpostgresql-21a4e4a4c9fe417e2462b6f90f6b0e49e32ceba6.tar.gz
postgresql-21a4e4a4c9fe417e2462b6f90f6b0e49e32ceba6.zip
Fix BRIN free space computations
A bug in the original free space computation made it possible to return a page which wasn't actually able to fit the item. Since the insertion code isn't prepared to deal with PageAddItem failing, a PANIC resulted ("failed to add BRIN tuple [to new page]"). Add a macro to encapsulate the correct computation, and use it in brin_getinsertbuffer's callers before calling that routine, to raise an early error. I became aware of the possiblity of a problem in this area while working on ccc4c074994d734. There's no archived discussion about it, but it's easy to reproduce a problem in the unpatched code with something like CREATE TABLE t (a text); CREATE INDEX ti ON t USING brin (a) WITH (pages_per_range=1); for length in `seq 8000 8196` do psql -f - <<EOF TRUNCATE TABLE t; INSERT INTO t VALUES ('z'), (repeat('a', $length)); EOF done Backpatch to 9.5, where BRIN was introduced.
Diffstat (limited to 'src')
-rw-r--r--src/backend/access/brin/brin_pageops.c63
1 files changed, 43 insertions, 20 deletions
diff --git a/src/backend/access/brin/brin_pageops.c b/src/backend/access/brin/brin_pageops.c
index 6a2fd474d9a..f876f62cbbd 100644
--- a/src/backend/access/brin/brin_pageops.c
+++ b/src/backend/access/brin/brin_pageops.c
@@ -23,6 +23,16 @@
#include "utils/rel.h"
+/*
+ * Maximum size of an entry in a BRIN_PAGETYPE_REGULAR page. We can tolerate
+ * a single item per page, unlike other index AMs.
+ */
+#define BrinMaxItemSize \
+ MAXALIGN_DOWN(BLCKSZ - \
+ (MAXALIGN(SizeOfPageHeaderData + \
+ sizeof(ItemIdData)) + \
+ MAXALIGN(sizeof(BrinSpecialSpace))))
+
static Buffer brin_getinsertbuffer(Relation irel, Buffer oldbuf, Size itemsz,
bool *extended);
static Size br_page_get_freespace(Page page);
@@ -58,6 +68,18 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
Assert(newsz == MAXALIGN(newsz));
+ /* If the item is oversized, don't bother. */
+ if (newsz > BrinMaxItemSize)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("index row size %lu exceeds maximum %lu for index \"%s\"",
+ (unsigned long) newsz,
+ (unsigned long) BrinMaxItemSize,
+ RelationGetRelationName(idxrel))));
+ return false; /* keep compiler quiet */
+ }
+
/* make sure the revmap is long enough to contain the entry we need */
brinRevmapExtend(revmap, heapBlk);
@@ -332,6 +354,18 @@ brin_doinsert(Relation idxrel, BlockNumber pagesPerRange,
Assert(itemsz == MAXALIGN(itemsz));
+ /* If the item is oversized, don't even bother. */
+ if (itemsz > BrinMaxItemSize)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("index row size %lu exceeds maximum %lu for index \"%s\"",
+ (unsigned long) itemsz,
+ (unsigned long) BrinMaxItemSize,
+ RelationGetRelationName(idxrel))));
+ return InvalidOffsetNumber; /* keep compiler quiet */
+ }
+
/* Make sure the revmap is long enough to contain the entry we need */
brinRevmapExtend(revmap, heapBlk);
@@ -360,9 +394,9 @@ brin_doinsert(Relation idxrel, BlockNumber pagesPerRange,
*/
if (!BufferIsValid(*buffer))
{
- *buffer = brin_getinsertbuffer(idxrel, InvalidBuffer, itemsz, &extended);
- Assert(BufferIsValid(*buffer));
- Assert(extended || br_page_get_freespace(BufferGetPage(*buffer)) >= itemsz);
+ do
+ *buffer = brin_getinsertbuffer(idxrel, InvalidBuffer, itemsz, &extended);
+ while (!BufferIsValid(*buffer));
}
else
extended = false;
@@ -610,8 +644,9 @@ brin_page_cleanup(Relation idxrel, Buffer buf)
/*
* Return a pinned and exclusively locked buffer which can be used to insert an
- * index item of size itemsz. If oldbuf is a valid buffer, it is also locked
- * (in an order determined to avoid deadlocks.)
+ * index item of size itemsz (caller must ensure not to request sizes
+ * impossible to fulfill). If oldbuf is a valid buffer, it is also locked (in
+ * an order determined to avoid deadlocks.)
*
* If we find that the old page is no longer a regular index page (because
* of a revmap extension), the old buffer is unlocked and we return
@@ -636,20 +671,8 @@ brin_getinsertbuffer(Relation irel, Buffer oldbuf, Size itemsz,
Page page;
int freespace;
- /*
- * If the item is oversized, don't bother. We have another, more precise
- * check below.
- */
- if (itemsz > BLCKSZ - sizeof(BrinSpecialSpace))
- {
- ereport(ERROR,
- (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
- errmsg("index row size %lu exceeds maximum %lu for index \"%s\"",
- (unsigned long) itemsz,
- (unsigned long) BLCKSZ - sizeof(BrinSpecialSpace),
- RelationGetRelationName(irel))));
- return InvalidBuffer; /* keep compiler quiet */
- }
+ /* callers must have checked */
+ Assert(itemsz <= BrinMaxItemSize);
*extended = false;
@@ -759,7 +782,7 @@ brin_getinsertbuffer(Relation irel, Buffer oldbuf, Size itemsz,
* page that has since been repurposed for the revmap.)
*/
freespace = *extended ?
- BLCKSZ - sizeof(BrinSpecialSpace) : br_page_get_freespace(page);
+ BrinMaxItemSize : br_page_get_freespace(page);
if (freespace >= itemsz)
{
RelationSetTargetBlock(irel, BufferGetBlockNumber(buf));