Re: Add lookup table for replication slot invalidation causes

On Wed, Feb 21, 2024 at 09:49:37AM +0530, Bharath Rupireddy wrote:
> On Wed, Feb 21, 2024 at 5:04 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> Done that way. I'm fine with the designated initialization [1] that an
> ISO C99 compliant compiler offers. PostgreSQL installation guide
> https://www.postgresql.org/docs/current/install-requirements.html says
> that we need an at least C99-compliant ISO/ANSI C compiler.

Note the recent commit 74a730631065 where Alvaro has changed for the
lwlock tranche names. That's quite elegant.

> Right, but an assertion isn't a bad idea there as it can generate a
> backtrace as opposed to the crash generating just SEGV note (and
> perhaps a crash dump) in server logs.
>
> With these two asserts, the behavior (asserts on null and non-existent
> inputs) is the same as what GetSlotInvalidationCause has right now.

Well, I won't fight you over that.

>> +/* Maximum number of invalidation causes */
>> +#define RS_INVAL_MAX_CAUSES RS_INVAL_WAL_LEVEL
>>>> There is no need to add that to slot.h: it is only used in slot.c.

>
> Right, but it needs to be updated whenever a new cause is added to
> enum ReplicationSlotInvalidationCause. Therefore, I think it's better
> to be closer there in slot.h.

A new cause would require an update of SlotInvalidationCause, so if
you keep RS_INVAL_MAX_CAUSES close to it that's impossible to miss.
IMO, it makes just more sense to keep that in slot.c because of the
static assert as well.

+ * If you add a new invalidation cause here, remember to add its name in
+ * SlotInvalidationCauses in the same order as that of the cause.

The order does not matter with the way v2 does things with
SlotInvalidationCauses[], no?
--
Michael