pg_index.indisreplident and invalid indexes

Hi all,

While digging into a different patch involving DROP INDEX CONCURRENTLY
and replica indexes, I have found that the handling of indisreplident
is inconsistent for invalid indexes:
https://www.postgresql.org/message-id/20200827022835.GM2017@paquier.xyz

In short, imagine the following sequence:
CREATE TABLE test_replica_identity_4 (id int NOT NULL);
CREATE UNIQUE INDEX test_replica_index_4 ON
test_replica_identity_4(id);
ALTER TABLE test_replica_identity_4 REPLICA IDENTITY
USING INDEX test_replica_index_4;
-- imagine that this fails in the second transaction used in
-- index_drop().
DROP INDEX CONCURRENTLY test_replica_index_4;
-- here the index still exists, is invalid, marked with
-- indisreplident.
CREATE UNIQUE INDEX test_replica_index_4_2 ON
test_replica_identity_4(id);
ALTER TABLE test_replica_identity_4 REPLICA IDENTITY
USING INDEX test_replica_index_4_2;
-- set back the index to a valid state.
REINDEX INDEX test_replica_index_4;
-- And here we have two valid indexes usable as replica identities.
SELECT indexrelid::regclass, indisvalid, indisreplident FROM pg_index
WHERE indexrelid IN ('test_replica_index_4'::regclass,
'test_replica_index_4_2'::regclass);
indexrelid | indisvalid | indisreplident
------------------------+------------+----------------
test_replica_index_4_2 | t | t
test_replica_index_4 | t | t
(2 rows)

You can just use the following trick to emulate a failure in DIC:
@@ -2195,6 +2195,9 @@ index_drop(Oid indexId, bool concurrent, bool
concurrent_lock_mode)
if (userIndexRelation->rd_rel->relkind != RELKIND_PARTITIONED_INDEX)
RelationDropStorage(userIndexRelation);
+ if (concurrent)
+ elog(ERROR, "burp");

This results in some problems for ALTER TABLE in tablecmds.c, as it is
possible to reach a state in the catalogs where we have *multiple*
indexes marked with indisreplindex for REPLICA_IDENTITY_INDEX set on
the parent table. Even worse, this messes up with
RelationGetIndexList() as it would set rd_replidindex in the relcache
for the last index found marked with indisreplident, depending on the
order where the indexes are scanned, you may get a different replica
index loaded.

I think that this problem is similar to indisclustered, and that we
had better set indisreplident to false when clearing indisvalid for an
index concurrently dropped. This would prevent problems with ALTER
TABLE of course, but also the relcache.

Any objections to the attached? I am not sure that this is worth a
backpatch as that's unlikely going to be a problem in the field, so
I'd like to fix this issue only on HEAD. This exists since 9.4 and
the introduction of replica identities.
--
Michael