Thread
-
Re: Proposal: Conflict log history table for Logical Replication
Dilip Kumar <dilipbalaut@gmail.com> — 2025-12-23T12:22:14Z
On Tue, Dec 23, 2025 at 5:18 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Tue, Dec 23, 2025 at 10:55 AM shveta malik <shveta.malik@gmail.com> wrote: > > > > On Mon, Dec 22, 2025 at 9:11 PM Dilip Kumar <dilipbalaut@gmail.com> wrote: > > > > > > On Mon, Dec 22, 2025 at 3:09 PM shveta malik <shveta.malik@gmail.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