On Tue, 20 Feb 2024 at 12:11, Bharath Rupireddy
<bharath.rupireddyforpostg...@gmail.com> wrote:
> Thoughts?

Seems like a good improvement overall. But I'd prefer the definition
of the lookup table to use this syntax:

const char *const SlotInvalidationCauses[] = {
    [RS_INVAL_NONE] = "none",
    [RS_INVAL_WAL_REMOVED] = "wal_removed",
    [RS_INVAL_HORIZON] = "rows_removed",
    [RS_INVAL_WAL_LEVEL] = "wal_level_sufficient",
};


Regarding the actual patch:

-   Assert(conflict_reason);

Probably we should keep this Assert. As well as the Assert(0)


+   for (cause = RS_INVAL_NONE; cause <= RS_INVAL_MAX_CAUSES; cause++)

Strictly speaking this is a slight change in behaviour, since now
"none" is also parsed. That seems fine to me though.


Reply via email to