On 05/05/2019 01:38, Peter Geoghegan wrote:
> On Fri, Mar 1, 2019 at 3:59 PM Peter Geoghegan <pg@bowt.ie> wrote:
>> /*
>> * Perform the same check on this internal level that
>> * _bt_mark_page_halfdead performed on the leaf level.
>> */
>> if (_bt_is_page_halfdead(rel, *rightsib))
>
>> I thought that internal pages were never half-dead after Postgres 9.4.
>> If that happens, then the check within _bt_pagedel() will throw an
>> ERRCODE_INDEX_CORRUPTED error, and tell the DBA to REINDEX. Shouldn't
>> this internal level _bt_is_page_halfdead() check contain a "can't
>> happen" error, or even a simple assertion?
>
> I think that we should get rid of this code on HEAD shortly, because
> it's effectively dead code. You don't have to know anything about
> B-Trees to see why this must be true: VACUUM is specifically checking
> if an internal page is half-dead here, even though it's already
> treating half-dead internal pages as ipso facto corrupt in higher
> level code (it's the first thing we check in _bt_pagedel()). This is
> clearly contradictory. If there is a half-dead internal page, then
> there is no danger of VACUUM not complaining loudly that you need to
> REINDEX. This has been true since the page deletion overhaul that went
> into 9.4.
I don't understand that reasoning. Yes, _bt_pagedel() will complain if
it finds a half-dead internal page. But how does that mean that
_bt_lock_branch_parent() can't encounter one?
- Heikki