v3-0006-Add-heapgettup_advance_page.patch

From bc7332551da42cc1798b23f422c8d06f9057713a Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Wed, 30 Nov 2022 15:06:34 -0500
Subject: [PATCH v3 6/7] Add heapgettup_advance_page()

---
 src/backend/access/heap/heapam.c | 168 +++++++++++++------------------
 1 file changed, 72 insertions(+), 96 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index f9b2d5cadb..f1081ab529 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -634,6 +634,74 @@ heapgettup_continue_page(HeapScanDesc scan, BlockNumber block, ScanDirection
 	return page;
 }
 
+static inline BlockNumber
+heapgettup_advance_page(HeapScanDesc scan, BlockNumber block, ScanDirection dir)
+{
+	if (ScanDirectionIsBackward(dir))
+	{
+		if (block == scan->rs_startblock)
+			return InvalidBlockNumber;
+
+		if (scan->rs_numblocks != InvalidBlockNumber)
+		{
+			if (--scan->rs_numblocks == 0)
+				return InvalidBlockNumber;
+		}
+
+		if (block == 0)
+			block = scan->rs_nblocks;
+
+		block--;
+
+		return block;
+	}
+	else if (scan->rs_base.rs_parallel != NULL)
+	{
+		Assert(ScanDirectionIsForward(dir));
+
+		block = table_block_parallelscan_nextpage(scan->rs_base.rs_rd,
+				scan->rs_parallelworkerdata, (ParallelBlockTableScanDesc)
+				scan->rs_base.rs_parallel);
+
+		return block;
+	}
+	else
+	{
+		Assert(ScanDirectionIsForward(dir));
+
+		block++;
+
+		if (block >= scan->rs_nblocks)
+			block = 0;
+
+		if (block == scan->rs_startblock)
+			return InvalidBlockNumber;
+
+		if (scan->rs_numblocks != InvalidBlockNumber)
+		{
+			if (--scan->rs_numblocks == 0)
+				return InvalidBlockNumber;
+		}
+
+		/*
+		* Report our new scan position for synchronization purposes. We
+		* don't do that when moving backwards, however. That would just
+		* mess up any other forward-moving scanners.
+		*
+		* Note: we do this before checking for end of scan so that the
+		* final state of the position hint is back at the start of the
+		* rel.  That's not strictly necessary, but otherwise when you run
+		* the same query multiple times the starting position would shift
+		* a little bit backwards on every invocation, which is confusing.
+		* We don't guarantee any specific ordering in general, though.
+		*/
+		if (scan->rs_base.rs_flags & SO_ALLOW_SYNC)
+			ss_report_location(scan->rs_base.rs_rd, block);
+
+		return block;
+	}
+}
+
 /* ----------------
  *		heapgettup - fetch next heap tuple
  *
@@ -667,7 +735,6 @@ heapgettup(HeapScanDesc scan,
 	Snapshot	snapshot = scan->rs_base.rs_snapshot;
 	bool		backward = ScanDirectionIsBackward(dir);
 	BlockNumber block;
-	bool		finished;
 	Page		page;
 	OffsetNumber lineoff;
 	int			linesleft;
@@ -771,56 +838,12 @@ heapgettup(HeapScanDesc scan,
 		 */
 		LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
 
