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.

5 files changed, 23 insertions, 28 deletions

diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 76cc36c3469..ae655497951 100644
--- 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 c34b86c94c7..50f1dec1631 100644
--- 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 b08a94b742d..7f55ded1f19 100644
--- 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 8dc80ad1e52..ebef984e7f1 100644
--- 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 39135bf52e7..97dc3c0fa91 100644
--- 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)
 	{