Re: Proposal: Conflict log history table for Logical Replication

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

In response to

Browse pgsql-hackers by date

  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