Re: amcheck: add index-all-keys-match verification for B-Tree - Mailing list pgsql-hackers

From Zsolt Parragi
Subject Re: amcheck: add index-all-keys-match verification for B-Tree
Date
Msg-id CAN4CZFNCyEOC8exfEn9SVV=uhzUA4KQYcVRUea2E0ts-rvXg3w@mail.gmail.com
Whole thread
In response to amcheck: add index-all-keys-match verification for B-Tree  (Andrey Borodin <x4mmm@yandex-team.ru>)
Responses Re: amcheck: add index-all-keys-match verification for B-Tree
List pgsql-hackers
Hello

+ /* Only verify tuples pointing to visible heap rows */
+ if (!heap_entry_is_visible(state, tid))
+ return;
...
+ slot = table_slot_create(state->heaprel, NULL);
+ found = table_tuple_fetch_row_version(state->heaprel, tid,
+   state->snapshot, slot);
+ if (!found)
+ {

This seems like a duplication, heap_entry_is_visible does the same
thing and returns found.

This also means that the if (!found) block should be unreachable?

Wouldn't it be simpler to remove the is_visible check completely and
use an if(found) later?

+ indexinfo = state->indexinfo;
+ estate = CreateExecutorState();
+ GetPerTupleExprContext(estate)->ecxt_scantuple = slot;
+ FormIndexDatum(indexinfo, slot, estate, values, isnull);
+ FreeExecutorState(estate);

This doesn't need the same cleanup code as heapam.c:1754 and :1997?
Seems like state remains, so we have dangling pointers there and could
crash later.

/* These may have been pointing to the now-gone estate */
indexInfo->ii_ExpressionsState = NIL;
indexInfo->ii_PredicateState = NULL;



pgsql-hackers by date:

Previous
From: Ilia Evdokimov
Date:
Subject: Re: Hash-based MCV matching for large IN-lists
Next
From: Masahiko Sawada
Date:
Subject: Re: POC: Parallel processing of indexes in autovacuum