Re: Improve search for missing parent downlinks in amcheck

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