| From: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
|---|---|
| To: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
| Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(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>, shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Subject: | Re: Proposal: Conflict log history table for Logical Replication |
| Date: | 2026-07-02 10:03:38 |
| Message-ID: | CAJpy0uCcbSfNmM2dfQECjto6bJfSKzJX0WEsFHr5Q-WE3UeYnw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Jul 2, 2026 at 1:39 PM Dilip Kumar <dilipbalaut(at)gmail(dot)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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Ewan Young | 2026-07-02 10:07:26 | jsonpath .double() is a no-op on JSON numbers (differs from string input) |
| Previous Message | Amit Kapila | 2026-07-02 09:55:13 | Re: Fix publisher-side sequence permission reporting |