| 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>, vignesh C <vignesh21(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-23 12:22:14 |
| Message-ID: | CAFiTN-skBQAeuzuUd+PDK0Gqc8g+4x9ypBMwJhOrmW8ZCFKGSA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, Dec 23, 2025 at 5:18 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Tue, Dec 23, 2025 at 10:55 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > On Mon, Dec 22, 2025 at 9:11 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > >
> > > On Mon, Dec 22, 2025 at 3:09 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> > >
> > > I think this needs more thought, others can be fixed.
> > >
> > > > 2)
> > > > postgres=# drop schema shveta cascade;
> > > > NOTICE: drop cascades to subscription sub1
> > > > ERROR: global objects cannot be deleted by doDeletion
> > > >
> > > > Is this expected? Is the user supposed to see this error?
> > > >
> > > See below code, so this says if the object being dropped is the
> > > outermost object (i.e. if we are dropping the table directly) then it
> > > will disallow dropping the object on which it has INTERNAL DEPENDENCY,
> > > OTOH if the object is being dropped via recursive drop (i.e. the table
> > > is being dropped while dropping the schema) then object on which it
> > > has INTERNAL dependency will also be added to the deletion list and
> > > later will be dropped via doDeletion and later we are getting error as
> > > subscription is a global object. I thought maybe we can handle an
> > > additional case that the INTERNAL DEPENDENCY, is on subscription the
> > > disallow dropping it irrespective of whether it is being called
> > > directly or via recursive drop but then it will give an issue even
> > > when we are trying to drop table during subscription drop, we can make
> > > handle this case as well via 'flags' passed in findDependentObjects()
> > > but need more investigation.
> > >
> > > Seeing this complexity makes me think more on is it really worth it to
> > > maintain this dependency? Because during subscription drop we anyway
> > > have to call performDeletion externally because this dependency is
> > > local so we are just disallowing the conflict table drop, however the
> > > ALTER table is allowed so what we are really protecting by protecting
> > > the table drop, I think it can be just documented that if user try to
> > > drop the table then conflict will not be inserted anymore?
> > >
> > > findDependentObjects()
> > > {
> > > ...
> > > switch (foundDep->deptype)
> > > {
> > > ....
> > > case DEPENDENCY_INTERNAL:
> > > * 1. At the outermost recursion level, we must disallow the
> > > * DROP. However, if the owning object is listed in
> > > * pendingObjects, just release the caller's lock and return;
> > > * we'll eventually complete the DROP when we reach that entry
> > > * in the pending list.
> > > }
> > > }
> > >
> > > [1]
> > > postgres[1333899]=# select * from pg_depend where objid > 16410;
> > > classid | objid | objsubid | refclassid | refobjid | refobjsubid | deptype
> > > ---------+-------+----------+------------+----------+-------------+---------
> > > 1259 | 16420 | 0 | 2615 | 16410 | 0 | n
> > > 1259 | 16420 | 0 | 6100 | 16419 | 0 | i
> > > (4 rows)
> > >
> > > 16420 -> conflict_log_table_16419
> > > 16419 -> subscription
> > > 16410 -> schema s1
> > >
> >
> > One approach could be to use something similar to
> > PERFORM_DELETION_SKIP_EXTENSIONS in our case, but only for recursive
> > drops. The effect would be that 'DROP SCHEMA ... CASCADE' would
> > proceed without error, i.e., it would drop the tables as well without
> > including the subscription in the dependency list. But if we try to
> > drop a table directly (e.g., DROP TABLE CLT), it will still result in:
> > ERROR: cannot drop table because subscription sub1 requires it
> >
>
> I think this way of allowing dropping the conflict table without
> caring for the parent object (subscription) is not a good idea. How
> about creating a dedicated schema, say pg_conflict for the purpose of
> storing conflict tables? This will be similar to the pg_toast schema
> for toast tables. So, similar to that each database will have a
> pg_conflict schema. It prevents the "orphan" problem where a user
> accidentally drops the logging schema but the Subscription is still
> trying to write to it. pg_dump needs to ignore all system schemas
> EXCEPT pg_conflict. This ensures the history is preserved during
> migrations while still protecting the tables from accidental user
> deletion. About permissions, I think we need to set the schema
> permissions so that USAGE is public (so users can SELECT from their
> logs) but CREATE is restricted to the superuser/subscription owner. We
> may need to think some more about permissions.
>
> I also tried to reason out if we can allow storing the conflict table
> in pg_catalog but here are a few reasons why it won't be a good idea.
> I think by default, pg_dump completely ignores the pg_catalog schema.
> It assumes pg_catalog contains static system definitions (like
> pg_class, pg_proc, etc.) that are re-generated by the initdb process,
> not user data. If we place a conflict table in pg_catalog, it will not
> be backed up. If a user runs pg_dump/all to migrate to a new server,
> their subscription definition will survive, but their entire history
> of conflict logs will vanish. Also from the permissions angle, If a
> user wants to write a custom PL/pgSQL function to "retry" conflicts,
> they might need to DELETE rows from the conflict table after fixing
> them. Granting DELETE permissions on a table inside pg_catalog is
> non-standard and often frowned upon by security auditors. It blurs the
> line between "System Internals" (immutable) and "User Data" (mutable).
> So, in short a separate pg_conflict schema appears to be a better solution.
Yeah that makes sense. Although I haven't thought about all cases
whether it can be a problem anywhere, but meanwhile I tried
prototyping with this and it behaves what we want.
postgres[1651968]=# select * from pg_conflict.conflict_log_table_16406 ;
relid | schemaname | relname | conflict_type | remote_xid |
remote_commit_lsn | remote_commit_ts | remote_origin |
replica_identity | remote_tuple
|
local_conflicts
-------+------------+---------+-----------------------+------------+-------------------+-------------------------------+---------------+------------------+----------------
+------------------------------------------------------------------------------------------------------------------------------------
16385 | public | test | update_origin_differs | 761 |
0/01760BD8 | 2025-12-23 11:08:30.583816+00 | pg_16406 |
{"a":1} | {"a":1,"b":20}
| {"{\"xid\":\"772\",\"commit_ts\":\"2025-12-23T11:08:25.568561+00:00\",\"origin\":null,\"key\":null,\"tuple\":{\"a\":1,\"b\":10}}"}
(1 row)
-- Case1: Alter is not allowed
postgres[1651968]=# ALTER TABLE pg_conflict.conflict_log_table_16406
ADD COLUMN a int;
ERROR: 42501: permission denied: "conflict_log_table_16406" is a system catalog
LOCATION: RangeVarCallbackForAlterRelation, tablecmds.c:19634
-- Case2: drop is not allowed
postgres[1651968]=# drop table pg_conflict.conflict_log_table_16406;
ERROR: 42501: permission denied: "conflict_log_table_16406" is a system catalog
LOCATION: RangeVarCallbackForDropRelation, tablecmds.c:1803
--Case3: Drop subscription drops it internally
postgres[1651968]=# DROP SUBSCRIPTION sub ;
NOTICE: 00000: dropped replication slot "sub" on publisher
LOCATION: ReplicationSlotDropAtPubNode, subscriptioncmds.c:2470
DROP SUBSCRIPTION
postgres[1651968]=# \d pg_conflict.conflict_log_table_16406
Did not find any relation named "pg_conflict.conflict_log_table_16406".
--
Regards,
Dilip Kumar
Google
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Peter Eisentraut | 2025-12-23 13:15:09 | Change some Datum to void * for opaque pass-through pointer |
| Previous Message | Heikki Linnakangas | 2025-12-23 12:07:48 | Re: Visibility bug in tuple lock |