Re: Proposal: Conflict log history table for Logical Replication

On Fri, Jun 26, 2026 at 10:49 AM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Thu, 25 Jun 2026 at 19:15, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Thu, Jun 25, 2026 at 6:37 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > On Thu, Jun 25, 2026 at 6:30 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > > >
> > > > On Thu, Jun 25, 2026 at 4:57 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > > >
> > > >
> > > > > 2.
> > > > > @@ -2482,6 +2681,8 @@ DropSubscription(DropSubscriptionStmt *stmt,
> > > > > bool isTopLevel)
> > > > > deleteDependencyRecordsFor(SubscriptionRelationId, subid, false);
> > > > > deleteSharedDependencyRecordsFor(SubscriptionRelationId, subid, 0);
> > > > >
> > > > > + drop_sub_conflict_log_table(subid, subname, subconflictlogrelid);
> > > > >
> > > > > It would be better to drop the table before cleaning up the dependency
> > > > > record. Right now, it is okay even in current order because dependency
> > > > > removal is trying to remove where subid is depender.
> > > >
> > > > So whats your suggestion change it now or not? I feel either way is fine.
> > > >
> > >
> > > Let's change now itself.
> >
> > PFA, updated version of the patch, fixes all the comments of V57
>
> Thanks for the updated patches.
> Currently, we allow an empty string for conflict_log_destination:
> postgres=# CREATE SUBSCRIPTION sub1 CONNECTION 'dbname=postgres
> host=localhost port=5432' PUBLICATION pub1 WITH
> (conflict_log_destination = '');
> NOTICE: created replication slot "sub1" on publisher
> CREATE SUBSCRIPTION
>
> postgres=# SELECT subconflictlogdest FROM pg_subscription WHERE
> subname = 'sub1';
> subconflictlogdest
> --------------------
> log
> (1 row)
>
> However, this is inconsistent with the behavior of the other
> subscription options, which all reject an empty string. For example:
> postgres=# CREATE SUBSCRIPTION sub2 ... WITH (origin = '');
> ERROR: unrecognized origin value: ""
>
> postgres=# CREATE SUBSCRIPTION sub2 ... WITH (streaming = '');
> ERROR: streaming requires a Boolean value or "parallel"
>
> postgres=# CREATE SUBSCRIPTION sub2 ... WITH (copy_data = '');
> ERROR: copy_data requires a Boolean value
>
> postgres=# CREATE SUBSCRIPTION sub2 ... WITH (wal_receiver_timeout = '');
> ERROR: invalid value for parameter "wal_receiver_timeout": ""
>
> postgres=# CREATE SUBSCRIPTION sub2 ... WITH (max_retention_duration = '');
> ERROR: max_retention_duration requires an integer value
>
> postgres=# CREATE SUBSCRIPTION sub2 ... WITH (slot_name = '');
> ERROR: replication slot name "" is too short
>
> I think conflict_log_destination should behave consistently with the
> other subscription options and reject an empty string rather than
> silently treating it as log. If there is a specific reason for
> accepting '', it would be good to document that behavior.
>

I changed this in the latest version of the patch.

--
Regards,
Dilip Kumar
Google