Re: Potential ABI breakage in upcoming minor releases

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.