| 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 |
| 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 |