Hi Zsolt! Many thanks for the review!
> On 26 Feb 2026, at 04:18, Zsolt Parragi <[email protected]> wrote: > > 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? Yup, done so. Also thinking about it more, I decided to add more corruption checks here: non-existent tuples are also kind of corruption that must be reported. This issue addressed in patch 2 so you can verify your notes were addressed. I have a gut feeling that we might do without snapshot at all... > > + 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; Yup, fixed and added tests exercising ii_ExpressionsState. Thanks! Best regards, Andrey Borodin.
v22-0001-amcheck-add-indexallkeysmatch-verification-for-B.patch
Description: Binary data
v22-0002-amcheck-report-corruption-when-index-points-to-n.patch
Description: Binary data
