Re: Potential ABI breakage in upcoming minor releases

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "David E(dot) Wheeler" <david(at)justatheory(dot)com>
Cc: Noah Misch <noah(at)leadboat(dot)com>, Aleksander Alekseev <aleksander(at)timescale(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Marco Slot <marco(dot)slot(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Christoph Berg <myon(at)debian(dot)org>, Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com>
Subject: Re: Potential ABI breakage in upcoming minor releases
Date: 2024-11-26 01:56:50
Message-ID: 1576843.1732586210@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

[ getting back to the document-ABI-breakage-rules-better topic ... ]

I wrote:
> That text says exactly nothing about what specific code changes to
> make or not make. I'm not sure offhand where (or if) we have this
> documented, but there's an idea that adding fields at the end of
> a struct is safer ABI-wise than putting them in the middle. Which
> is true if you can't squeeze them into padding space. Here, that
> could have been done and probably should have.

I remembered where that's documented:

https://wiki.postgresql.org/wiki/Committing_checklist#Maintaining_ABI_compatibility_while_backpatching

The current text there is:

* You can only really change the signature of a function with local
linkage, perhaps with a few rare exceptions.

* You cannot modify any struct definition in src/include/*. If any new
members must be added to a struct, put them at the end in
backbranches. It's okay to have a different struct layout in
master. Even then, extensions that allocate the struct can break via
a dependency on its size.

* Move new enum values to the end.

I propose rewriting and expanding that:

* Don't change the signature of non-static functions. One common
workaround is to change the existing function into a wrapper that
calls a new function with more/different arguments.

* Don't change the contents of globally-visible structs, specifically
not the offsets of existing fields. If you must add a new field,
the very best way is to put it into existing alignment padding
between fields. (But consider both 32-bit and 64-bit cases when
deciding what is "padding".) If that's not possible, it may be okay
to add the field at the end of the struct; but you have to worry
about whether any extensions depend on the size of the struct.
This will not work well if extensions allocate new instances of the
struct. One particular gotcha is that it's not okay to change
sizeof(ResultRelInfo), as there are executor APIs that require
extensions to index into arrays of ResultRelInfos.

* Add new enum entries at the end of the enum's list, so that existing
entries don't change value.

* These rules only apply to released branches. In master, or in a new
branch that has not yet reached RC status, it's better to place new
fields and enum values in natural positions. Changing function
signatures is fine too, unless there are so many callers that
a compatibility wrapper seems advisable.

> The other bit of documentation we probably need is some annotation in
> struct ResultRelInfo saying "do not change the sizeof() this struct
> in back branches, period".

Something like

/*
* DO NOT change sizeof(ResultRelInfo) in a released branch. This
* generally makes it unsafe to add fields here in a released branch.
*/

at the end of struct ResultRelInfo's definition.

None of this is a substitute for installing some kind of ABI-checking
infrastructure; but that project doesn't seem to be moving fast.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2024-11-26 02:03:05 Re: Generating configure from configure.ac
Previous Message Tatsuo Ishii 2024-11-26 01:29:06 Generating configure from configure.ac