From 86ba2ace5e4f34eff3bf77f9746421c729fc2fa3 Mon Sep 17 00:00:00 2001 From: Nisha Moond Date: Mon, 5 Oct 2026 14:21:10 +0530 Subject: [PATCH v5_pg17] 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 1909de8e570..35092cd914e 100644 --- a/src/backend/replication/logical/relation.c +++ b/src/backend/replication/logical/relation.c @@ -269,36 +269,34 @@ logicalrep_report_missing_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, @@ -316,6 +314,8 @@ logicalrep_rel_mark_updatable(LogicalRepRelMapEntry *entry) break; } } + + index_close(idxrel, AccessShareLock); } /* @@ -440,12 +440,6 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode) /* be tidy */ 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 @@ -456,6 +450,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; } @@ -710,9 +710,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); @@ -729,6 +726,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 e2e3913f63b..f1c4608ca0b 100644 --- a/src/test/subscription/t/032_subscribe_use_index.pl +++ b/src/test/subscription/t/032_subscribe_use_index.pl @@ -731,6 +731,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)