Re: Proposal: Conflict log history table for Logical Replication

On Mon, Jun 8, 2026 at 2:52 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
>
> On Fri, Jun 5, 2026 at 4:22 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > Here is the updated patch which fixes all open issues except Peter
> > reported on 0004 patch, Vignesh would you take care of that?
> >
>
> Thanks Dilip for the updated patches.
>
> Patch 001 + 002
> 1) Issue #5 from [1] still appears to be present. I still see NULL
> values in remote_commit_ts. From patch, it looks like remote_commit_ts
> is updated in v46, but too late in the flow. I tested by moving:
> /* Set remote_commit_ts for conflict logging. */
> remote_commit_ts = commit_data.committime;
> before the apply_spooled_messages() call, and remote_commit_ts was
> logged correctly. It seems the assignment needs to happen earlier.

Yeah right, I have fixed it, will be available in next version

> 2) CLT reports NULL or stale remote_xid for two-phase transactions
> Test case:
> setup : pub-sub nodes with max_prepared_transactions = 10 to enable two_phase
> -- Publisher setup:
> CREATE TABLE clt_test (id int PRIMARY KEY);
> CREATE PUBLICATION clt_pub FOR TABLE clt_test;
>
> -- Subscriber setup:
> CREATE TABLE clt_test (id int PRIMARY KEY);
> INSERT INTO clt_test VALUES (1); -- pre-existing row to conflict
> CREATE SUBSCRIPTION clt_sub
> CONNECTION 'port=9933 dbname=postgres'
> PUBLICATION clt_pub
> WITH (two_phase = true, conflict_log_destination = 'all');
>
> -- Trigger a conflict via a prepared transaction on the publisher:
> -- Publisher:
> BEGIN;
> select txid_current();
> INSERT INTO clt_test VALUES (1); -- will conflict on subscriber
> PREPARE TRANSACTION 'clt_bug_test';
> COMMIT PREPARED 'clt_bug_test';
>
> -- Check the CLT on the subscriber. The publisher transaction was 699,
> but the CLT contains:
> postgres=# SELECT conflict_type, remote_xid, remote_commit_ts FROM
> pg_conflict.pg_conflict_log_16398 ;
> conflict_type | remote_xid | remote_commit_ts
> ---------------+------------+----------------------------------
> insert_exists | 698 | 2026-06-08 14:04:34.903284+05:30
> insert_exists | |
> insert_exists | |
> ...
> It looks like remote_xid is either stale values from a previous
> transaction or remain unset when apply worker tries again.
> The change below fixes it.
> @@ -1300,6 +1300,8 @@ apply_handle_begin_prepare(StringInfo s)
> set_apply_error_context_xact(begin_data.xid, begin_data.prepare_lsn);
>
> remote_final_lsn = begin_data.prepare_lsn;
> + remote_xid = begin_data.xid;
> + remote_commit_ts = 0;

I couldn't reproduce this, but I think we should reset remote_xid and
remote_commit_ts, but I will check on this.

> patch-003
> 3) alter_sub_conflictlogdestination():
> + relid = get_relname_relid(relname, PG_CONFLICT_NAMESPACE);
> + if (OidIsValid(relid))
> + {
> + /* Existing table from upgrade */
> + Assert(IsBinaryUpgrade);
> + }
> + else
> + {
> + relid = create_conflict_log_table(sub->oid, sub->name,
> + sub->owner);
> + }
> +
>
> I think Assert() above should be replaced with an ERROR similar to the
> one in create_conflict_log_table().
> One possible scenario is that a user manually creates
> pg_conflict.pg_conflict_log_<subid> with allow_system_table_mods = on.
> In that case:
> -- Assert builds will hit the assertion.
> -- Non-assert builds will silently skip table creation, and later CLT
> operations may fail in unexpected ways because the user-created table
> definition may not match the expected schema.
>
> Before patch-003, create_conflict_log_table() correctly reported an
> error if a table with the same name already existed, which seems
> safer.
>
> 4) 002_pg_dump.pl
> - WITH (connect = false, origin = any, streaming = on);',
> + WITH (connect = false, origin = any, streaming = on,
> conflict_log_destination= table);',
>
> typo: space before "=".
> /conflict_log_destination= table/conflict_log_destination = table/
> ~~~

Vignesh can you look into this?

> patch-005
> 5) doc/src/sgml/ddl.sgml
> + direct user manipulation. Unlike <literal>pg_catalog</literal>, the
> + <literal>pg_catalog</literal> schema is not implicitly included in the
> + search path, so objects within it must be referenced explicitly or by
>
> typo : repeated "pg_catalog"
> It should be: Unlike <literal>pg_catalog</literal>, the
> <literal>pg_conflict</literal> schema is not implicitly...
> ~~~
>
> [1] https://www.postgresql.org/message-id/CABdArM5N0LgkHc%2BJOKcbDjpx%3D0hdWDx%2BJ7y%3DUS2apEEmi07oyw%40mail.gmail.com
>

I have noted the doc changes, will fix after we agree on initial patches.

--
Regards,
Dilip Kumar
Google