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

Dear Nisha,

> While testing another feature patch, I came across this base-code
> issue. If a subscriber table's only key is a DEFERRABLE primary key,
> and the published table does not use REPLICA IDENTITY FULL, then
> UPDATE and DELETE apply trips an assertion.

Good catch, I confirmed the same.

> The cause is that two code paths disagree.
> logicalrep_rel_mark_updatable() finds no replica identity bitmap and
> falls back to INDEX_ATTR_BITMAP_PRIMARY_KEY.
> Since commit 270af6f0df7 (pg17), that bitmap includes deferrable
> primary keys, so the relation is marked updatable.
> FindLogicalRepLocalIndex(), however, uses GetRelationIdentityOrPK(),
> which calls RelationGetPrimaryKeyIndex(rel, false) and rejects
> deferrable keys. So it returns InvalidOid.

The analysis looks correct to me.

> The attached patch makes mark_updatable() fall back to the primary key
> only when RelationGetPrimaryKeyIndex(rel, false) returns it, which
> matches the lookup path. With the same test, the subscriber will now
> hit an error:
> ERROR: logical replication target relation "public.t" has neither
> REPLICA IDENTITY index nor PRIMARY KEY and published relation does not
> have REPLICA IDENTITY FULL

I could not apply your patch on HEAD as-is, have you had some premise patches?
Anyway, I have one comment.

RelationFindDeletedTupleInfoSeq() also has a fallback code. Per my understanding,
the same tuple-detection rule should be used everywhere thus it also should be fixed,
right? Like attached.

[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).
*/
indexbitmap = RelationGetIndexAttrBitmap(rel,
INDEX_ATTR_BITMAP_IDENTITY_KEY);

/* fallback to PK if no replica identity */
if (!indexbitmap)
indexbitmap = RelationGetIndexAttrBitmap(rel,
INDEX_ATTR_BITMAP_PRIMARY_KEY);

Best regards,
Hayato Kuroda
FUJITSU LIMITED