pg_upgrade silently truncates nextMultiOffset to 32 bits
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253563psql -h localhost -U postgresBuilt from patchset v7 (message #7), August 27, 2026 at 05:38 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253563_7 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253563_7 && git checkout t253563_7Patchset v7 (message #7) is on t253563_7
Hi all,
(CCing Heikki as the committer of commit bd8d9c9bdfa)
Commit bd8d9c9bdfa widened MultiXactOffset to uint64, but I found that
pg_upgrade still reads it as a uint32 value when reading the
pg_controldata continents:
else if ((p = strstr(bufin, "Latest checkpoint's NextMultiOffset:")) != NULL)
{
:
p++; /* remove ':' char */
cluster->controldata.chkpnt_nxtmxoff = str2uint(p);
I think it should use strtou64() instead. The attached 0001 patch
fixes it. It introduces str2uint64() as other fields are read by a
similar helper function str2uint().
Also, when checking other similar codes around the new
MultiXactOffset, I found that pg_control_checkpoint() still reports
the value as an xid. I think we should report it as bigint instead.
What do you think? The attached 0002 patch fixes it.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachments:
t253563_10002-Report-next_multi_offset-as-bigint-in-pg_control_che.patchtext/x-patch; charset=US-ASCII; name=0002-Report-next_multi_offset-as-bigint-in-pg_control_che.patchDownload+3-4
0001-pg_upgrade-Read-nextMultiOffset-as-a-64-bit-value.patchtext/x-patch; charset=US-ASCII; name=0001-pg_upgrade-Read-nextMultiOffset-as-a-64-bit-value.patchDownload+13-2
On Aug 27, 2026, at 08:59, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
Hi all,
(CCing Heikki as the committer of commit bd8d9c9bdfa)Commit bd8d9c9bdfa widened MultiXactOffset to uint64, but I found that
pg_upgrade still reads it as a uint32 value when reading the
pg_controldata continents:else if ((p = strstr(bufin, "Latest checkpoint's NextMultiOffset:")) != NULL)
{
:
p++; /* remove ':' char */
cluster->controldata.chkpnt_nxtmxoff = str2uint(p);I think it should use strtou64() instead. The attached 0001 patch
fixes it. It introduces str2uint64() as other fields are read by a
similar helper function str2uint().Also, when checking other similar codes around the new
MultiXactOffset, I found that pg_control_checkpoint() still reports
the value as an xid. I think we should report it as bigint instead.
What do you think? The attached 0002 patch fixes it.
bigint is a signed int64, so it cannot represent the full uint64 range, although perhaps this is only a theoretical concern. If we want to avoid this limitation, should we use numeric instead?
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
<0002-Report-next_multi_offset-as-bigint-in-pg_control_che.patch><0001-pg_upgrade-Read-nextMultiOffset-as-a-64-bit-value.patch>
Overall the patch looks good to me.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
On Thu, Aug 27, 2026 at 12:06 AM Chao Li <li.evan.chao@gmail.com> wrote:
On Aug 27, 2026, at 08:59, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
Hi all,
(CCing Heikki as the committer of commit bd8d9c9bdfa)Commit bd8d9c9bdfa widened MultiXactOffset to uint64, but I found that
pg_upgrade still reads it as a uint32 value when reading the
pg_controldata continents:else if ((p = strstr(bufin, "Latest checkpoint's NextMultiOffset:")) != NULL)
{
:
p++; /* remove ':' char */
cluster->controldata.chkpnt_nxtmxoff = str2uint(p);I think it should use strtou64() instead. The attached 0001 patch
fixes it. It introduces str2uint64() as other fields are read by a
similar helper function str2uint().Also, when checking other similar codes around the new
MultiXactOffset, I found that pg_control_checkpoint() still reports
the value as an xid. I think we should report it as bigint instead.
What do you think? The attached 0002 patch fixes it.bigint is a signed int64, so it cannot represent the full uint64 range, although perhaps this is only a theoretical concern. If we want to avoid this limitation, should we use numeric instead?
I'd prefer to keep bigint here. pg_get_multixact_stats() already
reports num_members and members_size as int8, and both are derived
from these same offsets. Also, other fields in pg_control_checkpoint()
are fixed-width types, whereas numeric is pass-by-reference.
I considered using xid8 instead but it has only comparison operators
and no arithmetic, so we couldn't compute a delta between two
checkpoints.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
On 27/08/2026 10:06, Chao Li wrote:
On Aug 27, 2026, at 08:59, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
Hi all,
(CCing Heikki as the committer of commit bd8d9c9bdfa)Commit bd8d9c9bdfa widened MultiXactOffset to uint64, but I found that
pg_upgrade still reads it as a uint32 value when reading the
pg_controldata continents:else if ((p = strstr(bufin, "Latest checkpoint's NextMultiOffset:")) != NULL)
{
:
p++; /* remove ':' char */
cluster->controldata.chkpnt_nxtmxoff = str2uint(p);I think it should use strtou64() instead. The attached 0001 patch
fixes it. It introduces str2uint64() as other fields are read by a
similar helper function str2uint().Also, when checking other similar codes around the new
MultiXactOffset, I found that pg_control_checkpoint() still reports
the value as an xid. I think we should report it as bigint instead.
What do you think? The attached 0002 patch fixes it.
Thanks, good catch!
bigint is a signed int64, so it cannot represent the full uint64 range, although perhaps this is only a theoretical concern. If we want to avoid this limitation, should we use numeric instead?
xid8 seems like the most straightforward replacement. It's a little
bogus as a multixact offset it's not really an XID. But we were using
the 32-bit 'xid' type for it previously, it's in line with that.
I'll go do that. This requires bumping the catalog version, but since
we've bumped it already since 19beta3, that's OK.
- Heikki
Sorry, I missed this reply of yours earlier.
On 27/08/2026 10:35, Masahiko Sawada wrote:
On Thu, Aug 27, 2026 at 12:06 AM Chao Li <li.evan.chao@gmail.com> wrote:
bigint is a signed int64, so it cannot represent the full uint64 range, although perhaps this is only a theoretical concern. If we want to avoid this limitation, should we use numeric instead?
I'd prefer to keep bigint here. pg_get_multixact_stats() already
reports num_members and members_size as int8, and both are derived
from these same offsets. Also, other fields in pg_control_checkpoint()
are fixed-width types, whereas numeric is pass-by-reference.I considered using xid8 instead but it has only comparison operators
and no arithmetic, so we couldn't compute a delta between two
checkpoints.
Hmm, that's a good point, although 'xid' didn't have those operators or
arithmetic either.
- Heikki
On 27/08/2026 11:20, Heikki Linnakangas wrote:
Sorry, I missed this reply of yours earlier.
On 27/08/2026 10:35, Masahiko Sawada wrote:
On Thu, Aug 27, 2026 at 12:06 AM Chao Li <li.evan.chao@gmail.com> wrote:
bigint is a signed int64, so it cannot represent the full uint64
range, although perhaps this is only a theoretical concern. If we
want to avoid this limitation, should we use numeric instead?I'd prefer to keep bigint here. pg_get_multixact_stats() already
reports num_members and members_size as int8, and both are derived
from these same offsets. Also, other fields in pg_control_checkpoint()
are fixed-width types, whereas numeric is pass-by-reference.I considered using xid8 instead but it has only comparison operators
and no arithmetic, so we couldn't compute a delta between two
checkpoints.Hmm, that's a good point, although 'xid' didn't have those operators or
arithmetic either.
That was inaccurate: both 'xid' and 'xid8' do have comparison operators.
But they don't have a "minus" or "diff" operator, so you indeed cannot
easily do "b - a".
I don't have a strong opinion, I'm happy with either bigint or xid8
here. Bigint is probably more convenient in practice, and it's good to
not confuse mxact offsets with transaction ids by abusing the xid8 type.
Then again, it was 'xid' before, which had the same issues and we went
with 'xid' anyway. Then again, now that it doesn't wrap around anymore,
maybe 'bigint' makes more sense now.
Would you like to decide and commit this, or would you prefer me to do it?
- Heikki
On Thu, Aug 27, 2026 at 1:58 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
On 27/08/2026 11:20, Heikki Linnakangas wrote:
Sorry, I missed this reply of yours earlier.
On 27/08/2026 10:35, Masahiko Sawada wrote:
On Thu, Aug 27, 2026 at 12:06 AM Chao Li <li.evan.chao@gmail.com> wrote:
bigint is a signed int64, so it cannot represent the full uint64
range, although perhaps this is only a theoretical concern. If we
want to avoid this limitation, should we use numeric instead?I'd prefer to keep bigint here. pg_get_multixact_stats() already
reports num_members and members_size as int8, and both are derived
from these same offsets. Also, other fields in pg_control_checkpoint()
are fixed-width types, whereas numeric is pass-by-reference.I considered using xid8 instead but it has only comparison operators
and no arithmetic, so we couldn't compute a delta between two
checkpoints.Hmm, that's a good point, although 'xid' didn't have those operators or
arithmetic either.That was inaccurate: both 'xid' and 'xid8' do have comparison operators.
But they don't have a "minus" or "diff" operator, so you indeed cannot
easily do "b - a".I don't have a strong opinion, I'm happy with either bigint or xid8
here. Bigint is probably more convenient in practice, and it's good to
not confuse mxact offsets with transaction ids by abusing the xid8 type.
Then again, it was 'xid' before, which had the same issues and we went
with 'xid' anyway. Then again, now that it doesn't wrap around anymore,
maybe 'bigint' makes more sense now.
I missed the point that we used to use 'xid' for that field. But I
agree that 'bigint' makes more sense.
Would you like to decide and commit this, or would you prefer me to do it?
I'm going to take them and go with the 'bigint' column if you're okay.
I've added the commit messages to the patches.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachments:
t253563_7v2-0001-pg_upgrade-Read-nextMultiOffset-as-a-64-bit-value.patchapplication/x-patch; name=v2-0001-pg_upgrade-Read-nextMultiOffset-as-a-64-bit-value.patchDownload+13-2
v2-0002-Report-next_multi_offset-as-bigint-in-pg_control_.patchapplication/x-patch; name=v2-0002-Report-next_multi_offset-as-bigint-in-pg_control_.patchDownload+3-4
On 27/08/2026 20:25, Masahiko Sawada wrote:
On Thu, Aug 27, 2026 at 1:58 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
On 27/08/2026 11:20, Heikki Linnakangas wrote:
On 27/08/2026 10:35, Masahiko Sawada wrote:
I considered using xid8 instead but it has only comparison operators
and no arithmetic, so we couldn't compute a delta between two
checkpoints.Hmm, that's a good point, although 'xid' didn't have those operators or
arithmetic either.That was inaccurate: both 'xid' and 'xid8' do have comparison operators.
But they don't have a "minus" or "diff" operator, so you indeed cannot
easily do "b - a".I don't have a strong opinion, I'm happy with either bigint or xid8
here. Bigint is probably more convenient in practice, and it's good to
not confuse mxact offsets with transaction ids by abusing the xid8 type.
Then again, it was 'xid' before, which had the same issues and we went
with 'xid' anyway. Then again, now that it doesn't wrap around anymore,
maybe 'bigint' makes more sense now.I missed the point that we used to use 'xid' for that field. But I
agree that 'bigint' makes more sense.Would you like to decide and commit this, or would you prefer me to do it?
I'm going to take them and go with the 'bigint' column if you're okay.
I've added the commit messages to the patches.
Ok, thank you!
- Heikki
On Thu, Aug 27, 2026 at 10:56 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
On 27/08/2026 20:25, Masahiko Sawada wrote:
On Thu, Aug 27, 2026 at 1:58 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
On 27/08/2026 11:20, Heikki Linnakangas wrote:
On 27/08/2026 10:35, Masahiko Sawada wrote:
I considered using xid8 instead but it has only comparison operators
and no arithmetic, so we couldn't compute a delta between two
checkpoints.Hmm, that's a good point, although 'xid' didn't have those operators or
arithmetic either.That was inaccurate: both 'xid' and 'xid8' do have comparison operators.
But they don't have a "minus" or "diff" operator, so you indeed cannot
easily do "b - a".I don't have a strong opinion, I'm happy with either bigint or xid8
here. Bigint is probably more convenient in practice, and it's good to
not confuse mxact offsets with transaction ids by abusing the xid8 type.
Then again, it was 'xid' before, which had the same issues and we went
with 'xid' anyway. Then again, now that it doesn't wrap around anymore,
maybe 'bigint' makes more sense now.I missed the point that we used to use 'xid' for that field. But I
agree that 'bigint' makes more sense.Would you like to decide and commit this, or would you prefer me to do it?
I'm going to take them and go with the 'bigint' column if you're okay.
I've added the commit messages to the patches.
Ok, thank you!
Pushed.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com