On Mon, Apr 20, 2026 at 5:25 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Thu, Apr 16, 2026 at 9:24 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Fri, Apr 10, 2026 at 4:54 PM Nisha Moond <nisha.moond412@gmail.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
