On Tue, Sep 27, 2022 at 5:50 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Maybe it wouldn't have any great impact. I don't know, but I don't
> think it's incumbent on me to measure that. You or the patch author
> should have had a handle on that question *before* committing.
I agree. I should have gone through and checked that every place where
RelFileLocator got embedded in some larger struct, there was no
problem with making it bigger and increasing the alignment
requirement. I'll go back and do that as soon as the immediate
problems are fixed. This case would have stood out as something
needing attention.
Some of the cases are pretty subtle, though. tamandua is still unhappy
even after pushing that fix, because xl_xact_relfilelocators embeds a
RelFileLocator which now requires 8-byte alignment, and
ParseCommitRecord has an undocumented assumption that none of the
things embedded in a commit record require more than 4-byte alignment.
Really, if it's critical for a struct to never require more than
4-byte alignment, there ought to be a comment about that *on the
struct itself*. Having a comment on a function that does something
with that struct is probably not really good enough, and we don't even
have that.
--
Robert Haas
EDB: http://www.enterprisedb.com