On Wed, Apr 8, 2026 at 12:24 PM Fujii Masao <[email protected]> wrote: > > On Wed, Apr 8, 2026 at 12:36 PM Fujii Masao <[email protected]> wrote: > > The backpatch added PROCSIG_SLOTSYNC_MESSAGE in the middle of enum > > ProcSignalReason, which could break the ABI. I’m planning to move it to > > the end of the enum in v17 and v18. > > > > That seems to work for v18. However, in v17, NUM_PROCSIGNALS is defined > > as the last enum value: > > > > NUM_PROCSIGNALS /* Must be last! */ > > } ProcSignalReason; > > > > So simply moving PROCSIG_SLOTSYNC_MESSAGE to the end would change the > > meaning > > of NUM_PROCSIGNALS. > > > > One option might be to remove NUM_PROCSIGNALS from the enum, move > > PROCSIG_SLOTSYNC_MESSAGE to the end, and define it separately, e.g. > > #define NUM_PROCSIGNALS (PROCSIG_SLOTSYNC_MESSAGE + 1). Would that > > be acceptable without breaking the ABI? Thoughts? > > The patches I'm planning to apply for v17 and v18 are attached. > > For v17, I'm still not entirely sure this change is safe from an ABI > perspective. Even if it is, abi-compliance-check may still report > a break since the patch removes NUM_PROCSIGNALS from the enum > (defines it as separate macro). If so, we may need to update > .abi-compliance-history to avoid false positives. >
Regarding the pg17 change, NUM_PROCSIGNALS is not a process signal reason but simply represents the array size, and its value will also increase in pg18 (+1) after this backpatch. AFAIU, the concern is that extensions might rely on the old compiled values of PROCSIG_*, so we should avoid changing their order. However, extensions should also not depend on NUM_PROCSIGNALS directly, otherwise the pg18 backpatch would pose the same ABI concern. So, it seems safe for pg17 as well. I also checked core extensions and did not find NUM_PROCSIGNALS being used. That said, I think both approaches - adding the new entry at the end and defining NUM_PROCSIGNALS outside as done in the patch or adding it just before NUM_PROCSIGNALS (like below) are semantically the same. …. PROCSIG_RECOVERY_CONFLICT_LAST = PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK, + PROCSIG_SLOTSYNC_MESSAGE /* ask slot synchronization to stop */ + NUM_PROCSIGNALS /* Must be last! */ } ProcSignalReason; As NUM_PROCSIGNALS increments in both cases, I don’t see any additional benefit in defining it outside. Thoughts? Happy to be corrected if I’m missing something. -- Thanks, Nisha
