Re: [PATCH] Improve amcheck to also check UNIQUE constraint in btree index.

On Mon, Feb 8, 2021 at 2:46 AM Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com> wrote:
> Caveat: if the first entry on the next index page has a key equal to the key on a previous page AND all heap tid's corresponding to this entry are invisible, currently cross-page check can not detect unique constraint violation between previous index page entry and 2nd, 3d and next current index page entries. In this case, there would be a message that recommends doing VACUUM to remove the invisible entries from the index and repeat the check. (Generally, it is recommended to do vacuum before the check, but for the testing purpose I'd recommend turning it off to check the detection of visible-invisible-visible duplicates scenarios)

It's rather unlikely that equal values in a unique index will end up
on different leaf pages. It's really rare, in fact. This following
comment block from nbtinsert.c (which appears right before we call
_bt_check_unique()) explains why this is:

* It might be necessary to check a page to the right in _bt_check_unique,
* though that should be very rare. In practice the first page the value ...

You're going to have to "couple" buffer locks in the style of
_bt_check_unique() (as well as keeping a buffer lock on "the first
leaf page a duplicate might be on" throughout) if you need the test to
work reliably. But why bother with that? The tool doesn't have to be
100% perfect at detecting corruption (nothing can be), and it's rather
unlikely that it will matter for this test. A simple test that doesn't
handle cross-page duplicates is still going to be very effective.

I don't think that it's acceptable for your new check to raise a
WARNING instead of an ERROR. I especially don't like that the new
unique checking functionality merely warns that the index *might* be
corrupt. False positives are always unacceptable within amcheck, and I
think that this is a false positive.

--
Peter Geoghegan