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.
