Re: Proposal: Conflict log history table for Logical Replication

On Sun, May 31, 2026 at 9:54 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> 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.

I think removing the word 'log' would introduce ambiguity.

IMO
A "conflict" is a *thing* (e.g. constraint violation) that happened.
And the "conflict log" is the *log* of the thing that happened.

e.g. "subconflictlogrelid" is clearly the relid of the conflict log
(CLT), but if it was just called "subconflictrelid" that sounds more
like the relid of where the conflict occurred.

>
> 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.
>

Another alternative for option name could be "conflict_reporting" with
values "log/table/all"

But, renaming the option may cause a cascade of necessary name changes
to everything else in docs and code.
e.g. Because the current option is "conflict_log_destination" we have
the CLT (Conflict Log Table)
e.g. If the option is "conflict_data_destination" then shouldn't the
CLT become the CDD (Conflict Data Table).
e.g. If the option is "conflict_reporting" then shouldn't the CLT
become the CRT (Conflict Report Table).
etc.

======
Kind Regards,
Peter Smith.
Fujitsu Australia