Re: Proposal: Conflict log history table for Logical Replication

On Fri, 2 Jan 2026 at 12:06, shveta malik <shveta.malik@gmail.com> wrote:
>
> On Fri, Dec 26, 2025 at 8:58 PM vignesh C <vignesh21@gmail.com> wrote:
> >
> >
> > Here is a rebased version of the remaining patches.
> >
>
> Thank You Vignesh. Please find a few comments on 004:
>
>
> 1)
> IIUC, SubscriptionConflictrelIndexId is an unique index on sub-oid and
> conf-relid, but we use it only on relid as key. Why didn't  we create
> it only on 'conf-relid' alone? Using a composite unique index is
> guaranteed to give unique row only when all keys are used, but for a
> single key, a unique row is not guaranteed. In our case, it will be a
> unique row as conflict-relid is not shared, but still as an overall
> general concept, it may not be.

As we are searching only on subconflictlogrelid, index on
subconflictlogrelid is sufficient. I have modified it.

> 2)
> IsConflictLogTable():
> + if (OidIsValid(subform->subconflictlogrelid))
>
> Do we need this check? Since we’ve already performed an index access
> using subconflictlogrelid as the key, isn’t it guaranteed to always be
> valid?

It is not required, removed it

> 3)
> Please update the commit message to indicate that this patch makes CLT
> publishable if a publication is explicitly created on it, else few
> changes become very confusing due to unclear intent.

Included this in the commit message.

> 4)
> pg_relation_is_publishable():
>
>   /* Subscription conflict log tables are not published */
> - result = is_publishable_class(relid, (Form_pg_class) GETSTRUCT(tuple)) &&
> - !IsConflictLogTable(relid);
>
> Comment should be removed too.

Removed

> 5)
> We need to remove below comment:
>
>  * Note: Conflict log tables are not publishable.  However, we intentionally
>  * skip this check here because this function is called for every change and
>  * performing this check during every change publication is costly.  To ensure
>  * unpublishable entries are ignored without incurring performance overhead,
>  * tuples inserted into the conflict log table uses the HEAP_INSERT_NO_LOGICAL
>  * flag.  This allows the decoding layer to bypass these entries automatically.
>  */
> bool
> is_publishable_relation(Relation rel)

Removed

> 6)
> get_rel_sync_entry:
> + /* is this relation used for conflict logging? */
> + isconflictlogrel = IsConflictLogTable(relid);
>
> Shall we add a comment indicating the intent of change in this
> function. Something like:
>
> /*
>  * Check whether this is a conflict log table. If so, avoid publishing it via
>  * FOR ALL TABLES or FOR TABLES IN SCHEMA publications, but still allow it
>  * to be published through a publication explicitly created for this table.
>  */

Included

The attached v19 patch has the changes for the same.

Regards,
Vignesh