On Thu, Jul 2, 2026 at 1:39 PM Dilip Kumar <dilipbalaut@gmail.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.
>
Thanks, a few trivial comments for 001:
1)
logical-replication.sgml:
+ <sect2 id="logical-replication-conflict-logging">
+ <title>Conflict logging</title>
+ <para>
+ Additional logging is triggered, and the conflict statistics are
collected (displayed in the
Starting new section with 'Additional' seems odd. we can simply say:
Conflict logging is triggered, and conflict statistics are collected
(displayed in the
2)
IMO, to relate the 3 sections:
'29.8.1. Conflict logging', '29.8.2. Table-based logging' and '
29.8.3. File-based logging', we can add below line as well to
29.8.1, thus overall it will look like:
Conflict logging is triggered, and conflict statistics are collected
(displayed in the pg_stat_subscription_stats view) for the conflict
cases described below. Depending on the value of
conflict_log_destination, the conflict information is written to the
server log, a conflict log table, or both.
3)
create_subscription.sgml:
+ Records conflict details to both destinations
<literal>log</literal>
+ and <literal>table</literal>.
Either we can have:
Records conflict details to both the server log and the conflict log table.
OR
Records conflict details to both destinations: the server log and the
conflict log table
4)
conflict.c:
+#include "access/xact.h"
+#include "utils/memutils.h"
These are not needed.
5)
build_local_conflicts_json_array:
+ json_datum = heap_copy_tuple_as_datum(tuple, tupdesc);
+
+ /*
+ * Build the higher level JSON datum in format described in function
+ * header.
+ */
+ json_datum = DirectFunctionCall1(row_to_json, json_datum);
The first one is not json_datum, it is simply datum. Perhaps we shall
change the varibale for first one to 'Datum datum' like we have at all
other places in this file.
thanks
Shveta
