On Mon, Feb 8, 2021 at 2:46 AM Pavel Borisov <pashkin.e...@gmail.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
