Re: Proposal: Conflict log history table for Logical Replication

On Sat, May 30, 2026 at 1:12 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>

Few comments on 0001 and 0002
===========================
1.
+ Oid         subconflictlogrelid; /* Relid of the conflict log table. */
 #ifdef CATALOG_VARLEN /* variable-length fields start here */
+ /*
+ * Strategy for logging replication conflicts:
+ * 'log' - server log only,
+ * 'table' - conflict log table only,
+ * 'all' - both log and table.
+ */
+ text subconflictlogdest BKI_FORCE_NOT_NULL;

'log' sounds redundant in the above two field names. I feel naming
them as subconflictrelid and subconflictdest should be sufficient.

2. If you agree with the above, then let's make similar changes at
other places in the patch. We can change
alter_sub_conflictlogdestination to alter_sub_conflict_destination.
Also, similar to AlterSubscription_refresh and
AlterSubscription_refresh_seq, we can name this new function as
AlterSubscription_conflict_dest.

3. Now, let's consider whether we should change the option name to
conflict_data_destination instead of conflict_log_destination? The
reason I am asking to consider this change is that one of the option
values is 'log', so it sounded a bit odd to name the option as
conflict_log_destination. If we change this then we can consider
changing the name of Enum ConflictLogDest as well.

Apart from above, I have made some changes in the attached. Kindly
review and see which all can be incorporated in the next version.

-- 
With Regards,
Amit Kapila.