Re: Proposal: Conflict log history table for Logical Replication

On Thu, 5 Nov 2026 at 14:24, shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> I am not sure if you have started taking comments on 002 yet. Here are
> a few comments; feel free to defer them to later versions if we want
> to conclude on 001 first.
>
> 1)
> CONFLICT_LOG_RELATION_NAME_FMT is only used in backend code
> (conflict.c and subscriptioncmds.c), so I think it need not be inside
> the '#ifdef EXPOSE_TO_CLIENT_CODE'?
>
>
> 2)
> + if (dopt->binary_upgrade)
> + tbinfo->dobj.dump = DUMP_COMPONENT_DEFINITION | DUMP_COMPONENT_ACL;
> + else
> + tbinfo->dobj.dump = DUMP_COMPONENT_NONE;
>
> In a regular (non-binary-upgrade) dump, we do not dump definition of
> CLT (which is fine) as the table gets created if needed on dumped
> 'ALTER SUB' but we do not even dump privileges granted on it (because
> the name of CLT is not known). So post-upgrade user has to execute
> GRANTs etc again (if any). I think it would be worth documenting this
> limitation in the pg_dump doc, alongside the existing information
> about subscriptions.
>
> 3)
> dumpTableSchema:
> + bool conflict_log_table =
> + (strcmp(tbinfo->dobj.namespace->dobj.name, "pg_conflict") == 0);
>
> selectDumpableNamespace:
> + if (strcmp(tbinfo->dobj.namespace->dobj.name, "pg_conflict") == 0)
>
> Should we make a function for this usage:
>
> static bool
> is_conflict_log_table(const TableInfo *tbinfo)
> {
> return strcmp(tbinfo->dobj.namespace->dobj.name, "pg_conflict") == 0;
> }

Thanks for the comments, these comments are addressed in the v93
version patch attached. Apart from these there were a couple of
additional comments of Hou-san addressed which he shared with me
offlist, thanks Hou-san for that. There is no change in 0001 patch, it
is the same as the v92 version patch.

Regards,
Vignesh