| From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
|---|---|
| To: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
| Cc: | vignesh C <vignesh21(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(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-01-12 05:29:18 |
| Message-ID: | CAHut+PuzB4gNYvqX9hb28KE0RK_xhU+2-=wUfL5OEVUCi92Hqw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Some review comments for patch v20-0003 (docs)
======
doc/src/sgml/logical-replication.sgml
(29.8. Conflicts)
1.
+ When the <link
linkend="sql-createsubscription-params-with-conflict-log-destination"><literal>conflict_log_destination</literal></link>
+ is set to <literal>table</literal> or <literal>all</literal>, the system
+ automatically creates a new table with a predefined schema to log conflict
+ details. This table is created in the dedicated
+ <literal>pg_conflict</literal> namespace. The name of the
conflict log table
+ is pg_conflict_<replaceable>subscription_oid</replaceable>. The
schema of this
+ table is detailed in
+ <xref linkend="logical-replication-conflict-log-schema"/>.
+ </para>
1a.
Instead of saying "When the conflict_log_destination is ...", maybe it
should say "When the conflict_log_destination parameter is ...".
That should be more consistent with the wording used elsewhere in this patch.
~
1b.
But, on the CREATE SUBSCRIPTION page, the table name is called:
<literal>pg_conflict_<subid></literal>
Both places should refer to the name using the same format -- the
CREATE SUBSCRIPTION way looked good to me.
~~~
2.
+ The conflicting row data, including the original local tuple and
+ the remote tuple, is stored in <type>JSON</type> columns
(<literal>local_tuple</literal>
+ and <literal>remote_tuple</literal>) for flexible querying and analysis.
+ </para>
As reported previously [1, comment #5], I didn't see any column called
'local_tuple' -- did you mean to refer to the elements of the
'local_conflicts' array here?
~~~
3.
A previous review comment [1, comment #6 ] suggesting making some
Chapter 29 supsection/s about "Logging conflicts" was not addressed.
You disagreed?
======
doc/src/sgml/ref/create_subscription.sgml
4.
A previous review comment [1, comment #13 ] suggesting adding some
links to refer to Chapter 29 for the logging details was not
addressed. You disagreed?
======
[1] my v19-0003 review -
https://www.postgresql.org/message-id/CAHut%2BPtu9-R6x5t%3D2aXdVUR-cjopGxYFEgOjHpUY1jsAfG1drA%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
| From | Date | Subject | |
|---|---|---|---|
| Next Message | li carol | 2026-01-12 05:36:15 | RE: Use correct macro for accessing offset numbers. |
| Previous Message | Peter Smith | 2026-01-12 05:26:23 | Re: Proposal: Conflict log history table for Logical Replication |