-		/*
-		 * advance to next/prior page and detect end of scan
-		 */
-		if (backward)
-		{
-			finished = (block == scan->rs_startblock) ||
-				(scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks == 0 : false);
-			if (block == 0)
-				block = scan->rs_nblocks;
-			block--;
-		}
-		else if (scan->rs_base.rs_parallel != NULL)
-		{
-			ParallelBlockTableScanDesc pbscan =
-			(ParallelBlockTableScanDesc) scan->rs_base.rs_parallel;
-			ParallelBlockTableScanWorker pbscanwork =
-			scan->rs_parallelworkerdata;
-
-			block = table_block_parallelscan_nextpage(scan->rs_base.rs_rd,
-													 pbscanwork, pbscan);
-			finished = (block == InvalidBlockNumber);
-		}
-		else
-		{
-			block++;
-			if (block >= scan->rs_nblocks)
-				block = 0;
-			finished = (block == scan->rs_startblock) ||
-				(scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks == 0 : false);
-
-			/*
-			 * Report our new scan position for synchronization purposes. We
-			 * don't do that when moving backwards, however. That would just
-			 * mess up any other forward-moving scanners.
-			 *
-			 * Note: we do this before checking for end of scan so that the
-			 * final state of the position hint is back at the start of the
-			 * rel.  That's not strictly necessary, but otherwise when you run
-			 * the same query multiple times the starting position would shift
-			 * a little bit backwards on every invocation, which is confusing.
-			 * We don't guarantee any specific ordering in general, though.
-			 */
-			if (scan->rs_base.rs_flags & SO_ALLOW_SYNC)
-				ss_report_location(scan->rs_base.rs_rd, block);
-		}
+		block = heapgettup_advance_page(scan, block, dir);
 
 		/*
 		 * return NULL if we've exhausted all the pages
 		 */
-		if (finished)
+		if (block == InvalidBlockNumber)
 		{
 			if (BufferIsValid(scan->rs_cbuf))
 				ReleaseBuffer(scan->rs_cbuf);
@@ -873,7 +896,6 @@ heapgettup_pagemode(HeapScanDesc scan,
 	HeapTuple	tuple = &(scan->rs_ctup);
 	bool		backward = ScanDirectionIsBackward(dir);
 	BlockNumber block;
-	bool		finished;
 	Page		page;
 	int			lineindex;
 	OffsetNumber lineoff;
@@ -969,57 +991,11 @@ heapgettup_pagemode(HeapScanDesc scan,
 				++lineindex;
 		}
 
-		/*
-		 * if we get here, it means we've exhausted the items on this page and
-		 * it's time to move to the next.
-		 */
-		if (backward)
-		{
-			finished = (block == scan->rs_startblock) ||
-				(scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks == 0 : false);
-			if (block == 0)
-				block = scan->rs_nblocks;
-			block--;
-		}
-		else if (scan->rs_base.rs_parallel != NULL)
-		{
-			ParallelBlockTableScanDesc pbscan =
-			(ParallelBlockTableScanDesc) scan->rs_base.rs_parallel;
-			ParallelBlockTableScanWorker pbscanwork =
-			scan->rs_parallelworkerdata;
-
-			block = table_block_parallelscan_nextpage(scan->rs_base.rs_rd,
-													 pbscanwork, pbscan);
-			finished = (block == InvalidBlockNumber);
-		}
-		else
-		{
-			block++;
-			if (block >= scan->rs_nblocks)
-				block = 0;
-			finished = (block == scan->rs_startblock) ||
-				(scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks == 0 : false);
-
-			/*
-			 * Report our new scan position for synchronization purposes. We
-			 * don't do that when moving backwards, however. That would just
-			 * mess up any other forward-moving scanners.
-			 *
-			 * Note: we do this before checking for end of scan so that the
-			 * final state of the position hint is back at the start of the
-			 * rel.  That's not strictly necessary, but otherwise when you run
-			 * the same query multiple times the starting position would shift
-			 * a little bit backwards on every invocation, which is confusing.
-			 * We don't guarantee any specific ordering in general, though.
-			 */
-			if (scan->rs_base.rs_flags & SO_ALLOW_SYNC)
-				ss_report_location(scan->rs_base.rs_rd, block);
-		}
-
+		block = heapgettup_advance_page(scan, block, dir);
 		/*
 		 * return NULL if we've exhausted all the pages
 		 */
-		if (finished)
+		if (block == InvalidBlockNumber)
 		{
 			if (BufferIsValid(scan->rs_cbuf))
 				ReleaseBuffer(scan->rs_cbuf);
-- 
2.34.1