Re: Proposal: Conflict log history table for Logical Replication

On Thu, Aug 20, 2026 at 8:15 AM Zhijie Hou (Fujitsu)
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Thursday, August 13, 2026 5:52 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > Yeah I have moved 29.8.1.1, Table-based logging and 29.8.1.2, File-based
> > logging section into the 29.8.1. Conflict logging section, but from the main
> > page I have only kept the link unabled for the 29.8.1. Conflict logging this is
> > same as how it is done for other subsections, e.g. 29.14.3.1 etc..
>
> Thanks for updating the patch.
>
> I was reviewing the v70 patch and noticed that it exposes three global flags
> for remote transaction information. I tried to see whether we could remove them,
> and realized that we already collect most of the remote transaction info in
> ApplyErrorCallbackArg.
>
> So, I think we could instead extend ApplyErrorCallbackArg to something more
> general that contains the remote transaction information, so that both the error
> callback and other places can directly get the value from there.
>
> One example is remote_final_lsn, where we store the value in both a separate
> static variable and in ApplyErrorCallbackArg. We could just remove the static
> variable and replace all existing references with ApplyErrorCallbackArg (saving
> one static variable).
>
> ApplyErrorCallbackArg could be renamed to something more general, like
> ApplyRemoteCtx, so it feels more natural for callers to use.
>
> The CLT patch will need one more piece of info (commit_ts), which can also be
> added to this general struct.
>
> Since this seems like something that can be added to HEAD independently, I put
> these changes into the 0001 patch and rebased the original v70 as 0002 on top of
> it. Sharing it here for reference.
>
> One more thing 0001 could potentially do is replace the existing references to
> stream_xid with ApplyRemoteCtx.remote_xid. It's not done in the patch yet, but
> can be if we reach consensus.
>
> --
>
> Apart from the above refactoring, my colleague Kuroda-San reported one issue
> off-list: a virtual generated column on the subscriber is converted to the
> constant string "virtual" in the log, but converted to NULL when inserted into
> the conflict log table.
>
> LOG: "local row (1, local_update, virtual)"
> table: {"a":1,"b":"local_update","c":null}
>
> I think even if there's a difference, since neither of them computes the real
> virtual value, it's not a big issue. Users can refer to the table schema to
> distinguish this from real NULL values, so I think there's little harm here.
>
> Besides, since a virtual column value isn't a real stored value, it doesn't
> really matter for conflict resolution. We could add some code to convert it to a
> constant string before JSON conversion, but the value doesn't justify the extra
> complexity.
>
> We could add some documentation in this version, and if real use cases outweigh
> the current approach, we can add the extra conversion logic later. I tried to
> write the doc and put it in the 0003 top-up patch.
>
> --
>
> One minor comment (not changed in the patch).
>
> +extern Relation GetConflictLogDestAndTable(ConflictLogDest *log_dest);
>
> GetConflictLogDestAndTable is only used inside conflict.c, so I think we can keep
> it static and remove the additional header inclusion.

Thanks for the patches, I have merged my changes with your patch and
also fixed this comment, and renamed GetConflictLogDestAndTable to
get_conflictlog_dest_and_table as this is a static function.

--
Regards,
Dilip Kumar
Google