Re: Proposal: Conflict log history table for Logical Replication

On Tue, Jun 2, 2026 at 10:51 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Mon, Jun 1, 2026 at 5:53 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
> >
> > On Sat, May 30, 2026 at 1:42 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > >
> > > In latest patch set I have fixed Nisha's comments by creating a toast
> > > table, a separate patch
> > > (v43-0005-Create-conflict-log-table-after-inserting-subscr.patch)
> > > attached for creating conflict log table after inserting subscription
> > > row.
> > >
> >
> > Thanks for the patches. Please find a couple of comments on v43:
> >
> > 1) A non-superuser cannot read the new columns 'subconflictlogrelid'
> > and 'subconflictlogdest' from pg_subscription.
> > A comment in system_views.sql say:
> > "-- All columns of pg_subscription except subconninfo are publicly readable."
> >
> > I think we should grant public access to these new columns as well.
> >
> > 2) patch-002: conflict.c
> >
> > -const ConflictLogColumnDef ConflictLogSchema[] = {
> > +StaticAssertDecl(lengthof(ConflictLogDestNames) == 3,
> > + "ConflictLogDestNames length mismatch");
> > +
> >
> > Should we use "CONFLICT_LOG_DEST_ALL + 1" instead of the hard-coded value "3"?
> >
> > The attached diff fixes the above items, along with a few indentation
> > and whitespace. Please consider it if you agree with the changes.
>
> PFA, latest version of patch
>
> Changes:
> 1. Includes Nisha's suggestion from v43_comments_nisha.patch
> 2. Include Amit's suggestion from v43-0001_amit.1.patch
> 3. Include Amit's suggestion: change GetLogDestination to
> GetConflictLogDest(), drop_sub_conflict_log_table() to
> drop_sub_conflict_log_table(), CONFLICTS_LOGGED_TO_FILE to
> CONFLICTS_LOGGED_TO_LOG and renamed IsConflictNamespace() to
> IsConflictLogTableNamespace()
> 4. Include Vignesh's fix Parallel_apply_failure.patch
> 5. Change conflict log table name back to conflict_log_table_<subid>
> 6. Changed acl to exclude INSERT/UPDATE/USAGE as it was there in initial patch
> 7. Merged 0002 to 0001 and 0004 to 0003 after review
>
> Open
> 1. Vignesh please rebase upgrade and \dRs+ page
> 2. I will review and update the doc patch accordingly

I missed to commit conflict table name change in 0002 due to that
there was a test failure, PFA updated version

--
Regards,
Dilip Kumar
Google