Thread

  1. Re: Proposal: Conflict log history table for Logical Replication

    shveta malik <shveta.malik@gmail.com> — 2025-12-29T06:02:38Z

    On Thu, Dec 25, 2025 at 1:10 PM Dilip Kumar <dilipbalaut@gmail.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