Re: [PATCH] Missing Assert in the code

On 2024-Nov-25, Dmitry Nikitin wrote:

> Subject: [PATCH 2/2] Add Assert to stop invalid values to pass on
>
> PageGetMaxOffsetNumber() can legitimately return zero
> (InvalidOffsetNumber) as an indication of error. However there are no
> any checks against that. As a result, for exampe, subsequent
> PageGetItemId() will be accessing an array at -1.

> @@ -236,6 +236,8 @@ getRightMostTuple(Page page)
> {
> OffsetNumber maxoff = PageGetMaxOffsetNumber(page);
>
> + Assert(OffsetNumberIsValid(maxoff));
> +
> return (IndexTuple) PageGetItem(page, PageGetItemId(page, maxoff));
> }

Hmm, I think if we believe this to be really possible, we should have an
'if/elog' test (or maybe a full ereport with ERRCODE_DATA_CORRUPTED
errcode) rather than an assertion. I think the assertion adds nothing
of value here, but maybe an 'if' test would.

Did you examine the other callers of PageGetMaxOffsetNumber()? It's a
large bunch.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
[…] indem ich in meinem Leben oft an euch gedacht, euch glücklich zu machen. Seyd es!
A menudo he pensado en vosotros, en haceros felices. ¡Sedlo, pues!
Heiligenstädter Testament, L. v. Beethoven, 1802
https://de.wikisource.org/wiki/Heiligenstädter_Testament