Hi Hackers,
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.
Testcase to trigger the assert:
Publisher:
CREATE TABLE t (a int PRIMARY KEY, b text);
CREATE PUBLICATION p FOR TABLE t;
INSERT INTO t VALUES (1, 'one');
Subscriber:
CREATE TABLE t (a int, b text, PRIMARY KEY (a) DEFERRABLE);
CREATE SUBSCRIPTION s CONNECTION '...' PUBLICATION p;
Publisher:
UPDATE t SET b = 'two' WHERE a = 1;
Subscriber:
TRAP: failed Assert("OidIsValid(localidxoid) || (remoterel->replident
== REPLICA_IDENTITY_FULL)"), File: "worker.c", Line: 3256, PID: 88525
0 postgres 0x0000000103741b08
ExceptionalCondition + 216
1 postgres 0x0000000103417514
FindReplTupleInLocalRel + 144
2 postgres 0x0000000103418180
apply_handle_update_internal + 168
3 postgres 0x000000010340fad4
apply_handle_update + 904
4 postgres 0x000000010340efe0 apply_dispatch + 144
5 postgres 0x0000000103412ed0
LogicalRepApplyLoop + 836
6 postgres 0x0000000103412ac0 start_apply + 120
...
For a no-asserts build, the changes are lost.
FindReplTupleInLocalRel() falls back to a sequential scan that
compares all columns. With DEFAULT identity the publisher sends only
the new tuple, or only the key columns for DELETE, so no row matches
and the change is reported as update_missing or delete_missing and
skipped
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.
Before 270af6f0df7, deferrable indexes never reached rd_pkindex, so
the relation was refused with the usual error. That commit meant to
keep deferrable PKs from serving as a replica identity, and this is
the one path it missed.
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
Patch has a TAP test.
--
Thanks,
Nisha