Andres Freund <[email protected]> writes:
> Thanks for the patch. Looks like a decent improvement.
Thanks for looking!
> On 2026-01-28 20:20:24 -0500, Tom Lane wrote:
>> ... It might be worth
>> worrying about the increased cost of att_align_nominal(),
>> but that macro is not that heavily used IMO.
> Perhaps we should make att_align_nominal() first determine the numerical
> alignment value and then have it use TYPEALIGN()? I think that'd be more
> likely to be pulled out of loops by the compile.
Yeah, I was thinking about actually breaking that into two source-code
steps, a function to map TYPALIGN_xxx to numeric alignment and then a
replacement for att_align_nominal that takes a numeric alignment.
If you think it's worth worrying about I'm happy to do that.
> Perhaps it's time to reformat att_align_nominal() into an static inline? It's
> pretty hard to read.
+1, I was not revisiting any of that for this draft, but if we're going
to refactor it then an inline function seems good.
> I don't love the 'l' for TYPALIGN_INT64, but I guess I don't really have a
> better suggestion.
Of course I was thinking 'l' for "long", but I agree it's not great
typographically. One idea is 'L' not 'l', but that gives up
consistency for visual separation. Any other ideas out there?
> It wouldn't hurt to have a short SQL level test for creating a type with int8
> & max alignments.
hmm ... yeah, I guess those code paths might not be covered already.
regards, tom lane
