Re: Proposal: Conflict log history table for Logical Replication

On Thu, Dec 25, 2025 at 1:10 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> Here is the patches I have changed by using IsSystemClass(), based on
> this many other things changed like we don't need to check for the
> temp schema and also the caller of create_conflict_log_table() now
> don't need to find the creation schema so it don't need to generate
> the relname so that part is also moved within
> create_conflict_log_table(). Fixed most of the comments given by
> Peter and Shveta, although some of them are still open e.g. the name
> of the conflict log table as of now I have kept as
> conflict_log_table_<subid> other options are
>
> 1. pg_conflict_<subid>
> 2. conflict_log_<subid>
> 3. sub_conflict_log_<subid>
>
> I prefer 3, considering it says this table holds subscription conflict
> logs. Thoughts?
>

I was checking how pg_toast does it. It creates tables with names:
"pg_toast_%u", relOid

We can do similar i.e., the schema name as pg_conflict and table name
as pg_conflict_<subid>. Thoughts?

Few comments on 001:

1)
It will be good to display conflict tablename in \dRs command

2)
postgres=# ALTER TABLE sch1.t3 set schema pg_toast;
ERROR: cannot move objects into or out of TOAST schema

But when we move to pg_conflict, it works. It should error out as well.
postgres=# ALTER TABLE sch1.t1 set schema pg_conflict;
ALTER TABLE

3)
Shall we LOG CLT creation and drop during create/alter sub?

4)
create_conflict_log_table()
+ /* Report an error if the specified conflict log table already exists. */
+ if (OidIsValid(get_relname_relid(relname, PG_CONFLICT_NAMESPACE)))
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_TABLE),
+ errmsg("relation \"%s.%s\" already exists",
+ get_namespace_name(PG_CONFLICT_NAMESPACE), relname)));

I am unable to think of a valid user-scenario when the above will be
hit. Do we need this as a user-error or simply an Assert or
internal-error will do?

5)
+ /*
+ * Establish an internal dependency between the conflict log table and the
+ * subscription. By using DEPENDENCY_INTERNAL, we ensure the table is
+ * automatically reaped when the subscription is dropped. This also
+ * prevents the table from being dropped independently unless the
+ * subscription itself is removed.
+ */
+ ObjectAddressSet(myself, RelationRelationId, relid);
+ ObjectAddressSet(subaddr, SubscriptionRelationId, subid);
+ recordDependencyOn(&myself, &subaddr, DEPENDENCY_INTERNAL);

Now that we have pg_conflict, which is treated similarly to a system
catalog, I’m wondering whether we actually need to maintain this
dependency to prevent the CLT table or schema from being dropped.
Also, given that this currently goes against the convention that a
shared object cannot be present in pg_depend, could DropSubscription()
and AlterSubscription() instead handle dropping the table explicitly
in required scenarios?

6)
+ descr => 'reserved schema for conflict tables',

Shall we say: 'reserved schema for subscription-specific conflict tables'

or anything better to include that it is subscription related?

thanks
Shveta