v3-0004-Streamline-heapgettup-for-refactor.patch

From 4d63c4942028d96adb431d4cd6c35330ed6e76f0 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Wed, 30 Nov 2022 12:08:05 -0500
Subject: [PATCH v3 4/7] Streamline heapgettup*() for refactor

Flip initial logic to set local variables in order to make it easier to
add helper functions. Also, remove unneeded local variables.
---
 src/backend/access/heap/heapam.c | 168 ++++++++++---------------------
 src/include/access/heapam.h      |   6 +-
 2 files changed, 58 insertions(+), 116 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 142bcc0a1c..a4365beee1 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -610,7 +610,6 @@ heapgettup(HeapScanDesc scan,
 	BlockNumber block;
 	bool		finished;
 	Page		page;
-	int			lines;
 	OffsetNumber lineoff;
 	int			linesleft;
 	ItemId		lpp;
@@ -621,84 +620,57 @@ heapgettup(HeapScanDesc scan,
 		return;
 	}
 
-	if (ScanDirectionIsForward(dir))
+	if (!scan->rs_inited)
 	{
-		if (!scan->rs_inited)
-		{
-			block = heapgettup_initial_page(scan, dir);
+		block = heapgettup_initial_page(scan, dir);
 
-			if (block == InvalidBlockNumber)
-			{
-				Assert(!BufferIsValid(scan->rs_cbuf));
-				tuple->t_data = NULL;
-				return;
-			}
-			heapgetpage((TableScanDesc) scan, block);
-			lineoff = FirstOffsetNumber;
-		}
-		else
+		if (block == InvalidBlockNumber)
 		{
-			/* continue from previously returned page/tuple */
-			block = scan->rs_cblock; /* current page */
-			lineoff =			/* next offnum */
-				OffsetNumberNext(ItemPointerGetOffsetNumber(&(tuple->t_self)));
+			Assert(!BufferIsValid(scan->rs_cbuf));
+			tuple->t_data = NULL;
+			return;
 		}
 
-		LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
+		heapgetpage((TableScanDesc) scan, block);
 
+		LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
 		page = BufferGetPage(scan->rs_cbuf);
 		TestForOldSnapshot(snapshot, scan->rs_base.rs_rd, page);
-		lines = PageGetMaxOffsetNumber(page);
-		/* block and lineoff now reference the physically next tid */
 
-		linesleft = lines - lineoff + 1;
+		linesleft = PageGetMaxOffsetNumber((Page) page) - FirstOffsetNumber + 1;
+
+		if (ScanDirectionIsForward(dir))
+			lineoff = FirstOffsetNumber;
+		else
+			lineoff = (OffsetNumber) linesleft;
 	}
 	else
 	{
-		Assert(backward);
-
-		if (!scan->rs_inited)
-		{
-			block = heapgettup_initial_page(scan, dir);
+		block = scan->rs_cblock; /* current page */
 
-			if (block == InvalidBlockNumber)
-			{
-				Assert(!BufferIsValid(scan->rs_cbuf));
-				tuple->t_data = NULL;
-				return;
-			}
+		LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
 
-			heapgetpage((TableScanDesc) scan, block);
-			LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
+		page = BufferGetPage(scan->rs_cbuf);
+		TestForOldSnapshot(snapshot, scan->rs_base.rs_rd, page);
 
-			page = BufferGetPage(scan->rs_cbuf);
-			TestForOldSnapshot(snapshot, scan->rs_base.rs_rd, page);
-			lines = PageGetMaxOffsetNumber(page);
-			lineoff = lines;	/* final offnum */
+		if (ScanDirectionIsForward(dir))
+		{
+			lineoff = OffsetNumberNext(scan->rs_cindex);
+			/* block and lineoff now reference the physically next tid */
+			linesleft = PageGetMaxOffsetNumber(page) - lineoff + 1;
 		}
 		else
 		{
-			/* continue from previously returned page/tuple */
-			block = scan->rs_cblock; /* current page */
-			LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
-
-			page = BufferGetPage(scan->rs_cbuf);
-			TestForOldSnapshot(snapshot, scan->rs_base.rs_rd, page);
-			lines = PageGetMaxOffsetNumber(page);
-
 			/*
 			 * The previous returned tuple may have been vacuumed since the
 			 * previous scan when we use a non-MVCC snapshot, so we must
 			 * re-establish the lineoff <= PageGetMaxOffsetNumber(page)
 			 * invariant
 			 */
-			lineoff =			/* previous offnum */
-				Min(lines,
-					OffsetNumberPrev(ItemPointerGetOffsetNumber(&(tuple->t_self))));
+			lineoff = Min(PageGetMaxOffsetNumber(page), OffsetNumberPrev(scan->rs_cindex));
+			/* block and lineoff now reference the physically previous tid */
+			linesleft = lineoff;
 		}
-		/* block and lineoff now reference the physically previous tid */
-
-		linesleft = lineoff;
 	}
 
 	/*
@@ -743,6 +715,7 @@ heapgettup(HeapScanDesc scan,
 				if (valid)
 				{
 					LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
+					scan->rs_cindex = lineoff;
 					return;
 				}
 			}
@@ -835,12 +808,11 @@ heapgettup(HeapScanDesc scan,
 
 		page = BufferGetPage(scan->rs_cbuf);
 		TestForOldSnapshot(snapshot, scan->rs_base.rs_rd, page);
-		lines = PageGetMaxOffsetNumber(page);
-		linesleft = lines;
+		linesleft = PageGetMaxOffsetNumber(page);
 		if (backward)
 		{
-			lineoff = lines;
-			lpp = PageGetItemId(page, lines);
+			lineoff = linesleft;
+			lpp = PageGetItemId(page, linesleft);
 		}
 		else
 		{
@@ -874,7 +846,6 @@ heapgettup_pagemode(HeapScanDesc scan,
 	BlockNumber block;
 	bool		finished;
 	Page		page;
-	int			lines;
 	int			lineindex;
 	OffsetNumber lineoff;
 	int			linesleft;
@@ -886,73 +857,41 @@ heapgettup_pagemode(HeapScanDesc scan,
 		return;
 	}
 
-	/*
-	 * calculate next starting lineindex, given scan direction
-	 */
-	if (ScanDirectionIsForward(dir))
+	if (!scan->rs_inited)
 	{
-		if (!scan->rs_inited)
-		{
-			block = heapgettup_initial_page(scan, dir);
+		block = heapgettup_initial_page(scan, dir);
 
-			if (block == InvalidBlockNumber)
-			{
-				Assert(!BufferIsValid(scan->rs_cbuf));
-				tuple->t_data = NULL;
-				return;
-			}
-
-			heapgetpage((TableScanDesc) scan, block);
-			lineindex = 0;
-		}
-		else
+		if (block == InvalidBlockNumber)
 		{
-			/* continue from previously returned page/tuple */
-			block = scan->rs_cblock; /* current page */
-			lineindex = scan->rs_cindex + 1;
+			Assert(!BufferIsValid(scan->rs_cbuf));
+			tuple->t_data = NULL;
+			return;
 		}
 
+		heapgetpage((TableScanDesc) scan, block);
 		page = BufferGetPage(scan->rs_cbuf);
 		TestForOldSnapshot(scan->rs_base.rs_snapshot, scan->rs_base.rs_rd, page);
-		lines = scan->rs_ntuples;
-		/* block and lineindex now reference the next visible tid */
-
-		linesleft = lines - lineindex;
+		linesleft = scan->rs_ntuples;
+		lineindex = ScanDirectionIsForward(dir) ? 0 : linesleft - 1;
 	}
 	else
 	{
-		Assert(backward);
-
-		if (!scan->rs_inited)
-		{
-			block = heapgettup_initial_page(scan, dir);
+		/* continue from previously returned page/tuple */
+		block = scan->rs_cblock; /* current page */
 
-			if (block == InvalidBlockNumber)
-			{
-				Assert(!BufferIsValid(scan->rs_cbuf));
-				tuple->t_data = NULL;
-				return;
-			}
+		page = BufferGetPage(scan->rs_cbuf);
+		TestForOldSnapshot(scan->rs_base.rs_snapshot, scan->rs_base.rs_rd, page);
 
-			heapgetpage((TableScanDesc) scan, block);
-			page = BufferGetPage(scan->rs_cbuf);
-			TestForOldSnapshot(scan->rs_base.rs_snapshot, scan->rs_base.rs_rd, page);
-			lines = scan->rs_ntuples;
-			lineindex = lines - 1;
-		}
+		lineindex = scan->rs_cindex + dir;
+		if (ScanDirectionIsForward(dir))
+			linesleft = scan->rs_ntuples - lineindex;
 		else
-		{
-			/* continue from previously returned page/tuple */
-			block = scan->rs_cblock; /* current page */
-			page = BufferGetPage(scan->rs_cbuf);
-			TestForOldSnapshot(scan->rs_base.rs_snapshot, scan->rs_base.rs_rd, page);
-			lines = scan->rs_ntuples;
-			lineindex = scan->rs_cindex - 1;
-		}
-		/* block and lineindex now reference the previous visible tid */
-
-		linesleft = lineindex + 1;
+			linesleft = scan->rs_cindex;
 	}
+	/*
+	* block and lineindex now reference the next or previous visible tid
+	*/
+
 
 	/*
 	 * advance the scan until we find a qualifying tuple or run out of stuff
@@ -1066,10 +1005,9 @@ heapgettup_pagemode(HeapScanDesc scan,
 
 		page = BufferGetPage(scan->rs_cbuf);
 		TestForOldSnapshot(scan->rs_base.rs_snapshot, scan->rs_base.rs_rd, page);
-		lines = scan->rs_ntuples;
-		linesleft = lines;
+		linesleft = scan->rs_ntuples;
 		if (backward)
-			lineindex = lines - 1;
+			lineindex = linesleft - 1;
 		else
 			lineindex = 0;
 	}
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 810baaf9d0..8db8ae7624 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -71,8 +71,12 @@ typedef struct HeapScanDescData
 	 */
 	ParallelBlockTableScanWorkerData *rs_parallelworkerdata;
 
+	/*
+	 * current tuple's index in vistuples or current lineoff in page
+	 */
+	int			rs_cindex;
+
 	/* these fields only used in page-at-a-time mode and for bitmap scans */
-	int			rs_cindex;		/* current tuple's index in vistuples */
 	int			rs_ntuples;		/* number of visible tuples on page */
 	OffsetNumber rs_vistuples[MaxHeapTuplesPerPage];	/* their offsets */
 }			HeapScanDescData;
-- 
2.34.1