diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c index 87498264256..8ffd2583afb 100644 --- a/src/backend/replication/logical/relation.c +++ b/src/backend/replication/logical/relation.c @@ -791,7 +791,17 @@ 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 +824,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 #