Re: Proposal: Conflict log history table for Logical Replication

On Thu, 11 Dec 2025 at 19:50, Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Thu, Dec 11, 2025 at 5:57 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Thu, Dec 11, 2025 at 5:10 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > On Thu, Dec 11, 2025 at 5:04 PM shveta malik <shveta.malik@gmail.com> wrote:
> > >
> > > > 2)
> > > > When we do below:
> > > > alter subscription sub1 SET (conflict_log_table=clt2);
> > > >
> > > > the previous conflict log table is dropped. Is this behavior
> > > > intentional and discussed/concluded earlier? It’s possible that a user
> > > > may want to create a new conflict log table for future events while
> > > > still retaining the old one for analysis. If the subscription itself
> > > > is dropped, then dropping the CLT makes sense, but I’m not sure this
> > > > behavior is intended for ALTER SUBSCRIPTION.  I do understand that
> > > > once we unlink CLT from subscription, later even DROP subscription
> > > > cannot drop it, but user can always drop it when not needed.
> > > >
> > > > If we plan to keep existing behavior, it should be clearly documented
> > > > in a CAUTION section, and the command should explicitly log the table
> > > > drop.
> > >
> > > Yeah we discussed this behavior and the conclusion was we would
> > > document this behavior and its user's responsibility to take necessary
> > > backup of the conflict log table data if they are setting a new log
> > > table or NONE for the subscription.
> > >
> >
> > +1. If we don't do this then it will be difficult to track for
> > postgres or users the previous conflict history tables.
>
> Right, it makes sense.
>
> Attached patch fixed most of the open comments
> 1) \dRs+ now show the schema qualified name
> 2) Now key_tuple and replica_identify tuple both are add in conflict
> log tuple wherever applicable
> 3) Refactored the code so that we can define the conflict log table
> schema only once in the header file and both create_conflict_log_table
> and ValidateConflictLogTable use it.
>
> I was considering the interdependence between the subscription and the
> conflict log table (CLT). IMHO, it would be logical to establish the
> subscription as dependent on the CLT. This way, if someone attempts to
> drop the CLT, the system would recognize the dependency of the
> subscription and prevent the drop unless the subscription is removed
> first or the CASCADE option is used.
>
> However, while investigating this, I encountered an error [1] stating
> that global objects are not supported in this context. This indicates
> that global objects cannot be made dependent on local objects.
> Although making an object dependent on global/shared objects is
> possible for certain types of shared objects [2], this is not our main
> objective.
>
> We do not need to make the CLT dependent on the subscription because
> the table can be dropped when the subscription is dropped anyway and
> we are already doing it as part of drop subscription as well as alter
> subscription when CLT is set to NONE or a different table. Therefore,
> extending the functionality of shared dependency is unnecessary for
> this purpose.
>
> Thoughts?
>
> [1]
> doDeletion()
> {
> ....
> /*
> * These global object types are not supported here.
> */
> case AuthIdRelationId:
> case DatabaseRelationId:
> case TableSpaceRelationId:
> case SubscriptionRelationId:
> case ParameterAclRelationId:
> elog(ERROR, "global objects cannot be deleted by doDeletion");
> break;
> }
>
> [2]
> typedef enum SharedDependencyType
> {
> SHARED_DEPENDENCY_OWNER = 'o',
> SHARED_DEPENDENCY_ACL = 'a',
> SHARED_DEPENDENCY_INITACL = 'i',
> SHARED_DEPENDENCY_POLICY = 'r',
> SHARED_DEPENDENCY_TABLESPACE = 't',
> SHARED_DEPENDENCY_INVALID = 0,
> } SharedDependencyType;
>
> Pending Items are:
> 1. Handling dump/upgrade

The attached patch has the changes for handling dump. This works on
top of v11 version, it does not work on v12 because of the issue
reported at [1]. Currently the upgrade does not work because of the
existing issue which is being tracked at [2], upgrade works with the
patch attached at [2].

[1] - https://www.postgresql.org/message-id/CALDaNm1zEYoSdf2Ns-%3DUJRw95E5sbfpB0oaNUWtRJN27Q1Knhw%40mail.gmail.com
[2] - https://www.postgresql.org/message-id/CALDaNm2x3rd7C0_HjUpJFbxpAqXgm%3DQtoKfkEWDVA8h%2BJFpa_w%40mail.gmail.com

Regards,
Vignesh