On tis, 2012-01-24 at 13:18 -0500, Tom Lane wrote:
> Robert Haas <robertmh...@gmail.com> writes:
> > Yes, that's what I meant when I suggested it originally.  I'm just not
> > sure it's any nicer than adding ifdefs for USE_ASSERT_CHECKING.
> 
> I'm inclined to think that it probably is nicer, just because of less
> vertical space used.  But again, this opinion is contingent on what it
> will look like after pgindent gets done with it ...

Here is a demo diff of what pgindent would do with the various
approaches (btw., nice job on making pgindent easy to use for everyone
now).

As you can see, pgindent adds whitespace on top of #ifdef
USE_ASSERT_CHECKING, and messes up the vertical alignment of variable
definitions that contain extra attributes.

All things considered, I like the PG_USED_FOR_ASSERTS_ONLY solution best.

diff --git i/src/backend/access/hash/hashovfl.c w/src/backend/access/hash/hashovfl.c
index 130c296..b61c8ee 100644
--- i/src/backend/access/hash/hashovfl.c
+++ w/src/backend/access/hash/hashovfl.c
@@ -391,7 +391,10 @@ _hash_freeovflpage(Relation rel, Buffer ovflbuf,
 	uint32		ovflbitno;
 	int32		bitmappage,
 				bitmapbit;
+
+#ifdef USE_ASSERT_CHECKING
 	Bucket		bucket;
+#endif
 
 	/* Get information from the doomed page */
 	_hash_checkpage(rel, ovflbuf, LH_OVERFLOW_PAGE);
@@ -400,7 +403,9 @@ _hash_freeovflpage(Relation rel, Buffer ovflbuf,
 	ovflopaque = (HashPageOpaque) PageGetSpecialPointer(ovflpage);
 	nextblkno = ovflopaque->hasho_nextblkno;
 	prevblkno = ovflopaque->hasho_prevblkno;
+#ifdef USE_ASSERT_CHECKING
 	bucket = ovflopaque->hasho_bucket;
+#endif
 
 	/*
 	 * Zero the page for debugging's sake; then write and release it. (Note:
diff --git i/src/backend/executor/execCurrent.c w/src/backend/executor/execCurrent.c
index b07161f..2c8929b 100644
--- i/src/backend/executor/execCurrent.c
+++ w/src/backend/executor/execCurrent.c
@@ -151,7 +151,7 @@ execCurrentOf(CurrentOfExpr *cexpr,
 	{
 		ScanState  *scanstate;
 		bool		lisnull;
-		Oid			tuple_tableoid;
+		Oid tuple_tableoid PG_USED_FOR_ASSERTS_ONLY;
 		ItemPointer tuple_tid;
 
 		/*
diff --git i/src/backend/executor/nodeMaterial.c w/src/backend/executor/nodeMaterial.c
index b320b54..3a6bfec 100644
--- i/src/backend/executor/nodeMaterial.c
+++ w/src/backend/executor/nodeMaterial.c
@@ -66,7 +66,7 @@ ExecMaterial(MaterialState *node)
 			 * Allocate a second read pointer to serve as the mark. We know it
 			 * must have index 1, so needn't store that.
 			 */
-			int			ptrno;
+			int ptrno	PG_USED_FOR_ASSERTS_ONLY;
 
 			ptrno = tuplestore_alloc_read_pointer(tuplestorestate,
 												  node->eflags);
diff --git i/src/backend/executor/nodeSetOp.c w/src/backend/executor/nodeSetOp.c
index 7fa5730..ad2e80d 100644
--- i/src/backend/executor/nodeSetOp.c
+++ w/src/backend/executor/nodeSetOp.c
@@ -344,7 +344,7 @@ setop_fill_hash_table(SetOpState *setopstate)
 	SetOp	   *node = (SetOp *) setopstate->ps.plan;
 	PlanState  *outerPlan;
 	int			firstFlag;
-	bool		in_first_rel;
+	bool in_first_rel __attribute__((unused));
 
 	/*
 	 * get state info from node
diff --git i/src/include/c.h w/src/include/c.h
index 82acd14..2dd5c67 100644
--- i/src/include/c.h
+++ w/src/include/c.h
@@ -850,4 +850,10 @@ extern int	fdatasync(int fildes);
 /* /port compatibility functions */
 #include "port.h"
 
+#ifdef USE_ASSERT_CHECKING
+#define PG_USED_FOR_ASSERTS_ONLY
+#else
+#define PG_USED_FOR_ASSERTS_ONLY __attribute__((unused))
+#endif
+
 #endif   /* C_H */
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to