On Mon, May 3, 2021 at 2:13 PM Jeff Davis <pgsql@j-davis.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