From 92a085925e84cd6c34c59861f2775e53a1048665 Mon Sep 17 00:00:00 2001 From: Pavel Borisov 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