Re: Proposal: Document ABI Compatibility

On 04.06.24 02:11, Laurenz Albe wrote:
> On Mon, 2024-06-03 at 15:38 -0400, Tom Lane wrote:
>> Andres Freund <andres(at)anarazel(dot)de> writes:
>>> On 2024-06-03 14:43:17 -0400, David E. Wheeler wrote:
>>>> * The ABI is guaranteed to change only in backward compatible ways in minor
>>>> releases. If for some reason it doesn’t it’s a bug that will need to be
>>>> fixed.
>>
>>> Thus I am not really on board with this statement as-is.
>>
>> Me either.  There are degrees of ABI compatibility, and we'll choose
>> the least invasive way, but it's seldom the case that no conceivable
>> extension will be broken.
>
> oracle_fdw has been broken by minor releases several times in the past.
> This may well be because of weird things that I am doing; still, my
> experience is that minor releases are not always binary compatible.
>
>>> It'd be interesting to see a few examples of actual minor-version-upgrade
>>> extension breakages, so we can judge what caused them.
>>
>> Yes, that could be a fruitful discussion.
>
> Digging through my commits brought up 6214e2b2280462cbc3aa1986e350e167651b3905,
> for one.

I'm not sure I can see how that would have broken oracle_fdw, but in any
case it's an interesting example. This patch did not change any structs
incompatibly, but it changed the semantics of a function without
changing the name:

extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
- Relation partition_root,
+ ResultRelInfo *partition_root_rri,
int instrument_options);

If an extension calls this function, something would possibly crash if
it's on the wrong side of the update fence.

This could possibly be avoided by renaming the symbol in backbranches.
Maybe something like

#define InitResultRelInfo InitResultRelInfo2

Then you'd get a specific error message when loading the module, rather
than a crash.

This might be something to consider:

no ABI break is better than an explicit ABI break is better than a
silent ABI break

(Although this is actually an API break, isn't it?)