On Thu, Dec 25, 2025 at 10:30:37AM +0900, Michael Paquier wrote:
> I am wondering if it would not be cleaner and less confusing to do
> things slightly differently (sorry I did not pay much attention to
> that previously):
> - Change GetMultiXactInfo() to return two offsets, nextOffset and
> oldestOffset.
> - Use uint64 for members and recalculate the difference in
> MultiXactMemberFreezeThreshold() and the function code. Heikki has
> just switched multixact offsets to be 64 bits, yippee.
> - Redefine MultiXactMemberStorageSize() so as it does not take a
> number of members in input, but as the amount of space taken between
> two offsets. At least that would be more consistent with all the
> other inline functions of multixact.h that rely on MultiXactOffset
> inputs. Using a int64 is still OK I guess, there may be a case to
> detect "negative" numbers and give a change to the users of the new
> inline function to notice that they did a computation wrong, rather
> than hiding a signedness problem.
So, here is what I have in mind, split into independent pieces:
- Remove the existing type confusion with GetMultiXactInfo(), due to
how things have always been done in MultiXactMemberFreezeThreshold().
- Add macro MultiXactOffsetStorageSize(), to calculate the amount of
space used between two offsets.
- The main patch, with adjustments in comments, the test (no
non-ASCII characters in that, please). One thing that was really
surprising is that you did not consider ROLE_PG_READ_ALL_STATS. We
expect all the stats information to be hidden if a role is not granted
access to them, and this function should be no exception especially as
it relates to disk space usage like database or tablespace size
functions.
Anyway, attached are all these updated pieces. The doc edits are what
I have mentioned upthread, close to what you have suggested to me
offline.
Comments?
--
Michael
