| From: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Cc: | shveta malik <shveta(dot)malik(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> |
| Subject: | Re: Proposal: Conflict log history table for Logical Replication |
| Date: | 2025-12-11 14:19:29 |
| Message-ID: | CAFiTN-vrKc6OWzrg6yvpwYcj79k=zkrDp3uwiZzjwrWLJAq6tw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Dec 11, 2025 at 5:57 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Dec 11, 2025 at 5:10 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Thu, Dec 11, 2025 at 5:04 PM shveta malik <shveta(dot)malik(at)gmail(dot)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
2. Test case for conflit insertion
3. Documentation patch
--
Regards,
Dilip Kumar
Google
| Attachment | Content-Type | Size |
|---|---|---|
| v11-0001-Add-configurable-conflict-log-table-for-Logical-.patch | application/octet-stream | 123.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Peter Eisentraut | 2025-12-11 14:43:36 | Re: [PATCH] Fix severe performance regression with gettext 0.20+ on Windows |
| Previous Message | Euler Taveira | 2025-12-11 13:59:07 | Re: [PATCH] Add pg_get_database_ddl() function to reconstruct CREATE DATABASE statement |