Fix some coding issues in BRIN

Reported by David Rowley: variadic macros are a problem.  Get rid of
them using a trick suggested by Tom Lane: add extra parentheses where
needed.  In the future we might decide we don't need the calls at all
and remove them, but it seems appropriate to keep them while this code
is still new.

Also from David Rowley: brininsert() was trying to use a variable before
initializing it.  Fix by moving the brin_form_tuple call (which
initializes the variable) to within the locked section.

Reported by Peter Eisentraut: can't use "new" as a struct member name,
because C++ compilers will choke on it, as reported by cpluspluscheck.

diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 76cc36c34699e0fbbba0384acfcdbc62b0dca554..ae655497951a60342c4ad99762d8caa11073adb0 100644 (file)
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -247,12 +247,10 @@ brininsert(PG_FUNCTION_ARGS)
                         * the same page though, so downstream we must be prepared to cope
                         * if this turns out to not be possible after all.
                         */
+                       newtup = brin_form_tuple(bdesc, heapBlk, dtup, &newsz);
                        samepage = brin_can_do_samepage_update(buf, origsz, newsz);
-
                        LockBuffer(buf, BUFFER_LOCK_UNLOCK);
 
-                       newtup = brin_form_tuple(bdesc, heapBlk, dtup, &newsz);
-
                        /*
                         * Try to update the tuple.  If this doesn't work for whatever
                         * reason, we need to restart from the top; the revmap might be
@@ -589,9 +587,10 @@ brinbuildCallback(Relation index,
        while (thisblock > state->bs_currRangeStart + state->bs_pagesPerRange - 1)
        {
 
-               BRIN_elog(DEBUG2, "brinbuildCallback: completed a range: %u--%u",
-                                 state->bs_currRangeStart,
-                                 state->bs_currRangeStart + state->bs_pagesPerRange);
+               BRIN_elog((DEBUG2,
+                                  "brinbuildCallback: completed a range: %u--%u",
+                                  state->bs_currRangeStart,
+                                  state->bs_currRangeStart + state->bs_pagesPerRange));
 
                /* create the index tuple and insert it */
                form_and_insert_tuple(state);

diff --git a/src/backend/access/brin/brin_pageops.c b/src/backend/access/brin/brin_pageops.c
index c34b86c94c70604f5cc8f305acf4ea60e95f4f51..50f1dec1631a376461fc7f4d4d62a2c5b56f450b 100644 (file)
--- a/src/backend/access/brin/brin_pageops.c
+++ b/src/backend/access/brin/brin_pageops.c
@@ -216,12 +216,12 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
 
                        info = XLOG_BRIN_UPDATE | (extended ? XLOG_BRIN_INIT_PAGE : 0);
 
-                       xlrec.new.node = idxrel->rd_node;
-                       ItemPointerSet(&xlrec.new.tid, BufferGetBlockNumber(newbuf), newoff);
-                       xlrec.new.heapBlk = heapBlk;
-                       xlrec.new.tuplen = newsz;
-                       xlrec.new.revmapBlk = BufferGetBlockNumber(revmapbuf);
-                       xlrec.new.pagesPerRange = pagesPerRange;
+                       xlrec.insert.node = idxrel->rd_node;
+                       ItemPointerSet(&xlrec.insert.tid, BufferGetBlockNumber(newbuf), newoff);
+                       xlrec.insert.heapBlk = heapBlk;
+                       xlrec.insert.tuplen = newsz;
+                       xlrec.insert.revmapBlk = BufferGetBlockNumber(revmapbuf);
+                       xlrec.insert.pagesPerRange = pagesPerRange;
                        ItemPointerSet(&xlrec.oldtid, BufferGetBlockNumber(oldbuf), oldoff);
 
                        rdata[0].data = (char *) &xlrec;
