From: | Peter Geoghegan <pg(at)bowt(dot)ie> |
---|---|
To: | Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru> |
Cc: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Improve search for missing parent downlinks in amcheck |
Date: | 2020-03-11 04:19:31 |
Message-ID: | CAH2-Wznn8jg6Ny1gfGfeppQ5_tvVCTRJXLzZ9v4rPuE3YG+46A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Mar 10, 2020 at 8:30 AM Alexander Korotkov
<a(dot)korotkov(at)postgrespro(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Andy Fan | 2020-03-11 04:23:30 | Re: [PATCH] Erase the distinctClause if the result is unique by definition |
Previous Message | Michael Paquier | 2020-03-11 04:10:02 | Re: Add an optional timeout clause to isolationtester step. |