On Tue, Mar 10, 2020 at 8:30 AM Alexander Korotkov
<a.korotkov@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