On Mon, Sep 21, 2026 at 2:03 PM Zhijie Hou <[email protected]> wrote: > > On Fri, Sep 18, 2026 at 8:55 PM Amit Kapila <[email protected]> wrote: > > Fixed in the attached. Apart from this I changed multiple comments to > > make those clear. One notable change is, I moved the newly added > > boolean after localindexoid as it reads better there because then we > > don't need to forward reference the fields. For back-branches, if it > > needs to be moved to an earlier location then we can do that in those > > versions but for HEAD and 19, the new location seems better. > > > > Also, shall we keep just one test, say Drop Index Concurrently instead > > of two as both tests do the same thing in a slightly different way? I > > have not done that but if you agree please update the patch > > accordingly. > > I also think one DROP INDEX CONCURRENTLY test is sufficient to cover this bug. > > The patch looks mostly good to me, I just have a few minor comments: > > 1. > > extern bool RelationFindReplTupleByIndex(Relation rel, Oid idxoid, > + bool isIdxSafeToSkipDuplicates, > > Shall we use a shorter parameter name, like skipduplicates, to stay consistent > with the other parameters of this function? >
LGTM. > 2. > > + * XXX This can disagree with the index the apply worker searched by, > + * see FindReplTupleInLocalRel(). It may not even be one that > + * ExecOpenIndices() locked. > */ > Oid replica_index = GetRelationIdentityOrPK(localrel); > > Just in case we want to fix this in the current patch, I think one option that > avoids adding a new parameter to multiple public functions would be to cache > the > replica identity index OID in remote_ctx and expose a function to retrieve it. > We could then call that function here instead of GetRelationIdentityOrPK(). > > Otherwise, we would need to add a new index OID parameter to > ExecSimpleRelationUpdate, even though the identity index isn't otherwise used > there - and ReportApplyConflict would need a new parameter as well. > I think this is a lower priority case. Let's do this separately. -- With Regards, Amit Kapila.
