From 1a5e0d960ca8e3dae04e55448e108a14f6788c74 Mon Sep 17 00:00:00 2001
From: Mikhail Nikalayeu <mihailnikalayeu@gmail.com>
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 <mihailnikalayeu@gmail.com>
Reviewed-by: Miłosz Bieniek <bieniek.milosz@proton.me>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Shlok Kyal <shlok.kyal.oss@gmail.com>
Reviewed-by: Vignesh C <vignesh21@gmail.com>
Reviewed-by: Ajin Cherian <itsajin@gmail.com>
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 87498264256..f1cf1dff31c 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -791,7 +791,16 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
 		Relation	idxRel;
 
 		idxRel = index_open(idxoid, AccessShareLock);
-		isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxRel, 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(idxRel, attrmap);
 		index_close(idxRel, AccessShareLock);
 
 		/* Return the first eligible index found */
@@ -814,6 +823,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 c755c1a7518..1ccd36ac227 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
+# =============================================================================
+
 # =============================================================================
 # Testcase start: Subscription can use hash index
 #
-- 
2.43.0

