v7-0006-Refactor-heapgettup-and-heapgettup_pagemode.patch

From 0dafb73195cb624c79ec6bf62e00b71117f36b5a Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Tue, 10 Jan 2023 14:30:21 -0500
Subject: [PATCH v7 6/6] Refactor heapgettup() and heapgettup_pagemode()

---
 src/backend/access/heap/heapam.c | 235 +++++++++++--------------------
 1 file changed, 81 insertions(+), 154 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 4ff3dc7138..e80ea377a2 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -736,12 +736,10 @@ heapgettup(HeapScanDesc scan,
 		   ScanKey key)
 {
 	HeapTuple	tuple = &(scan->rs_ctup);
-	bool		backward = ScanDirectionIsBackward(dir);
 	BlockNumber block;
 	Page		page;
 	OffsetNumber lineoff;
 	int			linesleft;
-	ItemId		lpp;
 
 	Assert(dir == ForwardScanDirection ||
 		   dir == BackwardScanDirection);
@@ -755,17 +753,7 @@ heapgettup(HeapScanDesc scan,
 		 * happen if the table is empty or if the other workers in a parallel
 		 * scan have already finished the scan.
 		 */
-		if (block == InvalidBlockNumber)
-		{
-			Assert(!BufferIsValid(scan->rs_cbuf));
-			tuple->t_data = NULL;
-			return;
-		}
-
-		heapgetpage((TableScanDesc) scan, block);
-
-		LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
-		page = heapgettup_start_page(scan, dir, &linesleft, &lineoff);
+		Assert(block != InvalidBlockNumber || !BufferIsValid(scan->rs_cbuf));
 	}
 	else
 	{
@@ -773,6 +761,7 @@ heapgettup(HeapScanDesc scan,
 
 		LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
 		page = heapgettup_continue_page(scan, dir, &linesleft, &lineoff);
+		goto continue_page;
 	}
 
 
@@ -781,9 +770,13 @@ heapgettup(HeapScanDesc scan,
 	 * advance the scan until we find a qualifying tuple or run out of stuff
 	 * to scan
 	 */
-	lpp = PageGetItemId(page, lineoff);
-	for (;;)
+	while (block != InvalidBlockNumber)
 	{
+		heapgetpage((TableScanDesc) scan, block);
+		LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
+		page = heapgettup_start_page(scan, dir, &linesleft, &lineoff);
+continue_page:
+
 		/*
 		 * Only continue scanning the page while we have lines left.
 		 *
@@ -791,53 +784,40 @@ heapgettup(HeapScanDesc scan,
 		 * PageGetMaxOffsetNumber(); both for forward scans when we resume the
 		 * table scan, and for when we start scanning a new page.
 		 */
-		while (linesleft > 0)
+		for (; linesleft > 0; linesleft--, lineoff += dir)
 		{
-			if (ItemIdIsNormal(lpp))
-			{
-				bool		valid;
-
-				tuple->t_data = (HeapTupleHeader) PageGetItem(page, lpp);
-				tuple->t_len = ItemIdGetLength(lpp);
-				ItemPointerSet(&(tuple->t_self), block, lineoff);
+			bool		visible;
+			ItemId		lpp = PageGetItemId(page, lineoff);
 
-				/*
-				 * if current tuple qualifies, return it.
-				 */
-				valid = HeapTupleSatisfiesVisibility(tuple,
-													 scan->rs_base.rs_snapshot,
-													 scan->rs_cbuf);
-
-				HeapCheckForSerializableConflictOut(valid, scan->rs_base.rs_rd,
-													tuple, scan->rs_cbuf,
-													scan->rs_base.rs_snapshot);
+			if (!ItemIdIsNormal(lpp))
+				continue;
 
-				if (valid && key != NULL)
-					valid = HeapKeyTest(tuple, RelationGetDescr(scan->rs_base.rs_rd),
-										nkeys, key);
 
-				if (valid)
-				{
-					LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
-					scan->rs_cindex = lineoff;
-					return;
-				}
-			}
+			tuple->t_data = (HeapTupleHeader) PageGetItem(page, lpp);
+			tuple->t_len = ItemIdGetLength(lpp);
+			ItemPointerSet(&(tuple->t_self), scan->rs_cblock, lineoff);
 
 			/*
-			 * otherwise move to the next item on the page
+			 * if current tuple qualifies, return it.
 			 */
-			--linesleft;
-			if (backward)
-			{
-				--lpp;			/* move back in this page's ItemId array */
-				--lineoff;
-			}
-			else
-			{
-				++lpp;			/* move forward in this page's ItemId array */
-				++lineoff;
-			}
+			visible = HeapTupleSatisfiesVisibility(tuple,
+												   scan->rs_base.rs_snapshot,
+												   scan->rs_cbuf);
+
+			HeapCheckForSerializableConflictOut(visible, scan->rs_base.rs_rd,
+												tuple, scan->rs_cbuf,
+												scan->rs_base.rs_snapshot);
+
+			if (!visible)
+				continue;
+
+			if (key && !HeapKeyTest(tuple, RelationGetDescr(scan->rs_base.rs_rd),
+									nkeys, key))
+				continue;
+
+			LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
+			scan->rs_cindex = lineoff;
+			return;
 		}
 
 		/*
@@ -847,41 +827,16 @@ heapgettup(HeapScanDesc scan,
 		LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
 
 		block = heapgettup_advance_block(scan, block, dir);
+	}
 
-		/*
-		 * return NULL if we've exhausted all the pages
-		 */
-		if (block == InvalidBlockNumber)
-		{
-			if (BufferIsValid(scan->rs_cbuf))
-				ReleaseBuffer(scan->rs_cbuf);
-			scan->rs_cbuf = InvalidBuffer;
-			scan->rs_cblock = InvalidBlockNumber;
-			tuple->t_data = NULL;
-			scan->rs_inited = false;
-			return;
-		}
-
-		heapgetpage((TableScanDesc) scan, block);
-
-		LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
-
-		page = BufferGetPage(scan->rs_cbuf);
+	/* end of scan */
+	if (BufferIsValid(scan->rs_cbuf))
+		ReleaseBuffer(scan->rs_cbuf);
 
-		TestForOldSnapshot(scan->rs_base.rs_snapshot,
-						   scan->rs_base.rs_rd, page);
-		linesleft = PageGetMaxOffsetNumber(page);
-		if (backward)
-		{
-			lineoff = linesleft;
-			lpp = PageGetItemId(page, linesleft);
-		}
-		else
-		{
-			lineoff = FirstOffsetNumber;
-			lpp = PageGetItemId(page, FirstOffsetNumber);
-		}
-	}
+	scan->rs_cbuf = InvalidBuffer;
+	scan->rs_cblock = InvalidBlockNumber;
+	tuple->t_data = NULL;
+	scan->rs_inited = false;
 }
 
 /* ----------------
@@ -904,13 +859,10 @@ heapgettup_pagemode(HeapScanDesc scan,
 					ScanKey key)
 {
 	HeapTuple	tuple = &(scan->rs_ctup);
-	bool		backward = ScanDirectionIsBackward(dir);
 	BlockNumber block;
 	Page		page;
 	int			lineindex;
-	OffsetNumber lineoff;
 	int			linesleft;
-	ItemId		lpp;
 
 	Assert(dir == ForwardScanDirection ||
 		   dir == BackwardScanDirection);
@@ -924,23 +876,13 @@ heapgettup_pagemode(HeapScanDesc scan,
 		 * happen if the table is empty or if the other workers in a parallel
 		 * scan have already finished the scan.
 		 */
-		if (block == InvalidBlockNumber)
-		{
-			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);
-		linesleft = scan->rs_ntuples;
-		lineindex = ScanDirectionIsForward(dir) ? 0 : linesleft - 1;
+		Assert(block != InvalidBlockNumber || !BufferIsValid(scan->rs_cbuf));
 	}
 	else
 	{
 		/* 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);
 
@@ -949,6 +891,9 @@ heapgettup_pagemode(HeapScanDesc scan,
 			linesleft = scan->rs_ntuples - lineindex;
 		else
 			linesleft = scan->rs_cindex;
+		/* lineindex now references the next or previous visible tid */
+
+		goto continue_page;
 	}
 
 
@@ -956,75 +901,57 @@ heapgettup_pagemode(HeapScanDesc scan,
 	 * advance the scan until we find a qualifying tuple or run out of stuff
 	 * to scan
 	 */
-	for (;;)
+	while (block != InvalidBlockNumber)
 	{
-		while (linesleft > 0)
+		heapgetpage((TableScanDesc) scan, block);
+		page = BufferGetPage(scan->rs_cbuf);
+		TestForOldSnapshot(scan->rs_base.rs_snapshot, scan->rs_base.rs_rd, page);
+		linesleft = scan->rs_ntuples;
+		lineindex = ScanDirectionIsForward(dir) ? 0 : linesleft - 1;
+
+		/* lineindex now references the next or previous visible tid */
+continue_page:
+
+		for (; linesleft > 0; linesleft--, lineindex += dir)
 		{
+			ItemId		lpp;
+			OffsetNumber lineoff;
+
 			lineoff = scan->rs_vistuples[lineindex];
 			lpp = PageGetItemId(page, lineoff);
 			Assert(ItemIdIsNormal(lpp));
 
-			tuple->t_data = (HeapTupleHeader) PageGetItem(page, lpp);
+			tuple->t_data = (HeapTupleHeader) PageGetItem((Page) page, lpp);
 			tuple->t_len = ItemIdGetLength(lpp);
 			ItemPointerSet(&(tuple->t_self), block, lineoff);
 
 			/*
-			 * if current tuple qualifies, return it.
+			 * if current tuple qualifies, return it. otherwise move to the
+			 * next item on the block
 			 */
-			if (key != NULL)
-			{
-				bool		valid;
-
-				valid = HeapKeyTest(tuple, RelationGetDescr(scan->rs_base.rs_rd),
-									nkeys, key);
-				if (valid)
-				{
-					scan->rs_cindex = lineindex;
-					return;
-				}
-			}
-			else
-			{
-				scan->rs_cindex = lineindex;
-				return;
-			}
+			if (key && !HeapKeyTest(tuple, RelationGetDescr(scan->rs_base.rs_rd),
+									nkeys, key))
+				continue;
 
-			/*
-			 * otherwise move to the next item on the page
-			 */
-			--linesleft;
-			if (backward)
-				--lineindex;
-			else
-				++lineindex;
+			scan->rs_cindex = lineindex;
+			return;
 		}
 
-		block = heapgettup_advance_block(scan, block, dir);
-
 		/*
-		 * return NULL if we've exhausted all the pages
+		 * if we get here, it means we've exhausted the items on this page and
+		 * it's time to move to the next.
 		 */
-		if (block == InvalidBlockNumber)
-		{
-			if (BufferIsValid(scan->rs_cbuf))
-				ReleaseBuffer(scan->rs_cbuf);
-			scan->rs_cbuf = InvalidBuffer;
-			scan->rs_cblock = InvalidBlockNumber;
-			tuple->t_data = NULL;
-			scan->rs_inited = false;
-			return;
-		}
+		block = heapgettup_advance_block(scan, block, dir);
+	}
 
-		heapgetpage((TableScanDesc) scan, block);
+	/* end of scan */
+	if (BufferIsValid(scan->rs_cbuf))
+		ReleaseBuffer(scan->rs_cbuf);
+	scan->rs_cbuf = InvalidBuffer;
+	scan->rs_cblock = InvalidBlockNumber;
+	tuple->t_data = NULL;
+	scan->rs_inited = false;
 
-		page = BufferGetPage(scan->rs_cbuf);
-		TestForOldSnapshot(scan->rs_base.rs_snapshot, scan->rs_base.rs_rd, page);
-		linesleft = scan->rs_ntuples;
-		if (backward)
-			lineindex = linesleft - 1;
-		else
-			lineindex = 0;
-	}
 }
 
 
-- 
2.37.2