amcheck: Refactoring the storage of the last visible entry

This commit introduces a new data structure BtreeLastVisibleEntry comprising
information about the last visible heap entry with the current value of key.
Usage of this data structure allows us to avoid passing all this information
as individual function arguments.

Reported-by: Alexander Korotkov
Discussion: https://www.postgresql.org/message-id/CAPpHfdsVbB9ToriaB1UHuOKwjKxiZmTFQcEF%3DjuzzC_nby31uA%40mail.gmail.com
Author: Pavel Borisov, Alexander Korotkov

diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c
index 70f65b645a609877241c274864b050c0ead6a493..187b95b2e7d57b23a301e7edb1d5b0f72045b7bc 100644 (file)
--- a/contrib/amcheck/verify_nbtree.c
+++ b/contrib/amcheck/verify_nbtree.c
@@ -145,6 +145,19 @@ typedef struct BtreeLevel
    bool        istruerootlevel;
 } BtreeLevel;
 
+/*
+ * Information about the last visible entry with current B-tree key.  Used
+ * for validation of the unique constraint.
+ */
+typedef struct BtreeLastVisibleEntry
+{
+   BlockNumber blkno;          /* Index block */
+   OffsetNumber offset;        /* Offset on index block */
+   int         postingIndex;   /* Number in the posting list (-1 for
+                                * non-deduplicated tuples) */
+   ItemPointer tid;            /* Heap tid */
+} BtreeLastVisibleEntry;
+
 PG_FUNCTION_INFO_V1(bt_index_check);
 PG_FUNCTION_INFO_V1(bt_index_parent_check);
 
@@ -165,17 +178,14 @@ 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,
+                               BtreeLastVisibleEntry *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);
+                                 BlockNumber targetblock, OffsetNumber offset,
+                                 BtreeLastVisibleEntry *lVis);
 static void bt_target_page_check(BtreeCheckState *state);
 static BTScanInsert bt_right_page_check_scankey(BtreeCheckState *state,
                                                OffsetNumber *rightfirstoffset);
@@ -997,8 +1007,7 @@ heap_entry_is_visible(BtreeCheckState *state, ItemPointer tid)
  */
 static void
 bt_report_duplicate(BtreeCheckState *state,
-                   ItemPointer tid, BlockNumber block, OffsetNumber offset,
-                   int posting,
+                   BtreeLastVisibleEntry *lVis,
                    ItemPointer nexttid, BlockNumber nblock, OffsetNumber noffset,
                    int nposting)
 {
@@ -1010,18 +1019,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->blkno, lVis->offset);
 
-   if (nblock != block || noffset != offset)
+   if (nblock != lVis->blkno || noffset != lVis->offset)
        nitid = psprintf(" tid=(%u,%u)", nblock, noffset);
 
-   if (posting >= 0)
-       pposting = psprintf(" posting %u", posting);
+   if (lVis->postingIndex >= 0)
+       pposting = psprintf(" posting %u", lVis->postingIndex);
 
    if (nposting >= 0)
        pnposting = psprintf(" posting %u", nposting);
@@ -1038,9 +1047,8 @@ 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,
+                     BtreeLastVisibleEntry *lVis)
 {
    ItemPointer tid;
    bool        has_visible_entry = false;
@@ -1049,7 +1057,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 +1067,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 +1080,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->blkno != targetblock && ItemPointerIsValid(lVis->tid))
                    return;
 
-               *lVis_i = i;
-               *lVis_tid = tid;
-               *lVis_offset = offset;
-               *lVis_block = targetblock;
+               lVis->blkno = targetblock;
+               lVis->offset = offset;
+               lVis->postingIndex = i;
+               lVis->tid = tid;
            }
        }
    }
@@ -1087,7 +1094,7 @@ bt_entry_unique_check(BtreeCheckState *state, IndexTuple itup,
    /*
     * Current tuple has no posting list. If TID is visible save info about it
     * for the next comparisons in the loop in bt_target_page_check(). Report
-    * duplicate if lVis_tid is already valid.
+    * duplicate if lVis->tid is already valid.
     */
    else
    {
@@ -1095,37 +1102,38 @@ 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->blkno = targetblock;
+           lVis->offset = offset;
+           lVis->tid = tid;
+           lVis->postingIndex = -1;
        }
    }
 
-   if (!has_visible_entry && *lVis_block != InvalidBlockNumber &&
-       *lVis_block != targetblock)
+   if (!has_visible_entry &&
+       lVis->blkno != InvalidBlockNumber &&
+       lVis->blkno != targetblock)
    {
        char       *posting = "";
 
-       if (*lVis_i >= 0)
-           posting = psprintf(" posting %u", *lVis_i);
+       if (lVis->postingIndex >= 0)
+           posting = psprintf(" posting %u", lVis->postingIndex);
        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->blkno, lVis->offset, posting,
+                          ItemPointerGetBlockNumberNoCheck(lVis->tid),
+                          ItemPointerGetOffsetNumberNoCheck(lVis->tid)),
                 errhint("VACUUM the table and repeat the check.")));
    }
 }
@@ -1372,12 +1380,8 @@ bt_target_page_check(BtreeCheckState *state)
    OffsetNumber max;
    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;
+   /* Last visible entry info for checking indexes with unique constraint */
+   BtreeLastVisibleEntry lVis = {InvalidBlockNumber, InvalidOffsetNumber, -1, NULL};
 
    topaque = BTPageGetOpaque(state->target);
    max = PageGetMaxOffsetNumber(state->target);
@@ -1776,8 +1780,7 @@ bt_target_page_check(BtreeCheckState *state)
        if (state->checkunique && state->indexinfo->ii_Unique &&
            P_ISLEAF(topaque) && !skey->anynullkeys)
            bt_entry_unique_check(state, itup, state->targetblock, offset,
-                                 &lVis_i, &lVis_tid, &lVis_offset,
-                                 &lVis_block);
+                                 &lVis);
 
        if (state->checkunique && state->indexinfo->ii_Unique &&
            P_ISLEAF(topaque) && OffsetNumberNext(offset) <= max)
@@ -1800,10 +1803,10 @@ 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.blkno = InvalidBlockNumber;
+               lVis.offset = InvalidOffsetNumber;
+               lVis.postingIndex = -1;
+               lVis.tid = NULL;
            }
            skey->scantid = scantid;    /* Restore saved scan key state */
        }
@@ -1902,9 +1905,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);
                }
            }
        }

diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 46a84c5714abcd309336932aa23b130e2036e8af..d427a1c16a504142fb2202eb5c2857ad4697b34f 100644 (file)
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -322,6 +322,7 @@ BrinStatsData
 BrinTuple
 BrinValues
 BtreeCheckState
+BtreeLastVisibleEntry
 BtreeLevel
 Bucket
 BufFile