From 3053bf32ba98397930b4bb2c6c50d7e4f6f919d6 Mon Sep 17 00:00:00 2001 From: Nisha Moond Date: Mon, 28 Sep 2026 17:39:39 +0530 Subject: [PATCH v5] 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. Fix by deciding updatability from the index already chosen by FindLogicalRepLocalIndex(), instead of looking up the replica identity and primary key again. The check now always agrees with the index used for the tuple lookup. Such tables now get the usual error unless the publisher uses REPLICA IDENTITY FULL. Author: Nisha Moond Reviewed-by: Hayato Kuroda Reviewed-by: Zhijie Hou Discussion: https://postgr.es/m/CABdArM5ydwdRrpaZyK1q2p3-vY_+pnBtTmkvg_pcM=gHwmH7Kg@mail.gmail.com Backpatch-through: 17 --- src/backend/replication/logical/relation.c | 52 +++++++++--------- .../subscription/t/032_subscribe_use_index.pl | 54 +++++++++++++++++++ 2 files changed, 80 insertions(+), 26 deletions(-) diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c index 6242ce70ad2..2723d1bd17f 100644 --- a/src/backend/replication/logical/relation.c +++ b/src/backend/replication/logical/relation.c @@ -303,36 +303,34 @@ logicalrep_report_missing_or_gen_attrs(LogicalRepRelation *remoterel, * replica identity is found to be insufficient for applying * updates/deletes (inserts don't care!) and leave it to * check_relation_updatable() to throw the actual error if needed. + * + * The caller must have set entry->localindexoid and entry->idxisreplident. */ static void logicalrep_rel_mark_updatable(LogicalRepRelMapEntry *entry) { - Bitmapset *idkey; LogicalRepRelation *remoterel = &entry->remoterel; - int i; + Relation idxrel; entry->updatable = true; - idkey = RelationGetIndexAttrBitmap(entry->localrel, - INDEX_ATTR_BITMAP_IDENTITY_KEY); - /* fallback to PK if no replica identity */ - if (idkey == NULL) + /* + * If no replica identity index and no usable PK, the published table must + * have replica identity FULL. + */ + if (!entry->idxisreplident) { - 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 (idkey == NULL && remoterel->replident != REPLICA_IDENTITY_FULL) + if (remoterel->replident != REPLICA_IDENTITY_FULL) entry->updatable = false; + + return; } - i = -1; - while ((i = bms_next_member(idkey, i)) >= 0) + idxrel = index_open(entry->localindexoid, AccessShareLock); + + for (int i = 0; i < idxrel->rd_index->indnkeyatts; i++) { - int attnum = i + FirstLowInvalidHeapAttributeNumber; + int attnum = idxrel->rd_index->indkey.values[i]; if (!AttrNumberIsForUserDefinedAttr(attnum)) ereport(ERROR, @@ -350,6 +348,8 @@ logicalrep_rel_mark_updatable(LogicalRepRelMapEntry *entry) break; } } + + index_close(idxrel, AccessShareLock); } /* @@ -484,12 +484,6 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode) bms_free(generatedattrs); bms_free(missingatts); - /* - * Set if the table's replica identity is enough to apply - * update/delete. - */ - logicalrep_rel_mark_updatable(entry); - /* * Finding a usable index is an infrequent task. It occurs when an * operation is first performed on the relation, or after invalidation @@ -500,6 +494,12 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode) entry->attrmap, &entry->idxisreplident); + /* + * Set if the table's replica identity is enough to apply + * update/delete. + */ + logicalrep_rel_mark_updatable(entry); + entry->localrelvalid = true; } @@ -749,9 +749,6 @@ logicalrep_partition_open(LogicalRepRelMapEntry *root, attrmap->maplen * sizeof(AttrNumber)); } - /* Set if the table's replica identity is enough to apply update/delete. */ - logicalrep_rel_mark_updatable(entry); - /* state and statelsn are left set to 0. */ MemoryContextSwitchTo(oldctx); @@ -768,6 +765,9 @@ logicalrep_partition_open(LogicalRepRelMapEntry *root, entry->attrmap, &entry->idxisreplident); + /* Set if the table's replica identity is enough to apply update/delete. */ + logicalrep_rel_mark_updatable(entry); + entry->localrelvalid = true; return entry; 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)