Re: pg_upgrade silently truncates nextMultiOffset to 32 bits

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_upgrade silently truncates nextMultiOffset to 32 bits
Date: 2026-08-27 17:25:31
Message-ID: CAD21AoAwaNbtxj73p4HhS_KHzouazWd9TvwTTMvnB4DGcptw3g@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Aug 27, 2026 at 1:58 AM Heikki Linnakangas <hlinnaka(at)iki(dot)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(dot)evan(dot)chao(at)gmail(dot)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

Attachment Content-Type Size
v2-0001-pg_upgrade-Read-nextMultiOffset-as-a-64-bit-value.patch application/x-patch 2.5 KB
v2-0002-Report-next_multi_offset-as-bigint-in-pg_control_.patch application/x-patch 3.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Rahila Syed 2026-08-27 17:52:51 Re: Fix unsafe coding in ResourceOwnerReleaseAll()
Previous Message Noah Misch 2026-08-27 17:18:30 pg_*_advice: tsv load failure, etc.