v1-0002-Amcheck-code-refactoring.patch

From 92a085925e84cd6c34c59861f2775e53a1048665 Mon Sep 17 00:00:00 2001
From: Pavel Borisov <pashkin.elfe@gmail.com>
Date: Thu, 25 Apr 2024 14:07:45 +0400
Subject: [PATCH v1 2/5] Amcheck: code refactoring

Use structure to store and transfer info about last visible heap entry
among the equal index/posting list entries.

Reported-by: Alexander Korotkov
Discussion: https://www.postgresql.org/message-id/CAPpHfdsVbB9ToriaB1UHuOKwjKxiZmTFQcEF%3DjuzzC_nby31uA%40mail.gmail.com
---
 contrib/amcheck/verify_nbtree.c | 112 ++++++++++++++------------------
 1 file changed, 49 insertions(+), 63 deletions(-)

diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c
index ae8012f15b..66f2e619a8 100644
--- a/contrib/amcheck/verify_nbtree.c
+++ b/contrib/amcheck/verify_nbtree.c
@@ -145,6 +145,15 @@ typedef struct BtreeLevel
 	bool		istruerootlevel;
 } BtreeLevel;
 
+/* Info for last visible entry for checking unique constraint */
+typedef struct lVisInfo
+{
+	ItemPointer tid; 	/* Heap tid */
+	BlockNumber block;	/* Index block */
+	OffsetNumber offset;	/* Offset on index block */
+	int i;			/* Number in posting list. (-1 for non-deduplicated) */
+} lVisInfo;
+
 PG_FUNCTION_INFO_V1(bt_index_check);
 PG_FUNCTION_INFO_V1(bt_index_parent_check);
 
@@ -165,17 +174,13 @@ static void bt_recheck_sibling_links(BtreeCheckState *state,
 									 BlockNumber btpo_prev_from_target,
 									 BlockNumber leftcurrent);
 static bool heap_entry_is_visible(BtreeCheckState *state, ItemPointer tid);
-static void bt_report_duplicate(BtreeCheckState *state, ItemPointer tid,
-								BlockNumber block, OffsetNumber offset,
-								int posting, ItemPointer nexttid,
+static void bt_report_duplicate(BtreeCheckState *state, lVisInfo *lVis,
+								ItemPointer nexttid,
 								BlockNumber nblock, OffsetNumber noffset,
 								int nposting);
 static void bt_entry_unique_check(BtreeCheckState *state, IndexTuple itup,
 								  BlockNumber targetblock,
-								  OffsetNumber offset, int *lVis_i,
-								  ItemPointer *lVis_tid,
-								  OffsetNumber *lVis_offset,
-								  BlockNumber *lVis_block);
+								  OffsetNumber offset, lVisInfo *lVis);
 static void bt_target_page_check(BtreeCheckState *state);
 static BTScanInsert bt_right_page_check_scankey(BtreeCheckState *state,
 												OffsetNumber *rightfirstoffset);
