From 78d8fd0ab8ef94c9de7f3a4c8f308ce0c2cba54b Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Mon, 15 Sep 2025 17:48:38 -0400
Subject: [PATCH v15 08/23] Combine vacuum phase I VM update cases

We update the VM after phase I of vacuum -- either setting both the VM
bits when all bits are currently unset or setting just the frozen bit
when the all-visible bit is already set.

Those two cases shared much of the same code -- leading to unnecessary
duplication. This commit combines them, which is simpler and easier to
understand.
---
 src/backend/access/heap/vacuumlazy.c | 68 ++++++++--------------------
 1 file changed, 18 insertions(+), 50 deletions(-)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 308abff16ca..5a6bbbd97f2 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -2058,15 +2058,22 @@ lazy_scan_prune(LVRelState *vacrel,
 	Assert(!presult.all_frozen || presult.all_visible);
 
 	/*
-	 * Handle setting visibility map bit based on information from the VM (as
+	 * Handle setting visibility map bits based on information from the VM (as
 	 * of last heap_vac_scan_next_block() call), and from all_visible and
-	 * all_frozen variables
+	 * all_frozen variables.
 	 */
-	if (!all_visible_according_to_vm && presult.all_visible)
+	if ((presult.all_visible && !all_visible_according_to_vm) ||
+		(presult.all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer)))
 	{
 		uint8		old_vmbits;
 		uint8		flags = VISIBILITYMAP_ALL_VISIBLE;
 
+		/*
+		 * If the page is all-frozen, we can pass InvalidTransactionId as our
+		 * cutoff_xid, since a snapshotConflictHorizon sufficient to make
+		 * everything safe for REDO was logged when the page's tuples were
+		 * frozen.
+		 */
 		if (presult.all_frozen)
 		{
 			Assert(!TransactionIdIsValid(presult.vm_conflict_horizon));
@@ -2079,6 +2086,12 @@ lazy_scan_prune(LVRelState *vacrel,
 									   flags);
 
 		/*
+		 * Even if we are only setting the all-frozen bit, there is a small
+		 * chance that the VM was modified sometime between setting
+		 * all_visible_according_to_vm and checking the visibility during
+		 * pruning. Check the return value of old_vmbits to ensure the
+		 * visibility map counters used for logging are accurate.
+		 *
 		 * If the page wasn't already set all-visible and/or all-frozen in the
 		 * VM, count it as newly set for logging.
 		 */
@@ -2100,6 +2113,8 @@ lazy_scan_prune(LVRelState *vacrel,
 	}
 
 	/*
+	 * Now handle two potential corruption cases:
+	 *
 	 * As of PostgreSQL 9.2, the visibility map bit should never be set if the
 	 * page-level bit is clear.  However, it's possible that the bit got
 	 * cleared after heap_vac_scan_next_block() was called, so we must recheck
@@ -2144,53 +2159,6 @@ lazy_scan_prune(LVRelState *vacrel,
 							VISIBILITYMAP_VALID_BITS);
 	}
 
-	/*
-	 * If the all-visible page is all-frozen but not marked as such yet, mark
-	 * it as all-frozen.
-	 */
-	else if (all_visible_according_to_vm && presult.all_frozen &&
-			 !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
-	{
-		uint8		old_vmbits;
-
-		/*
-		 * Set the page all-frozen (and all-visible) in the VM.
-		 *
-		 * We can pass InvalidTransactionId as our cutoff_xid, since a
-		 * snapshotConflictHorizon sufficient to make everything safe for REDO
-		 * was logged when the page's tuples were frozen.
-		 */
-		Assert(!TransactionIdIsValid(presult.vm_conflict_horizon));
-		old_vmbits = visibilitymap_set(vacrel->rel, blkno, buf,
-									   InvalidXLogRecPtr,
-									   vmbuffer, InvalidTransactionId,
-									   VISIBILITYMAP_ALL_VISIBLE |
-									   VISIBILITYMAP_ALL_FROZEN);
-
-		/*
-		 * The page was likely already set all-visible in the VM. However,
-		 * there is a small chance that it was modified sometime between
-		 * setting all_visible_according_to_vm and checking the visibility
-		 * during pruning. Check the return value of old_vmbits anyway to
-		 * ensure the visibility map counters used for logging are accurate.
-		 */
-		if ((old_vmbits & VISIBILITYMAP_ALL_VISIBLE) == 0)
-		{
-			vacrel->vm_new_visible_pages++;
-			vacrel->vm_new_visible_frozen_pages++;
-			*vm_page_frozen = true;
-		}
-
-		/*
-		 * We already checked that the page was not set all-frozen in the VM
-		 * above, so we don't need to test the value of old_vmbits.
-		 */
-		else
-		{
-			vacrel->vm_new_frozen_pages++;
-			*vm_page_frozen = true;
-		}
-	}
 
 	return presult.ndeleted;
 }
-- 
2.43.0

