Re: MaxOffsetNumber for Table AMs

On Mon, May 3, 2021 at 2:13 PM Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
> That's not clear to me at all, and is the whole reason I began this
> thread.
>
> a. You say "smaller than MaxOffsetNumber", but that's a little weird.
> If an offset can't be MaxOffsetNumber, it's not really the maximum, is
> it?

I wasn't trying to be that precise. I see that OffsetNumberIsValid()
returns true if the offset is <= MaxOffsetNumber, so therefore I agree
that using exactly MaxOffsetNumber ought to work.

> b. If you actually meant "less than or equal to MaxOffsetNumber",
> that will fail with the GIN posting list issue raised in my first
> email. Do you agree that's a bug?

Given the above, yes.

> c. Why can't we go all the way up to MovedPartitionsOffsetNumber - 1?
> Right now, MaxOffsetNumber is poorly named, because it actually
> represents the a number slightly higher than the maximum number of
> items that can fit on a page. That essentially wastes 5 bits of address
> space for no obvious reason.

Because of stuff like this:

[rhaas EDBAS]$ git grep -F '[MaxOffsetNumber'
src/backend/access/gist/gistvacuum.c: OffsetNumber
todelete[MaxOffsetNumber];
src/backend/access/gist/gistvacuum.c: OffsetNumber
todelete[MaxOffsetNumber];
src/backend/access/gist/gistvacuum.c: BlockNumber
leafs_to_delete[MaxOffsetNumber];
src/backend/access/hash/hash.c: OffsetNumber deletable[MaxOffsetNumber];
src/backend/access/hash/hashinsert.c: OffsetNumber
deletable[MaxOffsetNumber];
src/backend/access/hash/hashovfl.c: OffsetNumber
deletable[MaxOffsetNumber];

Maybe changing those places to use dynamic allocation wouldn't hurt
anything in terms of performance, but I'm not sure. Making them 32
times larger categorically does not seem like a good idea.

There might be other dependencies on this value in other parts of the
code; I'm not sure.

--
Robert Haas
EDB: http://www.enterprisedb.com