Revert no-op changes to BufferGetPage()

The reverted changes were intended to force a choice of whether any
newly-added BufferGetPage() calls needed to be accompanied by a
test of the snapshot age, to support the "snapshot too old"
feature.  Such an accompanying test is needed in about 7% of the
cases, where the page is being used as part of a scan rather than
positioning for other purposes (such as DML or vacuuming).  The
additional effort required for back-patching, and the doubt whether
the intended benefit would really be there, have indicated it is
best just to rely on developers to do the right thing based on
comments and existing usage, as we do with many other conventions.

This change should have little or no effect on generated executable
code.

Motivated by the back-patching pain of Tom Lane and Robert Haas

src/backend/access/hash/hash.c

@@ -278,8 +278,8 @@ hashgettuple(IndexScanDesc scan, ScanDirection dir)
 
 		buf = so->hashso_curbuf;
 		Assert(BufferIsValid(buf));
-		page = BufferGetPage(buf, scan->xs_snapshot, rel,
-							 BGP_TEST_FOR_OLD_SNAPSHOT);
+		page = BufferGetPage(buf);
+		TestForOldSnapshot(scan->xs_snapshot, rel, page);
 		maxoffnum = PageGetMaxOffsetNumber(page);
 		for (offnum = ItemPointerGetOffsetNumber(current);
 			 offnum <= maxoffnum;
@@ -328,8 +328,7 @@ hashgettuple(IndexScanDesc scan, ScanDirection dir)
 		while (res)
 		{
 			offnum = ItemPointerGetOffsetNumber(current);
-			page = BufferGetPage(so->hashso_curbuf, NULL, NULL,
-								 BGP_NO_SNAPSHOT_TEST);
+			page = BufferGetPage(so->hashso_curbuf);
 			if (!ItemIdIsDead(PageGetItemId(page, offnum)))
 				break;
 			res = _hash_next(scan, dir);
@@ -372,8 +371,7 @@ hashgetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
 			OffsetNumber offnum;
 
 			offnum = ItemPointerGetOffsetNumber(&(so->hashso_curpos));
-			page = BufferGetPage(so->hashso_curbuf, NULL, NULL,
-								 BGP_NO_SNAPSHOT_TEST);
+			page = BufferGetPage(so->hashso_curbuf);
 			add_tuple = !ItemIdIsDead(PageGetItemId(page, offnum));
 		}
 		else
@@ -518,8 +516,7 @@ hashbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	 * each bucket.
 	 */
 	metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_READ, LH_META_PAGE);
-	metap = HashPageGetMeta(BufferGetPage(metabuf, NULL, NULL,
-										  BGP_NO_SNAPSHOT_TEST));
+	metap = HashPageGetMeta(BufferGetPage(metabuf));
 	orig_maxbucket = metap->hashm_maxbucket;
 	orig_ntuples = metap->hashm_ntuples;
 	memcpy(&local_metapage, metap, sizeof(local_metapage));
@@ -563,7 +560,7 @@ loop_top:
 			buf = _hash_getbuf_with_strategy(rel, blkno, HASH_WRITE,
 										   LH_BUCKET_PAGE | LH_OVERFLOW_PAGE,
 											 info->strategy);
-			page = BufferGetPage(buf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+			page = BufferGetPage(buf);
 			opaque = (HashPageOpaque) PageGetSpecialPointer(page);
 			Assert(opaque->hasho_bucket == cur_bucket);
 
@@ -618,8 +615,7 @@ loop_top:
 
 	/* Write-lock metapage and check for split since we started */
 	metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_WRITE, LH_META_PAGE);
-	metap = HashPageGetMeta(BufferGetPage(metabuf, NULL, NULL,
-										  BGP_NO_SNAPSHOT_TEST));
+	metap = HashPageGetMeta(BufferGetPage(metabuf));
 
 	if (cur_maxbucket != metap->hashm_maxbucket)
 	{

src/backend/access/hash/hashinsert.c

@@ -53,8 +53,7 @@ _hash_doinsert(Relation rel, IndexTuple itup)
 
 	/* Read the metapage */
 	metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_READ, LH_META_PAGE);
-	metap = HashPageGetMeta(BufferGetPage(metabuf, NULL, NULL,
-										  BGP_NO_SNAPSHOT_TEST));
+	metap = HashPageGetMeta(BufferGetPage(metabuf));
 
 	/*
 	 * Check whether the item can fit on a hash page at all. (Eventually, we
@@ -112,7 +111,7 @@ _hash_doinsert(Relation rel, IndexTuple itup)
 
 	/* Fetch the primary bucket page for the bucket */
 	buf = _hash_getbuf(rel, blkno, HASH_WRITE, LH_BUCKET_PAGE);
