Re: Proposal: Conflict log history table for Logical Replication

On Fri, Nov 28, 2025 at 2:32 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Thu, Nov 27, 2025 at 5:50 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> >
> > I have fixed all these comments and also the comments of 0002, now I
> > feel we can actually merge 0001 and 0002, so I have merged both of
> > them.
> >
> > Now pending work status
> > 1) fixed review comments of 0003
> > 2) Run pgindent -- planning to do it after we complete the first level
> > of review
> > 3) Subscription TAP test for logging the actual conflicts
> >
>
> Thanks for the patch. A few observations:
>
> 1)
> It seems, as per LOG, 'key' and 'replica-identity' are different when
> it comes to insert_exists, update_exists and
> multiple_unique_conflicts, while I believe in CLT, key is
> replica-identity i.e. there are no 2 separate terms. Please see below:
>
> a)
> Update_Exists:
> 2025-11-28 14:08:56.179 IST [60383] ERROR: conflict detected on
> relation "public.tab1": conflict=update_exists
> 2025-11-28 14:08:56.179 IST [60383] DETAIL: Key already exists in
> unique index "tab1_pkey", modified locally in transaction 790 at
> 2025-11-28 14:07:17.578887+05:30.
> Key (i)=(40); existing local row (40, 10); remote row (40, 200);
> replica identity (i)=(20).
>
> postgres=# select conflict_type, key_tuple,local_tuple,remote_tuple
> from clt where conflict_type='update_exists';
> conflict_type | key_tuple | local_tuple | remote_tuple
> ---------------+-----------+-----------------+------------------
> update_exists | {"i":20} | {"i":40,"j":10} | {"i":40,"j":200}
>
> b)
> insert_Exists:
> ERROR: conflict detected on relation "public.tab1": conflict=insert_exists
> DETAIL: Key already exists in unique index "tab1_pkey", modified
> locally in transaction 767 at 2025-11-28 13:59:22.431097+05:30.
> Key (i)=(30); existing local row (30, 10); remote row (30, 10).
>
> postgres=# select conflict_type, key_tuple,local_tuple,remote_tuple from clt;
> conflict_type | key_tuple | local_tuple | remote_tuple
> ----------------+-----------+-----------------+-----------------
> insert_exists | | {"i":30,"j":10} | {"i":30,"j":10}
>
> case a) has key_tuple same as replica-identity of LOG
> case b) does not have replica-identity and thus key_tuple is NULL.
>
> Does that mean we need to maintain both key_tuple and RI separately in
> CLT? Thoughts?

Maybe we should then have a place for both key_tuple as well as
replica identity as we are logging, what others think about this case?

> 2)
> For multiple_unique_conflict (testcase is same as I shared earlier),
> it asserts here:
> CONTEXT: processing remote data for replication origin "pg_16390"
> during message type "INSERT" for replication target relation
> "public.conf_tab" in transaction 778, finished at 0/017E6DE8
> TRAP: failed Assert("MyLogicalRepWorker->conflict_log_tuple == NULL"),
> File: "conflict.c", Line: 749, PID: 60627
>
> I have not checked it, but maybe
> 'MyLogicalRepWorker->conflict_log_tuple' is left over from the
> previous few tests I tried?

Yeah, prepare_conflict_log_tuple() is called in loop and when there
are multiple tuple we need to collect all of the tuple before
inserting it at worker exit so the current code has a bug, I will see
how we can fix it, I think this also depends upon the other discussion
we are having related to how to insert multiple unique conflict.

--
Regards,
Dilip Kumar
Google