From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
cc: Markus Wanner <markus.wanner@enterprisedb.com>,
        "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
        Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Subject: Re: API stability [was: pgsql: Fix possible recovery trouble if
 TRUNCATE overlaps a checkpoint.]
In-reply-to: 
 <CA+TgmoaVwQj_nG-jPB8O=t8=h-QBoF2U+tKviv8BT8unfTEZiw@mail.gmail.com>
References: <E1nXTCX-001DGF-R0@gemulon.postgresql.org>
 <a7880f4d-1d74-582a-ada7-dad168d046d1@enterprisedb.com>
 <CA+TgmoaVwQj_nG-jPB8O=t8=h-QBoF2U+tKviv8BT8unfTEZiw@mail.gmail.com>
Comments: In-reply-to Robert Haas <robertmhaas@gmail.com>
	message dated "Tue, 05 Apr 2022 10:01:56 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2358495.1649168259.1@sss.pgh.pa.us>
Date: Tue, 05 Apr 2022 10:17:39 -0400
Message-ID: <2358496.1649168259@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/2358496.1649168259%40sss.pgh.pa.us>
Precedence: bulk

Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Apr 5, 2022 at 9:02 AM Markus Wanner
> <markus.wanner@enterprisedb.com> wrote:
>> And for this specific case: Is it worth reverting this change and
>> applying a fully backwards compatible fix, instead?

> I think it's normally our policy to avoid changing definitions of
> accessible structs in back branches, except that we allow ourselves
> the indulgence of adding new members at the end or in padding space.
> So what would probably be best is if, in the back-branches, we changed
> "delayChkpt" back to a boolean, renamed it to delayChkptStart, and
> added a separate Boolean called delayChkptEnd. Maybe that could be
> added just after statusFlags, where I think it would fall into padding
> space.

Renaming it would constitute an API break, which is if anything worse
than an ABI break.

While we're complaining at you, let me point out that changing a field's
content and semantics while not changing its name is a time bomb waiting
to break any third-party code that looks at (or modifies...) the field.

What I think you need to do is:

1. In the back branches, revert delayChkpt to its previous type and
semantics.  Squeeze a separate delayChkptEnd bool in somewhere
(you can't change the struct size either ...).

2. In HEAD, rename the field to something like delayChkptFlags,
to ensure that any code touching it has to be inspected and updated.

In other words, this is already an API break in HEAD, and that's
fine, but it didn't break it hard enough to draw anyone's attention,
which is not fine.

			regards, tom lane