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_backpatching
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.
