Re: amcheck: add index-all-keys-match verification for B-Tree

From: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>
To: Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: amcheck: add index-all-keys-match verification for B-Tree
Date: 2026-03-05 06:00:24
Message-ID: 35809064-01A3-4449-B393-0F71D4C18F7D@yandex-team.ru
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 3 Mar 2026, at 04:19, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> 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.

Attachment Content-Type Size
v4-0001-amcheck-add-indexallkeysmatch-verification-for-B-.patch application/octet-stream 28.1 KB
v4-0005-amcheck-docs-note-two-Bloom-filters-memory-usage-.patch application/octet-stream 1.7 KB
v4-0004-amcheck-address-review-fix-Bloom-filter-comment-a.patch application/octet-stream 2.8 KB
v4-0003-Document-indexallkeysmatch-parameter-for-amcheck-.patch application/octet-stream 5.8 KB
v4-0002-amcheck-report-corruption-when-index-points-to-no.patch application/octet-stream 2.7 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Joel Jacobson 2026-03-05 06:17:29 Re: [BUG?] estimate_hash_bucket_stats uses wrong ndistinct for avgfreq
Previous Message Chao Li 2026-03-05 05:58:12 Re: [BUG + PATCH] DSA pagemap out-of-bounds in make_new_segment odd-sized path