Reset indisreplident for an invalid index in DROP INDEX CONCURRENTLY

A failure when dropping concurrently an index used in a replica identity
could leave in pg_index an index marked as !indisvalid and
indisreplident.  Reindexing this index would switch back indisvalid to
true, and if the replica identity of the parent relation was switched to
use a different index, it would be possible to finish with more than one
index marked as indisreplident.  If that were to happen, this could mess
up with the relation cache as an incorrect index could be used for the
replica identity.

Indexes marked as invalid are discarded as candidates for the replica
identity, as of RelationGetIndexList(), so similarly to what is done
with indisclustered, resetting indisreplident when the index is marked
as invalid keeps things consistent.  REINDEX CONCURRENTLY's swapping
already resets the flag for the old index, while the new index inherits
the value of the old index to-be-dropped, so only DROP INDEX was an
issue.

Even if this is a bug, the sequence able to reproduce a problem requires
a failure while running DROP INDEX CONCURRENTLY, something unlikely
going to happen in the field, so no back is done.

Author: Michael Paquier
Reviewed-by: Dmitry Dolgov
Discussion: https://postgr.es/m/20200827025721[email protected]

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 1be27eec52e6e1150f8d29d01d0941f416dd7516..62e487bb4c8a7cc86e92689ea378604235a124d2 100644 (file)
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -1512,7 +1512,6 @@ index_concurrently_swap(Oid newIndexId, Oid oldIndexId, const char *oldName)
 
    /* Preserve indisreplident in the new index */
    newIndexForm->indisreplident = oldIndexForm->indisreplident;
-   oldIndexForm->indisreplident = false;
 
    /* Preserve indisclustered in the new index */
    newIndexForm->indisclustered = oldIndexForm->indisclustered;
@@ -1524,6 +1523,7 @@ index_concurrently_swap(Oid newIndexId, Oid oldIndexId, const char *oldName)
    newIndexForm->indisvalid = true;
    oldIndexForm->indisvalid = false;
    oldIndexForm->indisclustered = false;
+   oldIndexForm->indisreplident = false;
 
    CatalogTupleUpdate(pg_index, &oldIndexTuple->t_self, oldIndexTuple);
    CatalogTupleUpdate(pg_index, &newIndexTuple->t_self, newIndexTuple);
@@ -3349,10 +3349,13 @@ index_set_state_flags(Oid indexId, IndexStateFlagsAction action)
             * CONCURRENTLY that failed partway through.)
             *
             * Note: the CLUSTER logic assumes that indisclustered cannot be
-            * set on any invalid index, so clear that flag too.
+            * set on any invalid index, so clear that flag too.  Similarly,
+            * ALTER TABLE assumes that indisreplident cannot be set for
+            * invalid indexes.
             */
            indexForm->indisvalid = false;
            indexForm->indisclustered = false;
+           indexForm->indisreplident = false;
            break;
        case INDEX_DROP_SET_DEAD:
 
@@ -3364,6 +3367,8 @@ index_set_state_flags(Oid indexId, IndexStateFlagsAction action)
             * the index at all.
             */
            Assert(!indexForm->indisvalid);
+           Assert(!indexForm->indisclustered);
+           Assert(!indexForm->indisreplident);
            indexForm->indisready = false;
            indexForm->indislive = false;
            break;