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.