-	page = BufferGetPage(buf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+	page = BufferGetPage(buf);
 	pageopaque = (HashPageOpaque) PageGetSpecialPointer(page);
 	Assert(pageopaque->hasho_bucket == bucket);
 
@@ -132,7 +131,7 @@ _hash_doinsert(Relation rel, IndexTuple itup)
 			 */
 			_hash_relbuf(rel, buf);
 			buf = _hash_getbuf(rel, nextblkno, HASH_WRITE, LH_OVERFLOW_PAGE);
-			page = BufferGetPage(buf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+			page = BufferGetPage(buf);
 		}
 		else
 		{
@@ -146,7 +145,7 @@ _hash_doinsert(Relation rel, IndexTuple itup)
 
 			/* chain to a new overflow page */
 			buf = _hash_addovflpage(rel, metabuf, buf);
-			page = BufferGetPage(buf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+			page = BufferGetPage(buf);
 
 			/* should fit now, given test above */
 			Assert(PageGetFreeSpace(page) >= itemsz);
@@ -207,7 +206,7 @@ _hash_pgaddtup(Relation rel, Buffer buf, Size itemsize, IndexTuple itup)
 	uint32		hashkey;
 
 	_hash_checkpage(rel, buf, LH_BUCKET_PAGE | LH_OVERFLOW_PAGE);
-	page = BufferGetPage(buf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+	page = BufferGetPage(buf);
 
 	/* Find where to insert the tuple (preserving page's hashkey ordering) */
 	hashkey = _hash_get_indextuple_hashkey(itup);

src/backend/access/hash/hashovfl.c

@@ -123,7 +123,7 @@ _hash_addovflpage(Relation rel, Buffer metabuf, Buffer buf)
 	{
 		BlockNumber nextblkno;
 
-		page = BufferGetPage(buf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+		page = BufferGetPage(buf);
 		pageopaque = (HashPageOpaque) PageGetSpecialPointer(page);
 		nextblkno = pageopaque->hasho_nextblkno;
 
@@ -137,7 +137,7 @@ _hash_addovflpage(Relation rel, Buffer metabuf, Buffer buf)
 	}
 
 	/* now that we have correct backlink, initialize new overflow page */
-	ovflpage = BufferGetPage(ovflbuf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+	ovflpage = BufferGetPage(ovflbuf);
 	ovflopaque = (HashPageOpaque) PageGetSpecialPointer(ovflpage);
 	ovflopaque->hasho_prevblkno = BufferGetBlockNumber(buf);
 	ovflopaque->hasho_nextblkno = InvalidBlockNumber;
@@ -186,8 +186,7 @@ _hash_getovflpage(Relation rel, Buffer metabuf)
 	_hash_chgbufaccess(rel, metabuf, HASH_NOLOCK, HASH_WRITE);
 
 	_hash_checkpage(rel, metabuf, LH_META_PAGE);
-	metap = HashPageGetMeta(BufferGetPage(metabuf, NULL, NULL,
-										  BGP_NO_SNAPSHOT_TEST));
+	metap = HashPageGetMeta(BufferGetPage(metabuf));
 
 	/* start search at hashm_firstfree */
 	orig_firstfree = metap->hashm_firstfree;
@@ -225,7 +224,7 @@ _hash_getovflpage(Relation rel, Buffer metabuf)
 		_hash_chgbufaccess(rel, metabuf, HASH_READ, HASH_NOLOCK);
 
 		mapbuf = _hash_getbuf(rel, mapblkno, HASH_WRITE, LH_BITMAP_PAGE);
-		mappage = BufferGetPage(mapbuf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+		mappage = BufferGetPage(mapbuf);
 		freep = HashPageGetBitmap(mappage);
 
 		for (; bit <= last_inpage; j++, bit += BITS_PER_MAP)
@@ -397,7 +396,7 @@ _hash_freeovflpage(Relation rel, Buffer ovflbuf,
 	/* Get information from the doomed page */
 	_hash_checkpage(rel, ovflbuf, LH_OVERFLOW_PAGE);
 	ovflblkno = BufferGetBlockNumber(ovflbuf);
-	ovflpage = BufferGetPage(ovflbuf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+	ovflpage = BufferGetPage(ovflbuf);
 	ovflopaque = (HashPageOpaque) PageGetSpecialPointer(ovflpage);
 	nextblkno = ovflopaque->hasho_nextblkno;
 	prevblkno = ovflopaque->hasho_prevblkno;
@@ -424,7 +423,7 @@ _hash_freeovflpage(Relation rel, Buffer ovflbuf,
 														 HASH_WRITE,
 										   LH_BUCKET_PAGE | LH_OVERFLOW_PAGE,
 														 bstrategy);
-		Page		prevpage = BufferGetPage(prevbuf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+		Page		prevpage = BufferGetPage(prevbuf);
 		HashPageOpaque prevopaque = (HashPageOpaque) PageGetSpecialPointer(prevpage);
 
 		Assert(prevopaque->hasho_bucket == bucket);
@@ -438,7 +437,7 @@ _hash_freeovflpage(Relation rel, Buffer ovflbuf,
 														 HASH_WRITE,
 														 LH_OVERFLOW_PAGE,
 														 bstrategy);
-		Page		nextpage = BufferGetPage(nextbuf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+		Page		nextpage = BufferGetPage(nextbuf);
 		HashPageOpaque nextopaque = (HashPageOpaque) PageGetSpecialPointer(nextpage);
 
 		Assert(nextopaque->hasho_bucket == bucket);
@@ -450,8 +449,7 @@ _hash_freeovflpage(Relation rel, Buffer ovflbuf,
 
 	/* Read the metapage so we can determine which bitmap page to use */
 	metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_READ, LH_META_PAGE);
-	metap = HashPageGetMeta(BufferGetPage(metabuf, NULL, NULL,
-										  BGP_NO_SNAPSHOT_TEST));
+	metap = HashPageGetMeta(BufferGetPage(metabuf));
 
 	/* Identify which bit to set */
 	ovflbitno = blkno_to_bitno(metap, ovflblkno);
@@ -468,7 +466,7 @@ _hash_freeovflpage(Relation rel, Buffer ovflbuf,
 
 	/* Clear the bitmap bit to indicate that this overflow page is free */
 	mapbuf = _hash_getbuf(rel, blkno, HASH_WRITE, LH_BITMAP_PAGE);
-	mappage = BufferGetPage(mapbuf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+	mappage = BufferGetPage(mapbuf);
 	freep = HashPageGetBitmap(mappage);
 	Assert(ISSET(freep, bitmapbit));
 	CLRBIT(freep, bitmapbit);
@@ -523,7 +521,7 @@ _hash_initbitmap(Relation rel, HashMetaPage metap, BlockNumber blkno,
 	 * that it's not worth worrying about.
 	 */
 	buf = _hash_getnewbuf(rel, blkno, forkNum);
-	pg = BufferGetPage(buf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+	pg = BufferGetPage(buf);
 
 	/* initialize the page's special space */
 	op = (HashPageOpaque) PageGetSpecialPointer(pg);
@@ -603,7 +601,7 @@ _hash_squeezebucket(Relation rel,
 									  HASH_WRITE,
 									  LH_BUCKET_PAGE,
 									  bstrategy);
-	wpage = BufferGetPage(wbuf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+	wpage = BufferGetPage(wbuf);
 	wopaque = (HashPageOpaque) PageGetSpecialPointer(wpage);
 
 	/*
@@ -633,7 +631,7 @@ _hash_squeezebucket(Relation rel,
 										  HASH_WRITE,
 										  LH_OVERFLOW_PAGE,
 										  bstrategy);
-		rpage = BufferGetPage(rbuf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+		rpage = BufferGetPage(rbuf);
 		ropaque = (HashPageOpaque) PageGetSpecialPointer(rpage);
 		Assert(ropaque->hasho_bucket == bucket);
 	} while (BlockNumberIsValid(ropaque->hasho_nextblkno));
@@ -698,7 +696,7 @@ _hash_squeezebucket(Relation rel,
 												  HASH_WRITE,
 												  LH_OVERFLOW_PAGE,
 												  bstrategy);
-				wpage = BufferGetPage(wbuf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+				wpage = BufferGetPage(wbuf);
 				wopaque = (HashPageOpaque) PageGetSpecialPointer(wpage);
 				Assert(wopaque->hasho_bucket == bucket);
 				wbuf_dirty = false;
@@ -754,7 +752,7 @@ _hash_squeezebucket(Relation rel,
 										  HASH_WRITE,
 										  LH_OVERFLOW_PAGE,
 										  bstrategy);
-		rpage = BufferGetPage(rbuf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+		rpage = BufferGetPage(rbuf);
 		ropaque = (HashPageOpaque) PageGetSpecialPointer(rpage);
 		Assert(ropaque->hasho_bucket == bucket);
 	}

src/backend/access/hash/hashpage.c

@@ -161,8 +161,7 @@ _hash_getinitbuf(Relation rel, BlockNumber blkno)
 	/* ref count and lock type are correct */
 
 	/* initialize the page */
-	_hash_pageinit(BufferGetPage(buf, NULL, NULL, BGP_NO_SNAPSHOT_TEST),
-				   BufferGetPageSize(buf));
+	_hash_pageinit(BufferGetPage(buf), BufferGetPageSize(buf));
 
 	return buf;
 }
@@ -211,8 +210,7 @@ _hash_getnewbuf(Relation rel, BlockNumber blkno, ForkNumber forkNum)
 	/* ref count and lock type are correct */
 
 	/* initialize the page */
-	_hash_pageinit(BufferGetPage(buf, NULL, NULL, BGP_NO_SNAPSHOT_TEST),
-				   BufferGetPageSize(buf));
+	_hash_pageinit(BufferGetPage(buf), BufferGetPageSize(buf));
 
 	return buf;
 }
@@ -386,7 +384,7 @@ _hash_metapinit(Relation rel, double num_tuples, ForkNumber forkNum)
 	 * the physical index length.
 	 */
 	metabuf = _hash_getnewbuf(rel, HASH_METAPAGE, forkNum);
-	pg = BufferGetPage(metabuf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+	pg = BufferGetPage(metabuf);
 
 	pageopaque = (HashPageOpaque) PageGetSpecialPointer(pg);
 	pageopaque->hasho_prevblkno = InvalidBlockNumber;
@@ -454,7 +452,7 @@ _hash_metapinit(Relation rel, double num_tuples, ForkNumber forkNum)
 		CHECK_FOR_INTERRUPTS();
 
 		buf = _hash_getnewbuf(rel, BUCKET_TO_BLKNO(metap, i), forkNum);
-		pg = BufferGetPage(buf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+		pg = BufferGetPage(buf);
 		pageopaque = (HashPageOpaque) PageGetSpecialPointer(pg);
 		pageopaque->hasho_prevblkno = InvalidBlockNumber;
 		pageopaque->hasho_nextblkno = InvalidBlockNumber;
@@ -519,8 +517,7 @@ _hash_expandtable(Relation rel, Buffer metabuf)
 	_hash_chgbufaccess(rel, metabuf, HASH_NOLOCK, HASH_WRITE);
 
 	_hash_checkpage(rel, metabuf, LH_META_PAGE);
-	metap = HashPageGetMeta(BufferGetPage(metabuf, NULL, NULL,
-										  BGP_NO_SNAPSHOT_TEST));
+	metap = HashPageGetMeta(BufferGetPage(metabuf));
 
 	/*
 	 * Check to see if split is still needed; someone else might have already
@@ -777,10 +774,10 @@ _hash_splitbucket(Relation rel,
 	 * either bucket.
 	 */
 	obuf = _hash_getbuf(rel, start_oblkno, HASH_WRITE, LH_BUCKET_PAGE);
-	opage = BufferGetPage(obuf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+	opage = BufferGetPage(obuf);
 	oopaque = (HashPageOpaque) PageGetSpecialPointer(opage);
 
-	npage = BufferGetPage(nbuf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+	npage = BufferGetPage(nbuf);
 
 	/* initialize the new bucket's primary page */
 	nopaque = (HashPageOpaque) PageGetSpecialPointer(npage);
@@ -844,7 +841,7 @@ _hash_splitbucket(Relation rel,
 					_hash_chgbufaccess(rel, nbuf, HASH_WRITE, HASH_NOLOCK);
 					/* chain to a new overflow page */
 					nbuf = _hash_addovflpage(rel, metabuf, nbuf);
-					npage = BufferGetPage(nbuf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+					npage = BufferGetPage(nbuf);
 					/* we don't need nopaque within the loop */
 				}
 
@@ -891,7 +888,7 @@ _hash_splitbucket(Relation rel,
 
 		/* Else, advance to next old page */
 		obuf = _hash_getbuf(rel, oblkno, HASH_WRITE, LH_OVERFLOW_PAGE);
-		opage = BufferGetPage(obuf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+		opage = BufferGetPage(obuf);
 		oopaque = (HashPageOpaque) PageGetSpecialPointer(opage);
 	}
 

src/backend/access/hash/hashsearch.c

@@ -55,7 +55,7 @@ _hash_next(IndexScanDesc scan, ScanDirection dir)
 	current = &(so->hashso_curpos);
 	offnum = ItemPointerGetOffsetNumber(current);
 	_hash_checkpage(rel, buf, LH_BUCKET_PAGE | LH_OVERFLOW_PAGE);
-	page = BufferGetPage(buf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+	page = BufferGetPage(buf);
 	itup = (IndexTuple) PageGetItem(page, PageGetItemId(page, offnum));
 	so->hashso_heappos = itup->t_tid;
 
@@ -79,7 +79,7 @@ _hash_readnext(Relation rel,
 	if (BlockNumberIsValid(blkno))
 	{
 		*bufp = _hash_getbuf(rel, blkno, HASH_READ, LH_OVERFLOW_PAGE);
-		*pagep = BufferGetPage(*bufp, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+		*pagep = BufferGetPage(*bufp);
 		*opaquep = (HashPageOpaque) PageGetSpecialPointer(*pagep);
 	}
 }
@@ -102,7 +102,7 @@ _hash_readprev(Relation rel,
 	{
 		*bufp = _hash_getbuf(rel, blkno, HASH_READ,
 							 LH_BUCKET_PAGE | LH_OVERFLOW_PAGE);
-		*pagep = BufferGetPage(*bufp, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+		*pagep = BufferGetPage(*bufp);
 		*opaquep = (HashPageOpaque) PageGetSpecialPointer(*pagep);
 	}
 }
@@ -188,8 +188,8 @@ _hash_first(IndexScanDesc scan, ScanDirection dir)
 
 	/* Read the metapage */
 	metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_READ, LH_META_PAGE);
-	page = BufferGetPage(metabuf, scan->xs_snapshot, rel,
-						 BGP_TEST_FOR_OLD_SNAPSHOT);
+	page = BufferGetPage(metabuf);
+	TestForOldSnapshot(scan->xs_snapshot, rel, page);
 	metap = HashPageGetMeta(page);
 
 	/*
@@ -242,8 +242,8 @@ _hash_first(IndexScanDesc scan, ScanDirection dir)
 
 	/* Fetch the primary bucket page for the bucket */
 	buf = _hash_getbuf(rel, blkno, HASH_READ, LH_BUCKET_PAGE);
-	page = BufferGetPage(buf, scan->xs_snapshot, rel,
-						 BGP_TEST_FOR_OLD_SNAPSHOT);
+	page = BufferGetPage(buf);
+	TestForOldSnapshot(scan->xs_snapshot, rel, page);
 	opaque = (HashPageOpaque) PageGetSpecialPointer(page);
 	Assert(opaque->hasho_bucket == bucket);
 
@@ -261,7 +261,7 @@ _hash_first(IndexScanDesc scan, ScanDirection dir)
 	/* if we're here, _hash_step found a valid tuple */
 	offnum = ItemPointerGetOffsetNumber(current);
 	_hash_checkpage(rel, buf, LH_BUCKET_PAGE | LH_OVERFLOW_PAGE);
-	page = BufferGetPage(buf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+	page = BufferGetPage(buf);
 	itup = (IndexTuple) PageGetItem(page, PageGetItemId(page, offnum));
 	so->hashso_heappos = itup->t_tid;
 
@@ -297,7 +297,7 @@ _hash_step(IndexScanDesc scan, Buffer *bufP, ScanDirection dir)
 
 	buf = *bufP;
 	_hash_checkpage(rel, buf, LH_BUCKET_PAGE | LH_OVERFLOW_PAGE);
-	page = BufferGetPage(buf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+	page = BufferGetPage(buf);
 	opaque = (HashPageOpaque) PageGetSpecialPointer(page);
 
 	/*

src/backend/access/hash/hashutil.c

@@ -155,7 +155,7 @@ _hash_log2(uint32 num)
 void
 _hash_checkpage(Relation rel, Buffer buf, int flags)
 {
-	Page		page = BufferGetPage(buf, NULL, NULL, BGP_NO_SNAPSHOT_TEST);
+	Page		page = BufferGetPage(buf);
 
 	/*
 	 * ReadBuffer verifies that every newly-read page passes