Re: Orphan page in _bt_split

On Mon, Sep 01, 2025 at 08:03:18AM +0300, Konstantin Knizhnik wrote:
> _bp_getbuf just reads page in a shared buffer. But ReadBuffer actually can
> invoke half of Postgres code (locating buffer, evicting victim, writing to
> SMGR,..., So there are a lot of places where error can be thrown (including
> even such error as statement timeout).
> And in this case current transaction will be aborted without zeroing the
> right page.
> So vacuum will be confused.
>
> It can be quite easily demonstrated by inserting error here:

Nice investigation and report, that I assume you have just guessed
from a read of the code and that there could be plenty of errors that
could happen in this code path. It indeed looks like some weak
coding assumption introduced in this code path by 9b42e713761a from
2019, going down to v11.

We could have a SQL regression test for this case, just put a
INJECTION_POINT(), then force an ERROR callback to force an incorrect
state. The test can be made cheap enough.

> This code is not changed for quite long time so I wonder why nobody noticed
> this error before?

I am ready to believe that errors are just hard to reach in this path.

> It creates copy of right page in the same way as for left (original) page.
> And updates the page only in critical section.

Paying the cost of an extra allocation for a temporary page to bypass
the problem entirely surely makes the code safer in the long run, so
as any changes made in this area would never trigger the same problem
ever again. We do that already for GIN with critical sections, as one
example.

Peter G., as the committer of 9b42e713761a, an opinion?
--
Michael