Re: Proposal: Conflict log history table for Logical Replication

From: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: shveta malik <shveta(dot)malik(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>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Proposal: Conflict log history table for Logical Replication
Date: 2026-07-06 07:21:47
Message-ID: CAFiTN-vLKRRSDMwPGMvcBM1CRntXbsHARU-0e76PDk=-k5RO6A@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jul 6, 2026 at 11:42 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Fri, Jul 3, 2026 at 12:51 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > On Thu, Jul 2, 2026 at 3:33 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> > >
> > > On Thu, Jul 2, 2026 at 1:39 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > > >
> > > >
> > > > I rebased the remaining patches on top of HEAD. So far, I have run
> > > > pgindent and completed the doc merge for 0001. The 0002 and 0003 are
> > > > just rebased.
> > > >
> > >
> >
> > I am focusing on 001 alone.
> > I was verifying conflict insertion for all conflict_types in regular
> > worker flow (not parallel), I have just one comment there:
> >
> > For insert_exists conflict, both replica_identity (JSON) and
> > replica_identity_full (bool) are NULL. Is it conscious decision to
> > keep 'replica_identity_full' as NULL or shall we make this field as
> > 'bool NOT NULL DEFAULT false'. As per doc at [1], NULL value for
> > boolean represents 'unknown'
> >
>
> Displaying 'replica_identity_full' as false means replica_identity
> index is used and same is expected to be displayed as well.
>
> > If it is intentionaly kept NULL, do you think we should update below
> > doc to explain that it can be NULL for the cases where
> > replica_identity is not applicable.
> >
> > replica_identity_full boolean I
> > indicates whether replica_identity represents a full tuple (true) or
> > key values of a replica identity index (false).
> >
>
> I agree with the doc update for this case.
>
> I have one comment on 63-0001.
> *
> In 63-0001, it seems only a minor title change in docs:
> + <sect2 id="logical-replication-conflict-logging">
> + <title>Conflict logging</title>
> leads to the patch showing such a big change in
> logical-replication.sgml. Is that correct? If so, I suggest moving the
> logical-replication.sgml to a later doc-only patch as that will help
> in focusing the core code change. I understand that we need to update
> docs with the new section for logging conflict information in tables
> and that is important but we can handle it separately as it would
> require more thoughts on how to rearrange the current docs.

Attached patch fixes comments raised by Nisha, Shaveta and separates
out the logical-replication.sgml changes to a new patch.

--
Regards,
Dilip Kumar
Google

Attachment Content-Type Size
v64-0004-Add-conflict-log-table-information-to-describe-s.patch application/octet-stream 78.6 KB
v64-0001-Implement-the-conflict-insertion-infrastructure-.patch application/octet-stream 60.2 KB
v64-0003-Preserve-conflict-log-destination-and-subscripti.patch application/octet-stream 23.4 KB
v64-0002-Logical-replication-conflict-doc-only-patch.patch application/octet-stream 35.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ilmar Yunusov 2026-07-06 07:30:04 Re: File locks for data directory lockfile in the context of Linux namespaces
Previous Message Ilmar Yunusov 2026-07-06 06:47:34 Re: [RFC PATCH v0 0/7] Add EXPLAIN ANALYZE wait event reporting