On Tue, Mar 10, 2020 at 8:30 AM Alexander Korotkov <a.korot...@postgrespro.ru> wrote: > Yes, current example looks confusing in this aspect. But your comment > spotted to me an algorithmic issue. We don't match highkey of > leftmost child against parent pivot key. But we can. The "if > (!BlockNumberIsValid(blkno))" branch survived from the patch version > when we didn't match high keys. I've revised it. Now we enter the > loop even for leftmost page on child level and match high key for that > page.
Great. That looks better. > > BTW, a P_LEFTMOST() assertion at the beginning of > > bt_child_highkey_check() would make this easier to follow. > > Yes, but why should it be an assert? We can imagine corruption, when > there is left sibling of first child of leftmost target. I agree. I would only make it an assertion when it concerns an implementation detail of amcheck, but that doesn't apply here. > Thank you. I'd like to have another feedback from you assuming there > are logic changes. This looks committable. I only noticed one thing: The comments above bt_target_page_check() need to be updated to reflect the new check, which no longer has anything to do with "heapallindexed = true". -- Peter Geoghegan
