Re: Fix apply worker crash when subscriber table has only a deferrable primary key - Mailing list pgsql-hackers

On Thu, Oct 1, 2026 at 8:12 AM Nisha Moond <nisha.moond412@gmail.com> wrote:
>

Few comments:
============
1.
- * If the relation has a replica identity key or a primary key that is
- * unusable for locating deleted tuples (see
- * IsIndexUsableForFindingDeletedTuple), a full table scan becomes
- * necessary. In such cases, comparing the entire tuple is not required,
- * since the remote tuple might not include all column values. Instead,
- * the indexed columns alone are sufficient to identify the target tuple
- * (see logicalrep_rel_mark_updatable).
+ * We get here when the caller's index, if any, cannot be used for
+ * locating deleted tuples (see IsIndexUsableForFindingDeletedTuple). If
+ * that index is the replica identity or primary key, the remote tuple
+ * might not include all column values, but the index's key columns alone
+ * are sufficient to identify the target tuple. Otherwise, the remote
+ * relation has REPLICA IDENTITY FULL, so compare the entire tuple.

Why is this comment changed? I find the previous comment better.

2.
+ * If 'identidxoid' is valid, it must be the replica identity or primary key
+ * index, and only its key columns are compared. Otherwise, all columns are
+ * compared.


Saying must here may not be good as we can't have an assert for it?

3.
+ * Pass the index only if it is the replica identity or primary key,
+ * so that its key columns are compared. Use the relation map's choice
+ * rather than looking it up again, since concurrent DDL may have
+ * changed the relation's replica identity.
+ */
+ return RelationFindDeletedTupleInfoSeq(localrel,

Is this comment really required? IT can be inferred easily from the code.

4. I think keeping a deferrable key test case only is sufficient.

--
With Regards,
Amit Kapila.