From 18a7456018fa76bbbdcca9dd13c58a63c4784316 Mon Sep 17 00:00:00 2001 From: Nisha Moond Date: Mon, 28 Sep 2026 17:39:39 +0530 Subject: [PATCH v2] Don't treat a deferrable PK as replica identity on the subscriber logicalrep_rel_mark_updatable() fell back to the primary-key column bitmap when the local table had no replica identity index. Since 270af6f0df7, that bitmap includes deferrable primary keys, so a table whose only key was deferrable was marked updatable. The tuple lookup, however, correctly refuses such keys, so no index was found. Assert-enabled builds then failed an assertion in FindReplTupleInLocalRel(). Other builds fell back to a full-row match that could not succeed, and the UPDATE or DELETE was silently skipped. Fall back to the primary key only if RelationGetPrimaryKeyIndex() accepts it as non-deferrable, matching the lookup path. Such tables now get the usual error unless the publisher uses REPLICA IDENTITY FULL. RelationFindDeletedTupleInfoSeq() had the same fallback when searching for a recently deleted row. With REPLICA IDENTITY FULL on the publisher, it could match dead rows on the deferrable key alone, while the index path compared the whole row. The same change could then be reported as update_deleted or update_missing. Apply the same check there. Author: Nisha Moond Author: Hayato Kuroda Discussion: https://postgr.es/m/CABdArM5ydwdRrpaZyK1q2p3-vY_+pnBtTmkvg_pcM=gHwmH7Kg@mail.gmail.com Backpatch-through: 17 --- src/backend/executor/execReplication.c | 7 ++- src/backend/replication/logical/relation.c | 18 +++++-- .../subscription/t/032_subscribe_use_index.pl | 54 +++++++++++++++++++ 3 files changed, 72 insertions(+), 7 deletions(-) diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index dd42acc13e2..168d8a68c13 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -593,8 +593,11 @@ RelationFindDeletedTupleInfoSeq(Relation rel, TupleTableSlot *searchslot, indexbitmap = RelationGetIndexAttrBitmap(rel, INDEX_ATTR_BITMAP_IDENTITY_KEY); - /* fallback to PK if no replica identity */ - if (!indexbitmap) + /* + * fallback to PK if no replica identity, but only if the PK is not + * deferrable. + */ + if (!indexbitmap && OidIsValid(RelationGetPrimaryKeyIndex(rel, false))) indexbitmap = RelationGetIndexAttrBitmap(rel, INDEX_ATTR_BITMAP_PRIMARY_KEY); diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c index 6242ce70ad2..8b173e379d0 100644 --- a/src/backend/replication/logical/relation.c +++ b/src/backend/replication/logical/relation.c @@ -315,15 +315,23 @@ logicalrep_rel_mark_updatable(LogicalRepRelMapEntry *entry) idkey = RelationGetIndexAttrBitmap(entry->localrel, INDEX_ATTR_BITMAP_IDENTITY_KEY); - /* fallback to PK if no replica identity */ + + /* + * Fall back to the PK if no replica identity, but only if the PK is not + * deferrable. INDEX_ATTR_BITMAP_PRIMARY_KEY includes the columns of a + * deferrable PK, but such a PK cannot serve as a replica identity (its + * uniqueness may be transiently violated), and FindLogicalRepLocalIndex() + * will not use it to look up tuples. + */ if (idkey == NULL) { - idkey = RelationGetIndexAttrBitmap(entry->localrel, - INDEX_ATTR_BITMAP_PRIMARY_KEY); + if (OidIsValid(RelationGetPrimaryKeyIndex(entry->localrel, false))) + idkey = RelationGetIndexAttrBitmap(entry->localrel, + INDEX_ATTR_BITMAP_PRIMARY_KEY); /* - * If no replica identity index and no PK, the published table must - * have replica identity FULL. + * If no replica identity index and no usable PK, the published table + * must have replica identity FULL. */ if (idkey == NULL && remoterel->replident != REPLICA_IDENTITY_FULL) entry->updatable = false; diff --git a/src/test/subscription/t/032_subscribe_use_index.pl b/src/test/subscription/t/032_subscribe_use_index.pl index bd80465f727..a134e540496 100644 --- a/src/test/subscription/t/032_subscribe_use_index.pl +++ b/src/test/subscription/t/032_subscribe_use_index.pl @@ -733,6 +733,60 @@ SKIP: # demoted from replica identity # ============================================================================= +# ============================================================================= +# Testcase start: Subscription does not use a deferrable primary key +# +# A deferrable primary key cannot serve as a replica identity, because its +# uniqueness may be transiently violated. When it is the subscriber table's +# only key and the published table does not have REPLICA IDENTITY FULL, the +# apply worker must refuse the relation for UPDATE rather than proceed +# without an index to look up the tuple. +# + +# create tables pub and sub +$node_publisher->safe_psql('postgres', + "CREATE TABLE test_deferrable_pk (a int PRIMARY KEY, b text)"); +$node_subscriber->safe_psql('postgres', + "CREATE TABLE test_deferrable_pk (a int, b text, PRIMARY KEY (a) DEFERRABLE)" +); + +# insert some initial data +$node_publisher->safe_psql('postgres', + "INSERT INTO test_deferrable_pk VALUES (1, 'one')"); + +# create pub/sub +$node_publisher->safe_psql('postgres', + "CREATE PUBLICATION tap_pub_deferrable_pk FOR TABLE test_deferrable_pk"); +$node_subscriber->safe_psql('postgres', + "CREATE SUBSCRIPTION tap_sub_deferrable_pk CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_deferrable_pk" +); + +# wait for initial table synchronization to finish +$node_subscriber->wait_for_subscription_sync($node_publisher, $appname); + +# the update must be refused, since the deferrable PK is not usable +my $log_offset = -s $node_subscriber->logfile; +$node_publisher->safe_psql('postgres', + "UPDATE test_deferrable_pk SET b = 'two' WHERE a = 1"); +$node_subscriber->wait_for_log( + qr/ERROR: ( [A-Z0-9]+:)? logical replication target relation "public.test_deferrable_pk" has neither REPLICA IDENTITY index nor PRIMARY KEY and published relation does not have REPLICA IDENTITY FULL/, + $log_offset); + +$result = $node_subscriber->safe_psql('postgres', + "SELECT b FROM test_deferrable_pk WHERE a = 1"); +is($result, qq(one), 'update is not applied on subscriber'); + +# cleanup pub +$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_deferrable_pk"); +$node_publisher->safe_psql('postgres', "DROP TABLE test_deferrable_pk"); +# cleanup sub +$node_subscriber->safe_psql('postgres', + "DROP SUBSCRIPTION tap_sub_deferrable_pk"); +$node_subscriber->safe_psql('postgres', "DROP TABLE test_deferrable_pk"); + +# Testcase end: Subscription does not use a deferrable primary key +# ============================================================================= + $node_subscriber->stop('fast'); $node_publisher->stop('fast'); -- 2.54.0 (Apple Git-157)