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.
 
 

diff --git a/src/backend/access/brin/brin_pageops.c b/src/backend/access/brin/brin_pageops.c
index 6a2fd474d9aa2c664e142fa5b1967da5a8202789..f876f62cbbdf36239ec67e42fc698cf1b983f7a7 100644 (file)
--- 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));