Re: Decoupling our alignment assumptions about int64 and double
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2026-02-02T21:46:31Z
Lists: pgsql-hackers
Attachments
- v3-0001-Decouple-our-alignment-assumptions-about-int64-an.patch (text/x-diff) patch v3-0001
- v3-0002-Add-new-alignment-info-to-pg_control-and-check-it.patch (text/x-diff) patch v3-0002
I pushed the att_align_nominal() refactoring, since that seems to be a win whether we go forward with the rest of this or not. Attached a v3 of the main patch (now again numbered 0001), plus new work in 0002 that adds the two new alignment values to pg_control and insists on a match before pg_upgrade'ing. As I mentioned, this'd result in not being able to pg_upgrade from old versions to v19 on AIX, though everywhere else it should be fine. 0001 is the same as before except for one change: I modified the test in CreateCast that checks whether a binary-compatible cast is allowable. I did that because I hit errors in cross-version upgrade testing. The regression tests do this: create type int8alias1 (..., like int8); create cast (int8 as int8alias1) without function; which seems perfectly legit, but the cast fails dump/reload. That's because we don't remember the "like" clause as such, so the dump of int8alias1 from an existing branch will say alignment = double, and then that doesn't match int8's new alignment = int64. Of course we could fix the cross-version tests by teaching AdjustUpgrade.pm to drop these casts, but I think this is telling us about a real usability gotcha that is likely to bite users. A closely related issue is the change I'd already had to make in regression/sql/float8.sql to not try to cast bigint to float8 without function; that used to work but doesn't with the new typalign values. What I did below is to instead insist on a match of the represented alignment requirement: - typ1align != typ2align) + typalign_to_alignby(typ1align) != typalign_to_alignby(typ2align)) I'm not totally sold on that particular formulation, however. For one thing it feels a bit too loose: on most machines it'd allow alignment = max to match int64 or double, and I'm not sure we want that. For another, it makes the allowable casts platform-dependent. With the v3 code, we could skip changing float8.sql on most platforms, but it'd still fail on AIX. An alternative I've been thinking about, but haven't quite convinced myself about, is: for pass-by-value types, why do we need to insist on alignment match at all? Once the value is in Datum format it doesn't matter, and for purposes of a binary-compatible cast the input and output are always in Datum format. So I was thinking about - typ1align != typ2align) + (typ1align != typ2align && !typ1byval)) which'd allow all these cases to pass without SQL-code changes. Is there a hole in that? An even weaker restriction would be "allow if alignby matches or if !byval", but I'm not sure we need to go there in practice. It'd re-introduce the problem of the test being platform dependent, which doesn't seem great. I'd expect to squash these into one commit in the end, but I kept 0002 separate for ease of review, since 0001 is the same as before except for the CreateCast diff. Thoughts? regards, tom lane
Commits
-
Cope with AIX's alignment woes by using _Pragma("pack").
- ecae09725236 19 (unreleased) landed
-
Make some minor cleanups in typalign-related code.
- 7cdb633c89da 19 (unreleased) landed
-
Refactor att_align_nominal() to improve performance.
- da7a1dc0d62a 19 (unreleased) landed