Prevent reindex of invalid indexes on TOAST tables

Such indexes can only be duplicated leftovers of a previously failed
REINDEX CONCURRENTLY command, and a valid equivalent is guaranteed to
exist.  As toast indexes can only be dropped if invalid, reindexing
these would lead to useless duplicated indexes that can't be dropped
anymore, except if the parent relation is dropped.

Thanks to Justin Pryzby for reminding that this problem was reported
long ago during the review of the original patch of REINDEX
CONCURRENTLY, but the issue was never addressed.

Reported-by: Sergei Kornilov, Justin Pryzby
Author: Julien Rouhaud
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/36712441546604286%40sas1-890ba5c2334a.qloud-c.yandex.net
Discussion: https://postgr.es/m/20200216190835[email protected]
Backpatch-through: 12

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 479ddd04727c4f2e11f797b5538c35920424a22a..ad5dbb48461abed04642f5cf1ea7c1a2ad521595 100644 (file)
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3497,6 +3497,17 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                 errmsg("cannot reindex temporary tables of other sessions")));
 
+   /*
+    * Don't allow reindex of an invalid index on TOAST table.  This is a
+    * leftover from a failed REINDEX CONCURRENTLY, and if rebuilt it would
+    * not be possible to drop it anymore.
+    */
+   if (IsToastNamespace(RelationGetNamespace(iRel)) &&
+       !get_index_isvalid(indexId))
+       ereport(ERROR,
+               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                errmsg("cannot reindex invalid index on TOAST table")));
+
    /*
     * Also check for active uses of the index in the current transaction; we
     * don't want to reindex underneath an open indexscan.
@@ -3748,6 +3759,23 @@ reindex_relation(Oid relid, int flags, int options)
        foreach(indexId, indexIds)
        {
            Oid         indexOid = lfirst_oid(indexId);
+           Oid         indexNamespaceId = get_rel_namespace(indexOid);
+
+           /*
+            * Skip any invalid indexes on a TOAST table.  These can only be
+            * duplicate leftovers from a failed REINDEX CONCURRENTLY, and if
+            * rebuilt it would not be possible to drop them anymore.
+            */
+           if (IsToastNamespace(indexNamespaceId) &&
+               !get_index_isvalid(indexOid))
+           {
+               ereport(WARNING,
+                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                        errmsg("cannot reindex invalid index \"%s.%s\" on TOAST table, skipping",
+                               get_namespace_name(indexNamespaceId),
+                               get_rel_name(indexOid))));
+               continue;
+           }
 
            reindex_index(indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS),
                          persistence, options);

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 9869a4dd0e7b87ee83c319e5be79ed645c84a420..661e42fa3a9e6c110755b2a12e351b6a332d6a50 100644 (file)
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2899,6 +2899,16 @@ ReindexRelationConcurrently(Oid relationOid, int options)
                            (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                             errmsg("cannot reindex system catalogs concurrently")));
 
+               /*
+                * Don't allow reindex for an invalid index on TOAST table, as
+                * if rebuild it would not be possible to drop it.
+                */
+               if (IsToastNamespace(get_rel_namespace(relationOid)) &&
+                   !get_index_isvalid(relationOid))
+                   ereport(ERROR,
+                           (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                            errmsg("cannot reindex invalid index on TOAST table concurrently")));
+
                /* Save the list of relation OIDs in private context */
                oldcontext = MemoryContextSwitchTo(private_context);
 

diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index da7e75e7e7cc1b3c11064b8b30caab0b919b9da8..167d2592d854336f9c7df5c55772e636480a7c42 100644 (file)
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -3227,3 +3227,26 @@ get_index_column_opclass(Oid index_oid, int attno)
 
    return opclass;
 }
+
+/*
+ * get_index_isvalid
+ *
+ *     Given the index OID, return pg_index.indisvalid.
+ */
+bool
+get_index_isvalid(Oid index_oid)
+{
+   bool        isvalid;
+   HeapTuple   tuple;
+   Form_pg_index rd_index;
+
+   tuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(index_oid));
+   if (!HeapTupleIsValid(tuple))
+       elog(ERROR, "cache lookup failed for index %u", index_oid);
+
+   rd_index = (Form_pg_index) GETSTRUCT(tuple);
+   isvalid = rd_index->indisvalid;
+   ReleaseSysCache(tuple);
+
+   return isvalid;
+}

diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index 8093f50c4639e065b3d6aaa959f876bf9cedf8f5..810dbeeab99c6155e68f7abd8291905bcef1851e 100644 (file)
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -181,6 +181,7 @@ extern char *get_namespace_name_or_temp(Oid nspid);
 extern Oid get_range_subtype(Oid rangeOid);
 extern Oid get_range_collation(Oid rangeOid);
 extern Oid get_index_column_opclass(Oid index_oid, int attno);
+extern bool get_index_isvalid(Oid index_oid);
 
 #define type_is_array(typid)  (get_element_type(typid) != InvalidOid)
 /* type_is_array_domain accepts both plain arrays and domains over arrays */