Re: Potential ABI breakage in upcoming minor releases - Mailing list 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