From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Markus Wanner <markus(dot)wanner(at)enterprisedb(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
Subject: | Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.] |
Date: | 2022-04-05 14:17:39 |
Message-ID: | 2358496.1649168259@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Tue, Apr 5, 2022 at 9:02 AM Markus Wanner
> <markus(dot)wanner(at)enterprisedb(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2022-04-05 14:29:03 | Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.] |
Previous Message | Robert Haas | 2022-04-05 14:01:56 | Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.] |
From | Date | Subject | |
---|---|---|---|
Next Message | Greg Stark | 2022-04-05 14:26:58 | Re: ExecRTCheckPerms() and many prunable partitions |
Previous Message | Andrew Dunstan | 2022-04-05 14:16:36 | Re: Mingw task for Cirrus CI |