Fix apply worker crash when subscriber table has only a deferrable primary key - Mailing list pgsql-hackers

From Nisha Moond
Subject Fix apply worker crash when subscriber table has only a deferrable primary key
Date
Msg-id CABdArM5ydwdRrpaZyK1q2p3-vY_+pnBtTmkvg_pcM=gHwmH7Kg@mail.gmail.com
Whole thread
List pgsql-hackers
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

Attachment

pgsql-hackers by date:

Previous
From: Bernd Reiß
Date:
Subject: Use instr_time for pg_stat_database block read/write time counters
Next
From: Nazir Bilal Yavuz
Date:
Subject: Re: Stabilize and shorten test_checksums/013_rewind test