Re: Fix apply worker crash when subscriber table has only a deferrable primary key

On Wed, 30 Sept 2026 at 16:05, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
>
> On Wed, Sep 30, 2026 at 2:47 PM Hayato Kuroda (Fujitsu)
> <kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
> >
> > Hi Amit,
> >
> > > The caller of RelationFindDeletedTupleInfoSeq() already has
> > > information localindexoid/idxisreplident, why can't we use those
> > > values instead of computing the same information again? I am afraid
> > > that computing such an information again could lead to symptoms what
> > > we fixed in the recent commit
> > > ad36e3608c8cb6f0848737ec81e548d4d3a0af3c.
> >
> > Your point meant not to get the info from the relcache because it can be
> > invalidated by the concurrent DDLs, right? I think it's possible, but the
> > additional computation might be needed since bitmapset for key columns are not
> > cached on the relmap now. Attached top-up patch implemented the idea, can you
> > see it's same as your expectation?
> > Test code just showed my understanding, not intended to be included for now.
> >
>
> My understanding is also the same. Thanks for the patch; I’ve verified the fix.
>
> Here is the updated version, merged with v3-0001.

Thanks for the patch, one suggestion:
The two relcache lookups that previously obtained the replica identity
columns and, as a fallback, the primary key columns:
indexbitmap = RelationGetIndexAttrBitmap(rel,
INDEX_ATTR_BITMAP_IDENTITY_KEY);

if (!indexbitmap)
indexbitmap = RelationGetIndexAttrBitmap(rel,
INDEX_ATTR_BITMAP_PRIMARY_KEY);

are now replaced by deriving the columns from idxoid:
if (OidIsValid(idxoid))
{
Relation idxrel = index_open(idxoid, AccessShareLock);
...
}

The idxoid is obtained from localindexoid/idxisreplident in the
relation map entry. These values are populated by
FindLogicalRepLocalIndex(), which uses GetRelationIdentityOrPK():
Oid
GetRelationIdentityOrPK(Relation rel)
{
Oid idxoid = RelationGetReplicaIndex(rel);

if (!OidIsValid(idxoid))
idxoid = RelationGetPrimaryKeyIndex(rel, false);

return idxoid;
}

Thus, the same identity-index-then-primary-key fallback logic is
already handled when the relation map entry is built. A valid idxoid
represents either the replica identity index or the primary key index,
so the removed relcache lookups do not result in any loss of
functionality.

If you are ok, can we include something like this in the commit
message, as it makes it clear why the old lookups can be removed and
helps explain the change for anyone reviewing the commit later.

The changes look good overall.

Regards,
Vignesh