Re: Proposal: Conflict log history table for Logical Replication

On Fri, Dec 12, 2025 at 3:04 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Dec 11, 2025 at 7:49 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > 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.
> >
>
> What we need here is an equivalent of DEPENDENCY_INTERNAL for database
> objects. For example, consider following case:
> postgres=# create table t1(c1 int primary key);
> CREATE TABLE
> postgres=# \d+ t1
> Table "public.t1"
> Column | Type | Collation | Nullable | Default | Storage |
> Compression | Stats target | Description
> --------+---------+-----------+----------+---------+---------+-------------+--------------+-------------
> c1 | integer | | not null | | plain |
> | |
> Indexes:
> "t1_pkey" PRIMARY KEY, btree (c1)
> Publications:
> "pub1"
> Not-null constraints:
> "t1_c1_not_null" NOT NULL "c1"
> Access method: heap
> postgres=# drop index t1_pkey;
> ERROR: cannot drop index t1_pkey because constraint t1_pkey on table
> t1 requires it
> HINT: You can drop constraint t1_pkey on table t1 instead.
>
> Here, the PK index is created as part for CREATE TABLE operation and
> pk_index is not allowed to be dropped independently.
>
> > Although making an object dependent on global/shared objects is
> > possible for certain types of shared objects [2], this is not our main
> > objective.
> >
>
> As per my understanding from the above example, we need something like
> that only for shared object subscription and (internally created)
> table.
>

+1

~~

Few comments for v11:

1)
+#include "executor/spi.h"
+#include "replication/conflict.h"
+#include "utils/fmgroids.h"
+#include "utils/regproc.h"

subscriptioncmds.c compiles without the above inclusions.

2)
postgres=# create subscription sub3 connection '...' publication pub1
WITH(conflict_log_table='pg_temp.clt');
NOTICE: created replication slot "sub3" on publisher
CREATE SUBSCRIPTION

Should we restrict clt creation in pg_temp?

3)
+ /* Fetch the eixsting conflict table table information. */

typos: eixsting->existing,
table table -> table

4)
AlterSubscription():
+ values[Anum_pg_subscription_subconflictlognspid - 1] =
+ ObjectIdGetDatum(nspid);
+
+ if (relname != NULL)
+ values[Anum_pg_subscription_subconflictlogtable - 1] =
+ CStringGetTextDatum(relname);
+ else
+ nulls[Anum_pg_subscription_subconflictlogtable - 1] =
+ true;

Should we move the nspid setting inside 'if(relname != NULL)' block?

5)
Is there a way to reset/remove conflict_log_table? I did not see any
such handling in AlterSubscription? It gives error:

postgres=# alter subscription sub3 set (conflict_log_table='');
ERROR: invalid name syntax

6)
+char *
+get_subscription_conflict_log_table(Oid subid, Oid *nspid)
+{
+ HeapTuple tup;
+ Datum datum;
+ bool isnull;
+ char *relname = NULL;
+ Form_pg_subscription subform;
+
+ *nspid = InvalidOid;
+
+ tup = SearchSysCache1(SUBSCRIPTIONOID, ObjectIdGetDatum(subid));
+
+ if (!HeapTupleIsValid(tup))
+ return NULL;

Should we have elog(ERROR) here for cache lookup failure? Callers like
AlterSubscription, DropSubscription lock the sub entry, so it being
missing at this stage is not normal. I have not seen all the callers
though.

7)
+#include "access/htup.h"
+#include "access/skey.h"

+#include "access/table.h"
+#include "catalog/pg_attribute.h"
+#include "catalog/indexing.h"
+#include "catalog/namespace.h"
+#include "catalog/pg_namespace.h"
+#include "catalog/pg_type.h"

+#include "executor/spi.h"
+#include "utils/array.h"

conflict.c compiles without above inclusions.

thanks
Shveta