Horton

Re: Proposal: Conflict log history table for Logical Replication

shveta malik <shveta.malik@gmail.com>

View thread
From: shveta malik <shveta.malik@gmail.com>
To: Dilip Kumar <dilipbalaut@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, shveta malik <shveta.malik@gmail.com>
Date: 2025-12-23T10:03:56Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Allow logical replication conflicts to be logged to a table.

  2. Avoid orphaned objects dependencies

On Mon, Dec 22, 2025 at 4:01 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> Done in V15

Thanks for the patches. A few comments on v15-002 for the part I have
reviewed so far:

1)
Defined twice:

+#define MAX_LOCAL_CONFLICT_INFO_ATTRS 5

+#define MAX_LOCAL_CONFLICT_INFO_ATTRS \
+ (sizeof(LocalConflictSchema) / sizeof(LocalConflictSchema[0]))


2)
GetConflictLogTableInfo:
+ *log_dest = GetLogDestination(MySubscription->logdestination);
+ conflictlogrelid = MySubscription->conflictrelid;
+
+ /* If destination is 'log' only, no table to open. */
+ if (*log_dest == CONFLICT_LOG_DEST_LOG)
+ return NULL;

We can get conflictlogrelid after the if-check for DEST_LOG.

3)
In ReportApplyConflict(), we form err_detail by calling
errdetail_apply_conflict(). But when dest is TABLE, we don't use
err_detail. Shall we skip creating it for dest=TABLE case?

4)
ReportApplyConflict():
+ /*
+ * Get both the conflict log destination and the opened conflict log
+ * relation for insertion.
+ */
+ conflictlogrel = GetConflictLogTableInfo(&dest);
+

We can move it after errdetail_apply_conflict(), closer to where we
actually use it.

5)
start_apply:
+ /* Open conflict log table and insert the tuple. */
+ conflictlogrel = GetConflictLogTableInfo(&dest);
+ if (ValidateConflictLogTable(conflictlogrel))
+ InsertConflictLogTuple(conflictlogrel);

We can have Assert here too before we call Validate:
Assert(dest == CONFLICT_LOG_DEST_TABLE || dest == CONFLICT_LOG_DEST_ALL);

6)
start_apply:
+ if (ValidateConflictLogTable(conflictlogrel))
+ InsertConflictLogTuple(conflictlogrel);
+ MyLogicalRepWorker->conflict_log_tuple = NULL;

InsertConflictLogTuple() already sets conflict_log_tuple to NULL.
Above is not needed.

thanks
Shveta