On Mon, Apr 20, 2026 at 5:25 PM Nisha Moond <[email protected]> wrote: > > On Thu, Apr 16, 2026 at 9:24 PM Dilip Kumar <[email protected]> wrote: > > > > On Fri, Apr 10, 2026 at 4:54 PM Nisha Moond <[email protected]> > > 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
