Re: Proposal: Conflict log history table for Logical Replication - Mailing list pgsql-hackers

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.

Thoughts?

--
With Regards,
Amit Kapila.