From d702a7f0e8e9657bb474641fda06c721031e7d57 Mon Sep 17 00:00:00 2001 From: Mikhail Nikalayeu Date: Sat, 22 Aug 2026 20:07:33 +0200 Subject: [PATCH v1] Don't choose an invalid index for REPLICA IDENTITY FULL lookups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For a REPLICA IDENTITY FULL remote relation whose local counterpart has no primary key or replica identity, FindUsableIndexForReplicaIdentityFull() takes the first index of a suitable shape out of RelationGetIndexList(). That list omits only indexes that are not indislive, so the leftover of a failed CREATE INDEX CONCURRENTLY can be chosen. Such an index need not contain every row, so changes whose rows it fails to find are silently dropped as missing-tuple conflicts -- or, where it holds nothing at all, the scan errors out and the subscription stops. Skip invalid indexes, as the planner does. The test belongs here rather than in IsIndexUsableForReplicaIdentityFull(), whose other caller is an assertion that would then trip over an index DROP INDEX CONCURRENTLY has just invalidated but that still finds tuples fine. Oversight in 89e46da5e51. Back-patch to 16. Author: Mikhail Nikalayeu Reviewed-by: Miłosz Bieniek Reviewed-by: Amit Kapila Reviewed-by: Shlok Kyal Reviewed-by: Vignesh C Reviewed-by: Ajin Cherian Discussion: https://postgr.es/m/CADzfLwWuubcbJBDRZ_J1SSqHDNjNmUYSAgf5y=17LxmP401xbw@mail.gmail.com --- src/backend/replication/logical/relation.c | 15 ++++- .../subscription/t/032_subscribe_use_index.pl | 59 +++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c index 6edbd5643ae..643f025a4c3 100644 --- a/src/backend/replication/logical/relation.c +++ b/src/backend/replication/logical/relation.c @@ -755,7 +755,16 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap) idxRel = index_open(idxoid, AccessShareLock); idxInfo = BuildIndexInfo(idxRel); - isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxInfo, attrmap); + + /* + * indisvalid is checked here, not in IsIndexUsableForReplicaIdentityFull(), + * since that function's other caller (an assertion) must tolerate + * an index made transiently invalid by a concurrent DROP INDEX + * CONCURRENTLY, whereas a permanently invalid leftover of a failed + * CREATE INDEX CONCURRENTLY must never be chosen here. + */ + isUsableIdx = idxRel->rd_index->indisvalid && + IsIndexUsableForReplicaIdentityFull(idxInfo, attrmap); index_close(idxRel, AccessShareLock); /* Return the first eligible index found */ @@ -778,6 +787,10 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap) * map to check whether the local index attribute has a corresponding remote * attribute. * + * Note that this function does not check indisvalid. Callers that are + * selecting an index to use for future lookups must check indisvalid + * themselves and reject invalid indexes. + * * Note that the limitations of index scans for replica identity full only * adheres to a subset of the limitations of PK/RI. For example, we support * columns that are marked as [NULL] or we are not interested in the [NOT diff --git a/src/test/subscription/t/032_subscribe_use_index.pl b/src/test/subscription/t/032_subscribe_use_index.pl index 576eec6a578..0b800b6213d 100644 --- a/src/test/subscription/t/032_subscribe_use_index.pl +++ b/src/test/subscription/t/032_subscribe_use_index.pl @@ -478,6 +478,65 @@ $node_subscriber->safe_psql('postgres', "DROP TABLE test_replica_id_full"); # data # ============================================================================= +# ============================================================================= +# Testcase start: Subscription does not use an invalid index +# +# A failed CREATE INDEX CONCURRENTLY leaves behind a live but invalid +# index, which is not required to contain every row. The apply worker +# must not choose it for REPLICA IDENTITY FULL lookups. +# + +# create tables pub and sub +$node_publisher->safe_psql('postgres', + "CREATE TABLE test_invalid (x int, y int)"); +$node_publisher->safe_psql('postgres', + "ALTER TABLE test_invalid REPLICA IDENTITY FULL"); +$node_subscriber->safe_psql('postgres', + "CREATE TABLE test_invalid (x int, y int)"); + +# insert some initial data, including the row the index build trips over +$node_publisher->safe_psql('postgres', + "INSERT INTO test_invalid SELECT i, i FROM generate_series(1,10) i"); + +# create pub/sub +$node_publisher->safe_psql('postgres', + "CREATE PUBLICATION tap_pub_invalid FOR TABLE test_invalid"); +$node_subscriber->safe_psql('postgres', + "CREATE SUBSCRIPTION tap_sub_invalid CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub_invalid" +); + +# wait for initial table synchronization to finish +$node_subscriber->wait_for_subscription_sync($node_publisher, $appname); + +# leave an invalid index behind: the build fails on the y = 5 row +my ($cic_ret, $cic_out, $cic_err) = $node_subscriber->psql('postgres', + "CREATE INDEX CONCURRENTLY test_invalid_idx ON test_invalid (x, (1/(y-5)))" +); +isnt($cic_ret, 0, 'CREATE INDEX CONCURRENTLY fails'); +$result = $node_subscriber->safe_psql('postgres', + "SELECT indisvalid FROM pg_index" + . " WHERE indexrelid = 'test_invalid_idx'::regclass"); +is($result, qq(f), 'and leaves an invalid index behind'); + +# the update must still be applied +$node_publisher->safe_psql('postgres', + "UPDATE test_invalid SET y = 99 WHERE x = 7"); +$node_publisher->wait_for_catchup($appname); +$result = $node_subscriber->safe_psql('postgres', + "SELECT y FROM test_invalid WHERE x = 7"); +is($result, qq(99), + 'ensure subscriber has the correct data at the end of the test'); + +# cleanup pub +$node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_invalid"); +$node_publisher->safe_psql('postgres', "DROP TABLE test_invalid"); +# cleanup sub +$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_invalid"); +$node_subscriber->safe_psql('postgres', "DROP TABLE test_invalid"); + +# Testcase end: Subscription does not use an invalid index +# ============================================================================= + $node_subscriber->stop('fast'); $node_publisher->stop('fast'); -- 2.43.0