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

I did notice (I meant to point out) that I have concerns about this
> part of the new uniqueness check code:
> "
> if (P_IGNORE(topaque) || !P_ISLEAF(topaque))
> break;
> "

My concern here is with the !P_ISLEAF(topaque) test -- it shouldn't be
> required

I agree. But I didn't see the need to check uniqueness constraints
violations in internal pages. Furthermore, it doesn't mean only a violation
of constraint, but a major index corruption. I agree that checking and
reporting this type of corruption separately is a possible thing.

Separately, I dislike the way the target block changes within
> bt_target_page_check(). The general idea behind verify_nbtree.c's
> target block is that every block becomes the target exactly once, in a
> clearly defined place. All corruption (in the index structure itself)
> is formally considered to be a problem with that particular target
> block. I want to be able to clearly distinguish between the target and
> target's right sibling here, to explain my concerns, but they're kinda
> both the target, so that's a lot harder than it should be. (Admittedly
> directly blaming the target block has always been a little bit
> arbitrary, at least in certain cases, but even there it provides
> structure that makes things much easier to describe unambiguously.)
>

The possible way to load the target block only once is to get rid of the
cross-page uniqueness violation check. I introduced it to catch more
possible cases of uniqueness violations. Though they are expected to be
extremely rare, and anyway the algorithm doesn't get any warranty, just
does its best to catch what is possible. I don't object to this change.

Regards,
Pavel.