Hi, On Fri, Oct 2, 2026 at 5:29 AM Amit Kapila <[email protected]> wrote: > > On Thu, Oct 1, 2026 at 8:12 AM Nisha Moond <[email protected]> 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.
Agreed. I reverted it to the original version and only adjusted it slightly to mention the passed-in index. > > 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? Removed this word. > > 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. I also think this is not required, removed. > > 4. I think keeping a deferrable key test case only is sufficient. I merged a deferrable key test from 0002 into 0001 in this version. Here is the V7 patch that addressed above comments. I confirmed it applies cleanly on all required branches and the test passed. Best Regards, Zhijie Hou
v7-0001-Use-the-relation-map-s-index-when-searching-delet.patch
Description: Binary data
