RE: Proposal: Conflict log history table for Logical Replication

From: "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>
To: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, saurabh singh <saurabh(dot)singh214(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, shveta malik <shveta(dot)malik(at)gmail(dot)com>
Subject: RE: Proposal: Conflict log history table for Logical Replication
Date: 2026-08-20 02:45:47
Message-ID: TY4PR01MB177186ED4D38C5356294CC51C94A42@TY4PR01MB17718.jpnprd01.prod.outlook.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

Best Regards,
Zhijie Hou

Attachment Content-Type Size
v70-0001-Refactor-apply-worker-s-remote-transaction-track.patch application/octet-stream 20.1 KB
v70-0003-Doc-note-virtual-generated-columns-in-the-confli.patch application/octet-stream 1.5 KB
v70-0002-Implement-the-conflict-insertion-infrastructure-.patch application/octet-stream 81.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2026-08-20 02:50:49 Re: Test tidscan,sql is not immune to autovacuum in v14
Previous Message Bharath Rupireddy 2026-08-20 02:37:00 Re: Report index currently being vacuumed in pg_stat_progress_vacuum