| From: | vignesh C <vignesh21(at)gmail(dot)com> |
|---|---|
| To: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
| Cc: | Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, shveta malik <shveta(dot)malik(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-04-23 14:06:36 |
| Message-ID: | CALDaNm20PDtmG2E3qaTC+YuL5twv+c9k573wL3sb=OwgmZphxQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, 21 Apr 2026 at 15:21, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Mon, Apr 20, 2026 at 7:31 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
> >
> > On Mon, Apr 20, 2026 at 5:25 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
> > >
> > > On Thu, Apr 16, 2026 at 9:24 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > > >
> > > > On Fri, Apr 10, 2026 at 4:54 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
> > > > >
> > > > > Hi Dilip,
> > > > > I’m planning to review/test the feature patches, but they don’t apply
> > > > > cleanly on the latest HEAD. Could you please rebase them?
> > > > >
> > > > Thanks Nisha. Here is the rebased version
> > >
> > > Thank you Dilip. I reviewed the patches and did basic testing.
> > >
> > > Here are a few comments for the first two patches -
> >
> > Please find further comments for the document patch-0003:
> >
> > 1) File: /doc/src/sgml/logical-replication.sgml
> > + <xref linkend="logical-replication-conflict-log-schema"/>.
> > + </para>
> > +
> > + <table id="logical-replication-conflict-log-schema">
> > + <title>Conflict Log Table Schema</title>
> >
> > The "replica_identity" column is missing from the schema table in docs.
> > ~~~
> >
> > Few minor comments:
> > 2) File: /doc/src/sgml/logical-replication.sgml: couple of typos in
> > below change-
> > - The log format for logical replication conflicts is as follows:
> > + The <link linkend="sql-createsubscription-params-with-conflict-log-destination"><literal>conflict_log_destination</literal></link>
> > + parameter can automatically creates a dedicated conflict log
> > table. This table is created in the dedicated
> > + <literal>pg_conflict</literal> namespace. The name of the
> > conflict log table
> > + is <literal>pg_conflict_<subid></literal>. The predefined
> > schema of this table is
> > + detailed in
> > + <xref linkend="logical-replication-conflict-log-schema"/>.
> > + </para>
> >
> > 2a) "can automatically creates" / "automatically creates"
> > 2b) there are double spaces after full stop.
> > "conflict log table. This table "
> > " namespace. The name of the conflict"
> > ~~~
> >
> > 3) Commit message typo -
> > Doccumentation patch / Documentation patch
> > ~~~
> >
> > 4) File: /doc/src/sgml/ref/alter_subscription.sgml
> > + When the <link
> > linkend="sql-createsubscription-params-with-conflict-log-destination"><literal>conflict_log_destination</literal></link>
> > + parameter is set to <literal>table</literal> or
> > <literal>all</literal>, the system
> > + automatically creates the internal conflict log table if it
> > does not already
> > + exist.
> >
> > The phrase “if it does not already exist” seems misleading here. The
> > table shouldn’t pre-exist, as it’s always dropped and recreated when
> > conflict_log_destination changes. Let me know if I’m missing any case
> > where it could pre-exist.
> > ~~~
>
> Thanks Nisha for the review. All your comments make sense so I have
> fixed them in the latest patch.
The v27 patch required a rebase due to a recent commit. Attached is
the rebased version along with the rebased version of the 'Preserve
conflict log destination and subscription OID for subscriptions' patch
which was not included in the last few versions.
Regards,
Vignesh
| Attachment | Content-Type | Size |
|---|---|---|
| v28-0004-Preserve-conflict-log-destination-and-subscripti.patch | application/octet-stream | 24.7 KB |
| v28-0002-Implement-the-conflict-insertion-infrastructure-.patch | application/octet-stream | 28.6 KB |
| v28-0001-Add-configurable-conflict-log-table-for-Logical-.patch | application/octet-stream | 118.3 KB |
| v28-0003-Documentation-patch.patch | application/octet-stream | 10.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Marcos Pegoraro | 2026-04-23 14:09:11 | Re: Adding an explaining title to Notes on SGML |
| Previous Message | vignesh C | 2026-04-23 13:56:18 | Re: StringInfo fixes, v19 edition. Plus a few oddities |