@@ -342,8 +342,8 @@ brin_doinsert(Relation idxrel, BlockNumber pagesPerRange,
                elog(ERROR, "could not insert new index tuple to page");
        MarkBufferDirty(*buffer);
 
-       BRIN_elog(DEBUG2, "inserted tuple (%u,%u) for range starting at %u",
-                         blk, off, heapBlk);
+       BRIN_elog((DEBUG2, "inserted tuple (%u,%u) for range starting at %u",
+                          blk, off, heapBlk));
 
        ItemPointerSet(&tid, blk, off);
        brinSetHeapBlockItemptr(revmapbuf, pagesPerRange, heapBlk, tid);
@@ -593,8 +593,8 @@ brin_getinsertbuffer(Relation irel, Buffer oldbuf, Size itemsz,
                        newblk = BufferGetBlockNumber(buf);
                        *was_extended = extended = true;
 
-                       BRIN_elog(DEBUG2, "brin_getinsertbuffer: extending to page %u",
-                                         BufferGetBlockNumber(buf));
+                       BRIN_elog((DEBUG2, "brin_getinsertbuffer: extending to page %u",
+                                          BufferGetBlockNumber(buf)));
                }
                else if (newblk == oldblk)
                {

diff --git a/src/backend/access/brin/brin_revmap.c b/src/backend/access/brin/brin_revmap.c
index b08a94b742d11d1e86afdffca37068e35176d561..7f55ded1f1945a09618e5fa2321187d9102b92ff 100644 (file)
--- a/src/backend/access/brin/brin_revmap.c
+++ b/src/backend/access/brin/brin_revmap.c
@@ -331,10 +331,6 @@ revmap_get_buffer(BrinRevmap *revmap, BlockNumber heapBlk)
        Assert(mapBlk != BRIN_METAPAGE_BLKNO &&
                   mapBlk <= revmap->rm_lastRevmapPage);
 
-       BRIN_elog(DEBUG2, "getting revmap page for logical page %lu (physical %u) for heap %u",
-                         HEAPBLK_TO_REVMAP_BLK(revmap->rm_pagesPerRange, heapBlk),
-                         mapBlk, heapBlk);
-
        /*
         * Obtain the buffer from which we need to read.  If we already have the
         * correct buffer in our access struct, use that; otherwise, release that,

diff --git a/src/backend/access/brin/brin_xlog.c b/src/backend/access/brin/brin_xlog.c
index 8dc80ad1e522e303a85e804f5e8d58bfc525fcf4..ebef984e7f161e7870fa3d31d841f631da12abb9 100644 (file)
--- a/src/backend/access/brin/brin_xlog.c
+++ b/src/backend/access/brin/brin_xlog.c
@@ -144,7 +144,7 @@ brin_xlog_update(XLogRecPtr lsn, XLogRecord *record)
 
        /* First remove the old tuple */
        blkno = ItemPointerGetBlockNumber(&(xlrec->oldtid));
-       action = XLogReadBufferForRedo(lsn, record, 2, xlrec->new.node,
+       action = XLogReadBufferForRedo(lsn, record, 2, xlrec->insert.node,
                                                                   blkno, &buffer);
        if (action == BLK_NEEDS_REDO)
        {
@@ -164,7 +164,7 @@ brin_xlog_update(XLogRecPtr lsn, XLogRecord *record)
        }
 
        /* Then insert the new tuple and update revmap, like in an insertion. */
-       brin_xlog_insert_update(lsn, record, &xlrec->new, newtup);
+       brin_xlog_insert_update(lsn, record, &xlrec->insert, newtup);
 
        if (BufferIsValid(buffer))
                UnlockReleaseBuffer(buffer);

diff --git a/src/backend/access/rmgrdesc/brindesc.c b/src/backend/access/rmgrdesc/brindesc.c
index 39135bf52e7225ff55e2fc35b7627bb10a9a3751..97dc3c0fa9115290db63ae412fd047d2601c6596 100644 (file)
--- a/src/backend/access/rmgrdesc/brindesc.c
+++ b/src/backend/access/rmgrdesc/brindesc.c
@@ -49,14 +49,14 @@ brin_desc(StringInfo buf, XLogRecord *record)
                xl_brin_update *xlrec = (xl_brin_update *) rec;
 
                appendStringInfo(buf, "rel %u/%u/%u heapBlk %u revmapBlk %u pagesPerRange %u old TID (%u,%u) TID (%u,%u)",
-                                                xlrec->new.node.spcNode, xlrec->new.node.dbNode,
-                                                xlrec->new.node.relNode,
-                                                xlrec->new.heapBlk, xlrec->new.revmapBlk,
-                                                xlrec->new.pagesPerRange,
+                                                xlrec->insert.node.spcNode, xlrec->insert.node.dbNode,
+                                                xlrec->insert.node.relNode,
+                                                xlrec->insert.heapBlk, xlrec->insert.revmapBlk,
+                                                xlrec->insert.pagesPerRange,
                                                 ItemPointerGetBlockNumber(&xlrec->oldtid),
                                                 ItemPointerGetOffsetNumber(&xlrec->oldtid),
-                                                ItemPointerGetBlockNumber(&xlrec->new.tid),
-                                                ItemPointerGetOffsetNumber(&xlrec->new.tid));
+                                                ItemPointerGetBlockNumber(&xlrec->insert.tid),
+                                                ItemPointerGetOffsetNumber(&xlrec->insert.tid));
        }
        else if (info == XLOG_BRIN_SAMEPAGE_UPDATE)
        {

diff --git a/src/include/access/brin_internal.h b/src/include/access/brin_internal.h
index 651ab5f67e4f276c360106a812ea66412c9dfabc..34491ef5f3f4d8170263a9d46e2417ffc2a6b791 100644 (file)
--- a/src/include/access/brin_internal.h
+++ b/src/include/access/brin_internal.h
@@ -72,13 +72,12 @@ typedef struct BrinDesc
 #define BRIN_PROCNUM_UNION                     4
 /* procedure numbers up to 10 are reserved for BRIN future expansion */
 
-#define BRIN_DEBUG
+#undef BRIN_DEBUG
 
-/* we allow debug if using GCC; otherwise don't bother */
-#if defined(BRIN_DEBUG) && defined(__GNUC__)
-#define BRIN_elog(level, ...)          elog(level, __VA_ARGS__)
+#ifdef BRIN_DEBUG
+#define BRIN_elog(args)                        elog args
 #else
-#define BRIN_elog(a)   void(0)
+#define BRIN_elog(args)                        ((void) 0)
 #endif
 
 /* brin.c */

diff --git a/src/include/access/brin_xlog.h b/src/include/access/brin_xlog.h
index 3d959e81d63defbdd405a5de825d0cd66ee18261..d748db4d0c6b0b0fe98ef2979c1e053870c90267 100644 (file)
--- a/src/include/access/brin_xlog.h
+++ b/src/include/access/brin_xlog.h
@@ -76,10 +76,10 @@ typedef struct xl_brin_insert
 typedef struct xl_brin_update
 {
        ItemPointerData oldtid;
-       xl_brin_insert new;
+       xl_brin_insert insert;
 } xl_brin_update;
 
-#define SizeOfBrinUpdate       (offsetof(xl_brin_update, new) + SizeOfBrinInsert)
+#define SizeOfBrinUpdate       (offsetof(xl_brin_update, insert) + SizeOfBrinInsert)
 
 /* This is what we need to know about a BRIN tuple samepage update */
 typedef struct xl_brin_samepage_update