> On 3 Mar 2026, at 04:19, Zsolt Parragi <[email protected]> wrote: > > I don't see any other logic problems in the code, I only have a few > minor comments/questions: > > + * Bloom filter says (key, tid) not in heap. Follow TID to verify; this > + * amortizes random heap lookups when the filter has false negatives, or > > This comment could be a bit confusing, as bloom filters typically have > false positives, not negatives. > Maybe it would be better to phrase this somehow differently? > > Another thing is that now amcheck can create two bloom filters, > allocated at the same time, both up to maintenance_work_mem. > Isn't that a detail that should be at least mentioned somewhere? > > And in the documentation, shouldn't this new check mention something > similar to heapallindexed, which describes the check possibly missing > corruption because of the bloom filter? > > + (errcode(ERRCODE_INDEX_CORRUPTED), > + errmsg("index tuple in index \"%s\" does not match heap tuple", > + RelationGetRelationName(state->rel)), > > Shouldn't this also print out the table name?
Thanks for the review! I've addressed all these issues in patch steps 4 and 5. Other steps are intact. PFA. Best regards, Andrey Borodin.
v4-0001-amcheck-add-indexallkeysmatch-verification-for-B-.patch
Description: Binary data
v4-0005-amcheck-docs-note-two-Bloom-filters-memory-usage-.patch
Description: Binary data
v4-0004-amcheck-address-review-fix-Bloom-filter-comment-a.patch
Description: Binary data
v4-0003-Document-indexallkeysmatch-parameter-for-amcheck-.patch
Description: Binary data
v4-0002-amcheck-report-corruption-when-index-points-to-no.patch
Description: Binary data
