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.
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?
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.
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.
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)
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.
*/
thanks
Shveta
