On 05/05/2019 01:38, Peter Geoghegan wrote:
On Fri, Mar 1, 2019 at 3:59 PM Peter Geoghegan <p...@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
