Re: Proposal: Conflict log history table for Logical Replication

From: vignesh C <vignesh21(at)gmail(dot)com>
To: Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>
Cc: shveta malik <shveta(dot)malik(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Peter Smith <smithpb2250(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 <shvetamalik(at)gmail(dot)com>
Subject: Re: Proposal: Conflict log history table for Logical Replication
Date: 2026-05-19 14:00:43
Message-ID: CALDaNm1yTG2JntdiAAsEzT8XCe8ifLAZjBffnDJP=i5JxKmeEA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, 15 May 2026 at 15:59, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
>
> Thanks for the patches. Please find below comments for v34 patch-set.
>
> patch-003:
> 4) conflict.c: ReportApplyConflict()
> + bool log_dest_clt = false;
> + bool log_dest_logfile;
>
> log_dest_logfile should also be initialized to false, since for dest
> == CONFLICT_LOG_DEST_TABLE, it is never assigned.

It is not required to be initialized now as it is being assigned
before used in this function now.

> 5) worker_internal.h
> extern PGDLLIMPORT List *table_states_not_ready;
>
> +extern XLogRecPtr remote_final_lsn;
> +extern TimestampTz remote_commit_ts;
> +extern TransactionId remote_xid;
>
> Should these new declarations also use PGDLLIMPORT?

I think these don't require PGDLLIMPORT as it will be used by the same
apply worker backend process.

Rest of the comments are handled, the attached v36 version patches
have the changes for the same.
Also the comment from [1] has been fixed in this version.

[1] - https://www.postgresql.org/message-id/CABdArM5XgHE4-HCryi54BxENgNqLDn81cMCUyqBdCeF9d3dbvA%40mail.gmail.com

Regards,
Vignesh

Attachment Content-Type Size
v36-0003-transfer-ownership.patch application/octet-stream 2.0 KB
v36-0004-Review-comment-fixes-for-transfer-ownership-patc.patch application/octet-stream 4.4 KB
v36-0005-Implement-the-conflict-insertion-infrastructure-.patch application/octet-stream 28.6 KB
v36-0001-Add-configurable-conflict-log-table-for-Logical-.patch application/octet-stream 121.3 KB
v36-0002-Review-comment-fixes-for-Add-configurable-confli.patch application/octet-stream 110.7 KB
v36-0006-Review-comment-fixes-for-Implement-the-conflict-.patch application/octet-stream 13.0 KB
v36-0007-Preserve-conflict-log-destination-and-subscripti.patch application/octet-stream 25.7 KB
v36-0009-Review-comment-fixes-for-Documentation-patch.patch application/octet-stream 2.2 KB
v36-0008-Documentation-patch.patch application/octet-stream 11.0 KB
v36-0010-Add-conflict-log-table-information-to-describe-s.patch application/octet-stream 77.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ilia Evdokimov 2026-05-19 14:29:56 Fix incorrect size check in statext_dependencies_deserialize
Previous Message Michael Paquier 2026-05-19 13:55:39 Re: Fix pg_stat_wal_receiver to show CONNECTING status