public inbox for [email protected]
help / color / mirror / Atom feedFrom: Noah Misch <[email protected]>
To: Pavan Deolasee <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: Tom Lane <[email protected]>
Subject: Re: Potential ABI breakage in upcoming minor releases
Date: Thu, 14 Nov 2024 06:35:24 -0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CABOikdNmVBC1LL6pY26dyxAS2f+gLZvTsNt=2XbcyG7WxXVBBQ@mail.gmail.com>
References: <CABOikdNmVBC1LL6pY26dyxAS2f+gLZvTsNt=2XbcyG7WxXVBBQ@mail.gmail.com>
On Thu, Nov 14, 2024 at 04:18:02PM +0530, Pavan Deolasee wrote:
> Commit 51ff46de29f67d73549b2858f57e77ada8513369 (backported all the way
> back to v12) added a new member to `ResultRelInfo` struct. This can
> potentially cause ABI breakage for the extensions that allocate the struct
> and pass it down to the PG code. The previously built extensions may
> allocate a shorter struct, while the new PG code would expect a larger
> struct, thus overwriting some memory unintentionally.
>
> A better approach may have been what Tom did in
> 8cd190e13a22dab12e86f7f1b59de6b9b128c784, but I understand it might be too
> late to change this since the releases are already tagged. Nevertheless, I
> thought of bringing it up if others have different views.
The release is now announced. We could issue new releases (likely next
Thursday) if there's a problem that warrants it.
Based on a grep of PGXN code, here are some or all of the modules that react
to sizeof(ResultRelInfo):
$ grepx -r 'lloc.*ResultRelInfo' | tee /tmp/1 | sed 's/-[^:]*/:/'|sort -u
apacheage:: resultRelInfo = palloc(sizeof(ResultRelInfo));
pg_pathman:: ResultRelInfo *resultRelInfo = (ResultRelInfo *) palloc(sizeof(ResultRelInfo));
$ grepx -r 'Node.*ResultRelInfo' | tee /tmp/2 | sed 's/-[^:]*/:/'|sort -u
apacheage:: cypher_node->resultRelInfo = makeNode(ResultRelInfo);
apacheage:: cypher_node->resultRelInfo = makeNode(ResultRelInfo);
citus:: resultRelInfo = makeNode(ResultRelInfo);
citus:: ResultRelInfo *resultRelInfo = makeNode(ResultRelInfo);
pg_bulkload:: checker->resultRelInfo = makeNode(ResultRelInfo);
pg_bulkload:: self->relinfo = makeNode(ResultRelInfo);
pg_pathman:: child_result_rel_info = makeNode(ResultRelInfo);
pg_pathman:: parent_result_rel = makeNode(ResultRelInfo);
pg_pathman:: parent_rri = makeNode(ResultRelInfo);
pg_pathman:: part_result_rel_info = makeNode(ResultRelInfo);
vops:: resultRelInfo = makeNode(ResultRelInfo);
I don't know whether we should make a new release, amend the release
announcement to call for extension rebuilds, or just stop here.
https://wiki.postgresql.org/wiki/Committing_checklist#Maintaining_ABI_compatibility_while_backpatchi...
mentions the problem, but neither it nor the new standard at
postgr.es/c/e54a42a say how reticent we'll be to add to the end of a struct on
which extensions do sizeof.
The postgr.es/c/e54a42a standard would have us stop here. But I'm open to
treating the standard as mistaken and changing things.
view thread (3+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: Potential ABI breakage in upcoming minor releases
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox