Hi, Previously ([1] #19 and #22) I had suggested that some conflict_reason code could be split off and pushed first as a prerequisite/ preparatory/ independent patch.
At the time, my suggestion was premature because there was still a lot of code under development. But now the affected code is in the first patch 00001, and there are already other precedents of slot-sync preparatory patches getting pushed. So I thought to resurrect this splitting suggestion again, as perhaps is the right time to do it. Details are below: ====== IMO the new #defines for the conflict_reason stuff plus where they get used can be pushed as an independent patch. Specifically, this stuff: ~~~ >From src/include/replication/slot.h: +/* + * The possible values for 'conflict_reason' returned in + * pg_get_replication_slots. + */ +#define SLOT_INVAL_WAL_REMOVED_TEXT "wal_removed" +#define SLOT_INVAL_HORIZON_TEXT "rows_removed" +#define SLOT_INVAL_WAL_LEVEL_TEXT "wal_level_insufficient" + ~~~ >From src/backend/replication/logical/slotsync.c: Also, IMO this function should live in slot.c; Although slotsync.c might be the only caller, this is not really a slot-sync specific function. +/* + * Maps the pg_replication_slots.conflict_reason text value to + * ReplicationSlotInvalidationCause enum value + */ +static ReplicationSlotInvalidationCause +get_slot_invalidation_cause(char *conflict_reason) +{ + Assert(conflict_reason); + + if (strcmp(conflict_reason, SLOT_INVAL_WAL_REMOVED_TEXT) == 0) + return RS_INVAL_WAL_REMOVED; + else if (strcmp(conflict_reason, SLOT_INVAL_HORIZON_TEXT) == 0) + return RS_INVAL_HORIZON; + else if (strcmp(conflict_reason, SLOT_INVAL_WAL_LEVEL_TEXT) == 0) + return RS_INVAL_WAL_LEVEL; + else + Assert(0); + + /* Keep compiler quiet */ + return RS_INVAL_NONE; +} ~~~ >From src/backend/replication/slotfuncs.c: case RS_INVAL_WAL_REMOVED: - values[i++] = CStringGetTextDatum("wal_removed"); + values[i++] = CStringGetTextDatum(SLOT_INVAL_WAL_REMOVED_TEXT); break; case RS_INVAL_HORIZON: - values[i++] = CStringGetTextDatum("rows_removed"); + values[i++] = CStringGetTextDatum(SLOT_INVAL_HORIZON_TEXT); break; case RS_INVAL_WAL_LEVEL: - values[i++] = CStringGetTextDatum("wal_level_insufficient"); + values[i++] = CStringGetTextDatum(SLOT_INVAL_WAL_LEVEL_TEXT); break; ~~~ Thoughts? ====== [1] https://www.postgresql.org/message-id/CAHut%2BPtJAAPghc4GPt0k%3DjeMz1qu4H7mnaDifOHsVsMqi-qOLA%40mail.gmail.com Kind Regards, Peter Smith. Fujitsu Australia