On Thu, Jun 12, 2025 at 1:16 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> Related to this, I
> realized that TidStoreSetBlockOffsets() checks if the given offset
> number is a valid OffsetNumber
To be specific, it does not check for OffsetNumberIsValid, it checks
that the offset can actually be stored in TidStore's bitmap array (see
comment above MAX_OFFSET_IN_BITMAP). In practice, I think it would
only matter with e.g. the combination of 32-bit platform and 32kB page
size, and even then only if some fork/AM/future core change allowed
about half the page to fill up with line pointers, since INT8_MAX *
BITS_PER_BITMAPWORD * sizeof(ItemIdData) = 16256
We should probably turn that elog into an assert. Maybe it would be
clearer if we just asserted the bitmap index is within range. We could
be more sure about the assertion if we changed "nwords" to be
unsigned. I think I had the idea to reserve -1 etc for a possible
special meaning, but I'm now thinking there's no use for that, since
we already have a separate struct member for flags.
> but doesn't do that for the given block
> number, which is not a bug neither, but I think we can add a check
> along with the test change.
The difference here is no configuration I can think of will cause an
unstorable block number to arrive here by accident. In fact, I imagine
TidStore would work just fine with 64-bit block numbers.
--
John Naylor
Amazon Web Services