@@ -997,8 +1002,7 @@ heap_entry_is_visible(BtreeCheckState *state, ItemPointer tid)
  */
 static void
 bt_report_duplicate(BtreeCheckState *state,
-					ItemPointer tid, BlockNumber block, OffsetNumber offset,
-					int posting,
+					lVisInfo *lVis,
 					ItemPointer nexttid, BlockNumber nblock, OffsetNumber noffset,
 					int nposting)
 {
@@ -1010,18 +1014,18 @@ bt_report_duplicate(BtreeCheckState *state,
 			   *pnposting = "";
 
 	htid = psprintf("tid=(%u,%u)",
-					ItemPointerGetBlockNumberNoCheck(tid),
-					ItemPointerGetOffsetNumberNoCheck(tid));
+					ItemPointerGetBlockNumberNoCheck(lVis->tid),
+					ItemPointerGetOffsetNumberNoCheck(lVis->tid));
 	nhtid = psprintf("tid=(%u,%u)",
 					 ItemPointerGetBlockNumberNoCheck(nexttid),
 					 ItemPointerGetOffsetNumberNoCheck(nexttid));
-	itid = psprintf("tid=(%u,%u)", block, offset);
+	itid = psprintf("tid=(%u,%u)", lVis->block, lVis->offset);
 
-	if (nblock != block || noffset != offset)
+	if (nblock != lVis->block || noffset != lVis->offset)
 		nitid = psprintf(" tid=(%u,%u)", nblock, noffset);
 
-	if (posting >= 0)
-		pposting = psprintf(" posting %u", posting);
+	if (lVis->i >= 0)
+		pposting = psprintf(" posting %u", lVis->i);
 
 	if (nposting >= 0)
 		pnposting = psprintf(" posting %u", nposting);
@@ -1038,9 +1042,7 @@ bt_report_duplicate(BtreeCheckState *state,
 /* Check if current nbtree leaf entry complies with UNIQUE constraint */
 static void
 bt_entry_unique_check(BtreeCheckState *state, IndexTuple itup,
-					  BlockNumber targetblock, OffsetNumber offset, int *lVis_i,
-					  ItemPointer *lVis_tid, OffsetNumber *lVis_offset,
-					  BlockNumber *lVis_block)
+					  BlockNumber targetblock, OffsetNumber offset, lVisInfo *lVis)
 {
 	ItemPointer tid;
 	bool		has_visible_entry = false;
@@ -1049,7 +1051,7 @@ bt_entry_unique_check(BtreeCheckState *state, IndexTuple itup,
 
 	/*
 	 * Current tuple has posting list. Report duplicate if TID of any posting
-	 * list entry is visible and lVis_tid is valid.
+	 * list entry is visible and lVis->tid is valid.
 	 */
 	if (BTreeTupleIsPosting(itup))
 	{
@@ -1059,11 +1061,10 @@ bt_entry_unique_check(BtreeCheckState *state, IndexTuple itup,
 			if (heap_entry_is_visible(state, tid))
 			{
 				has_visible_entry = true;
-				if (ItemPointerIsValid(*lVis_tid))
+				if (ItemPointerIsValid(lVis->tid))
 				{
 					bt_report_duplicate(state,
-										*lVis_tid, *lVis_block,
-										*lVis_offset, *lVis_i,
+										lVis,
 										tid, targetblock,
 										offset, i);
 				}
@@ -1073,13 +1074,13 @@ bt_entry_unique_check(BtreeCheckState *state, IndexTuple itup,
 				 * between the posting list entries of the first tuple on the
 				 * page after cross-page check.
 				 */
-				if (*lVis_block != targetblock && ItemPointerIsValid(*lVis_tid))
+				if (lVis->block != targetblock && ItemPointerIsValid(lVis->tid))
 					return;
 
-				*lVis_i = i;
-				*lVis_tid = tid;
-				*lVis_offset = offset;
-				*lVis_block = targetblock;
+				lVis->i = i;
+				lVis->tid = tid;
+				lVis->offset = offset;
+				lVis->block = targetblock;
 			}
 		}
 	}
@@ -1095,37 +1096,36 @@ bt_entry_unique_check(BtreeCheckState *state, IndexTuple itup,
 		if (heap_entry_is_visible(state, tid))
 		{
 			has_visible_entry = true;
-			if (ItemPointerIsValid(*lVis_tid))
+			if (ItemPointerIsValid(lVis->tid))
 			{
 				bt_report_duplicate(state,
-									*lVis_tid, *lVis_block,
-									*lVis_offset, *lVis_i,
+									lVis,
 									tid, targetblock,
 									offset, -1);
 			}
-			*lVis_i = -1;
-			*lVis_tid = tid;
-			*lVis_offset = offset;
-			*lVis_block = targetblock;
+			lVis->i = -1;
+			lVis->tid = tid;
+			lVis->offset = offset;
+			lVis->block = targetblock;
 		}
 	}
 
-	if (!has_visible_entry && *lVis_block != InvalidBlockNumber &&
-		*lVis_block != targetblock)
+	if (!has_visible_entry && lVis->block != InvalidBlockNumber &&
+		lVis->block != targetblock)
 	{
 		char	   *posting = "";
 
-		if (*lVis_i >= 0)
-			posting = psprintf(" posting %u", *lVis_i);
+		if (lVis->i >= 0)
+			posting = psprintf(" posting %u", lVis->i);
 		ereport(DEBUG1,
 				(errcode(ERRCODE_NO_DATA),
 				 errmsg("index uniqueness can not be checked for index tid=(%u,%u) in index \"%s\"",
 						targetblock, offset,
 						RelationGetRelationName(state->rel)),
 				 errdetail("It doesn't have visible heap tids and key is equal to the tid=(%u,%u)%s (points to heap tid=(%u,%u)).",
-						   *lVis_block, *lVis_offset, posting,
-						   ItemPointerGetBlockNumberNoCheck(*lVis_tid),
-						   ItemPointerGetOffsetNumberNoCheck(*lVis_tid)),
+						   lVis->block, lVis->offset, posting,
+						   ItemPointerGetBlockNumberNoCheck(lVis->tid),
+						   ItemPointerGetOffsetNumberNoCheck(lVis->tid)),
 				 errhint("VACUUM the table and repeat the check.")));
 	}
 }
@@ -1373,11 +1373,7 @@ bt_target_page_check(BtreeCheckState *state)
 	BTPageOpaque topaque;
 
 	/* last visible entry info for checking indexes with unique constraint */
-	int			lVis_i = -1;	/* the position of last visible item for
-								 * posting tuple. for non-posting tuple (-1) */
-	ItemPointer lVis_tid = NULL;
-	BlockNumber lVis_block = InvalidBlockNumber;
-	OffsetNumber lVis_offset = InvalidOffsetNumber;
+	lVisInfo 	lVis = {NULL, InvalidBlockNumber, InvalidOffsetNumber, -1};
 
 	topaque = BTPageGetOpaque(state->target);
 	max = PageGetMaxOffsetNumber(state->target);
@@ -1776,11 +1772,9 @@ bt_target_page_check(BtreeCheckState *state)
 		 */
 		if (state->checkunique && state->indexinfo->ii_Unique &&
 			P_ISLEAF(topaque) && !skey->anynullkeys &&
-			(BTreeTupleIsPosting(itup) || ItemPointerIsValid(lVis_tid)))
+			(BTreeTupleIsPosting(itup) || ItemPointerIsValid(lVis.tid)))
 		{
-			bt_entry_unique_check(state, itup, state->targetblock, offset,
-								  &lVis_i, &lVis_tid, &lVis_offset,
-								  &lVis_block);
+			bt_entry_unique_check(state, itup, state->targetblock, offset, &lVis);
 			unique_checked = true;
 		}
 
@@ -1805,17 +1799,13 @@ bt_target_page_check(BtreeCheckState *state)
 			if (_bt_compare(state->rel, skey, state->target,
 							OffsetNumberNext(offset)) != 0 || skey->anynullkeys)
 			{
-				lVis_i = -1;
-				lVis_tid = NULL;
-				lVis_block = InvalidBlockNumber;
-				lVis_offset = InvalidOffsetNumber;
+				lVis = (lVisInfo) {NULL, InvalidBlockNumber, InvalidOffsetNumber, -1};
 			}
 			else if (!unique_checked)
 			{
-				bt_entry_unique_check(state, itup, state->targetblock, offset,
-									  &lVis_i, &lVis_tid, &lVis_offset,
-									  &lVis_block);
+				bt_entry_unique_check(state, itup, state->targetblock, offset, &lVis);
 			}
+
 			skey->scantid = scantid;	/* Restore saved scan key state */
 		}
 
@@ -1901,9 +1891,7 @@ bt_target_page_check(BtreeCheckState *state)
 				if (_bt_compare(state->rel, rightkey, state->target, max) == 0 && !rightkey->anynullkeys)
 				{
 					if (!unique_checked)
-						bt_entry_unique_check(state, itup, state->targetblock, offset,
-											  &lVis_i, &lVis_tid, &lVis_offset,
-											  &lVis_block);
+						bt_entry_unique_check(state, itup, state->targetblock, offset, &lVis);
 
 					elog(DEBUG2, "cross page equal keys");
 					state->target = palloc_btree_page(state,
@@ -1918,9 +1906,7 @@ bt_target_page_check(BtreeCheckState *state)
 												  rightfirstoffset);
 					itup = (IndexTuple) PageGetItem(state->target, itemid);
 
-					bt_entry_unique_check(state, itup, rightblock_number, rightfirstoffset,
-										  &lVis_i, &lVis_tid, &lVis_offset,
-										  &lVis_block);
+					bt_entry_unique_check(state, itup, rightblock_number, rightfirstoffset, &lVis);
 				}
 			}
 		}
-- 
2.34.1