v6-0003-Streamline-heapgettup-for-refactor.patch

From 2ceae3bb5e615b5510efa6ce55ffebc0dec24ebd Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Mon, 9 Jan 2023 18:48:49 -0500
Subject: [PATCH v6 3/6] Streamline heapgettup*() for refactor

Backward and forward scan share much of the page acquisition setup code.
They differ only in calculating where in the page the current tuple is.
Consolidate the common page acquisition code to pave the way for helper
functions.
---
 src/backend/access/heap/heapam.c | 185 ++++++++++---------------------
 src/include/access/heapam.h      |   6 +-
 2 files changed, 64 insertions(+), 127 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 8ec1df0426..a5c0ada4cd 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -610,12 +610,10 @@ heapgettup(HeapScanDesc scan,
 		   ScanKey key)
 {
 	HeapTuple	tuple = &(scan->rs_ctup);
-	Snapshot	snapshot = scan->rs_base.rs_snapshot;
 	bool		backward = ScanDirectionIsBackward(dir);
 	BlockNumber block;
 	bool		finished;
 	Page		page;
-	int			lines;
 	OffsetNumber lineoff;
 	int			linesleft;
 	ItemId		lpp;
@@ -626,89 +624,61 @@ heapgettup(HeapScanDesc scan,
 		return;
 	}
 
-	/*
-	 * calculate next starting lineoff, given scan direction
-	 */
-	if (ScanDirectionIsForward(dir))
+	if (!scan->rs_inited)
 	{
-		if (!scan->rs_inited)
-		{
-			block = heapgettup_initial_block(scan, dir);
+		block = heapgettup_initial_block(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 */
+		TestForOldSnapshot(scan->rs_base.rs_snapshot, scan->rs_base.rs_rd,
+						   page);
 
-		linesleft = lines - lineoff + 1;
+		linesleft = PageGetMaxOffsetNumber(page) - FirstOffsetNumber + 1;
+
+		if (ScanDirectionIsForward(dir))
+			lineoff = FirstOffsetNumber;
+		else
+			lineoff = (OffsetNumber) linesleft;
 	}
 	else
 	{
-		Assert(backward);
-
-		if (!scan->rs_inited)
-		{
-			block = heapgettup_initial_block(scan, dir);
-
-			if (block == InvalidBlockNumber)
-			{
-				Assert(!BufferIsValid(scan->rs_cbuf));
-				tuple->t_data = NULL;
-				return;
-			}
+		block = scan->rs_cblock;
 
-			heapgetpage((TableScanDesc) scan, block);
-			LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
+		LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
+		page = BufferGetPage(scan->rs_cbuf);
+		TestForOldSnapshot(scan->rs_base.rs_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);
+			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));
+			linesleft = lineoff;
 		}
-		/* block and lineoff now reference the physically previous tid */
-
-		linesleft = lineoff;
 	}
 
+
+
 	/*
 	 * advance the scan until we find a qualifying tuple or run out of stuff
 	 * to scan
@@ -737,12 +707,12 @@ heapgettup(HeapScanDesc scan,
 				 * if current tuple qualifies, return it.
 				 */
 				valid = HeapTupleSatisfiesVisibility(tuple,
-													 snapshot,
+													 scan->rs_base.rs_snapshot,
 													 scan->rs_cbuf);
 
 				HeapCheckForSerializableConflictOut(valid, scan->rs_base.rs_rd,
 													tuple, scan->rs_cbuf,
-													snapshot);
+													scan->rs_base.rs_snapshot);
 
 				if (valid && key != NULL)
 					valid = HeapKeyTest(tuple, RelationGetDescr(scan->rs_base.rs_rd),
@@ -751,6 +721,7 @@ heapgettup(HeapScanDesc scan,
 				if (valid)
 				{
 					LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
+					scan->rs_cindex = lineoff;
 					return;
 				}
 			}
@@ -842,13 +813,14 @@ heapgettup(HeapScanDesc scan,
 		LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
 
 		page = BufferGetPage(scan->rs_cbuf);
-		TestForOldSnapshot(snapshot, scan->rs_base.rs_rd, page);
-		lines = PageGetMaxOffsetNumber(page);
-		linesleft = lines;
+
+		TestForOldSnapshot(scan->rs_base.rs_snapshot,
+						   scan->rs_base.rs_rd, page);
+		linesleft = PageGetMaxOffsetNumber(page);
 		if (backward)
 		{
-			lineoff = lines;
-			lpp = PageGetItemId(page, lines);
+			lineoff = linesleft;
+			lpp = PageGetItemId(page, linesleft);
 		}
 		else
 		{
@@ -882,7 +854,6 @@ heapgettup_pagemode(HeapScanDesc scan,
 	BlockNumber block;
 	bool		finished;
 	Page		page;
-	int			lines;
 	int			lineindex;
 	OffsetNumber lineoff;
 	int			linesleft;
@@ -894,75 +865,38 @@ 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_block(scan, dir);
+		block = heapgettup_initial_block(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_block(scan, dir);
-
-			if (block == InvalidBlockNumber)
-			{
-				Assert(!BufferIsValid(scan->rs_cbuf));
-				tuple->t_data = NULL;
-				return;
-			}
+		/* continue from previously returned page/tuple */
+		block = scan->rs_cblock;
+		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;
 	}
 
+
 	/*
 	 * advance the scan until we find a qualifying tuple or run out of stuff
 	 * to scan
@@ -1075,10 +1009,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 417108f1e0..5a4df34e48 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -72,8 +72,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