Rework handling of invalid indexes with REINDEX CONCURRENTLY

Per discussion with others, allowing REINDEX INDEX CONCURRENTLY to work
for invalid indexes when working directly on them can have a lot of
value to unlock situations with invalid indexes without having to use a
dance involving DROP INDEX followed by an extra CREATE INDEX
CONCURRENTLY (which would not work for indexes with constraint
dependency anyway).  This also does not create extra bloat on the
relation involved as this works on individual indexes, so let's enable
it.

Note that REINDEX TABLE CONCURRENTLY still bypasses invalid indexes as
we don't want to bloat the number of indexes defined on a relation in
the event of multiple and successive failures of REINDEX CONCURRENTLY.

More regression tests are added to cover those behaviors, using an
invalid index created with CREATE INDEX CONCURRENTLY.

Reported-by: Dagfinn Ilmari Mannsåker, Álvaro Herrera
Author: Michael Paquier
Reviewed-by: Peter Eisentraut, Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/20190411134947.GA22043@alvherre.pgsql

diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml
index e62ab33905c1156baf4639bbb1a3dfd4375c05d0..521e32b197c114b58a74b71c79d4a04ad2e348d7 100644 (file)
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -586,10 +586,8 @@ Indexes:
 
     The recommended recovery
     method in such cases is to drop the index and try again to perform
-    <command>CREATE INDEX CONCURRENTLY</command>.  (Another possibility is to rebuild
-    the index with <command>REINDEX</command>.  However, since <command>REINDEX</command>
-    does not support concurrent builds, this option is unlikely to seem
-    attractive.)
+    <command>CREATE INDEX CONCURRENTLY</command>.  (Another possibility is
+    to rebuild the index with <command>REINDEX INDEX CONCURRENTLY</command>).
    </para>
 
    <para>

diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index e05a76c6d8846b45466df4a5ff13467281f13c8b..303436c89db5bc3426353aa3db362fafef74a31e 100644 (file)
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -65,12 +65,11 @@ REINDEX [ ( VERBOSE ) ] { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } [ CONCURR
 
     <listitem>
      <para>
-      An index build with the <literal>CONCURRENTLY</literal> option failed, leaving
-      an <quote>invalid</quote> index. Such indexes are useless but it can be
-      convenient to use <command>REINDEX</command> to rebuild them. Note that
-      <command>REINDEX</command> will not perform a concurrent build on an invalid index. To build the
-      index without interfering with production you should drop the index and
-      reissue the <command>CREATE INDEX CONCURRENTLY</command> command.
+      If an index build fails with the <literal>CONCURRENTLY</literal> option,
+      this index is left as <quote>invalid</quote>. Such indexes are useless
+      but it can be convenient to use <command>REINDEX</command> to rebuild
+      them. Note that only <command>REINDEX INDEX</command> is able
+      to perform a concurrent build on an invalid index.
      </para>
     </listitem>
 

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 46f32c21f97146ed3a0f20f3a3b237c4edf9faeb..a1c91b5fb8737a4574621b5185f81f34412d9724 100644 (file)
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2776,11 +2776,6 @@ ReindexRelationConcurrently(Oid relationOid, int options)
                        }
                case RELKIND_INDEX:
                        {
-                               /*
-                                * For an index simply add its Oid to list. Invalid indexes
-                                * cannot be included in list.
-                                */
-                               Relation        indexRelation = index_open(relationOid, ShareUpdateExclusiveLock);
                                Oid                     heapId = IndexGetRelation(relationOid, false);
 
                                /* A shared relation cannot be reindexed concurrently */
@@ -2801,25 +2796,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
                                /* Track the heap relation of this index for session locks */
                                heapRelationIds = list_make1_oid(heapId);
 
-                               MemoryContextSwitchTo(oldcontext);
-
-                               if (!indexRelation->rd_index->indisvalid)
-                                       ereport(WARNING,
-                                                       (errcode(ERRCODE_INDEX_CORRUPTED),
-                                                        errmsg("cannot reindex concurrently invalid index \"%s.%s\", skipping",
-                                                                       get_namespace_name(get_rel_namespace(relationOid)),
-                                                                       get_rel_name(relationOid))));
-                               else
-                               {
-                                       /* Save the list of relation OIDs in private context */
-                                       oldcontext = MemoryContextSwitchTo(private_context);
-
-                                       indexIds = lappend_oid(indexIds, relationOid);
-
-                                       MemoryContextSwitchTo(oldcontext);
-                               }
+                               /*
+                                * Save the list of relation OIDs in private context.  Note
+                                * that invalid indexes are allowed here.
+                                */
+                               indexIds = lappend_oid(indexIds, relationOid);
 
-                               index_close(indexRelation, NoLock);
+                               MemoryContextSwitchTo(oldcontext);
                                break;
                        }
                case RELKIND_PARTITIONED_TABLE:

diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 39159e091578ddab56539d168d8c92675c635482..326dc4417768a10cd963ce0f1e9febfcab06dc88 100644 (file)
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2118,6 +2118,53 @@ Referenced by:
 
 DROP MATERIALIZED VIEW concur_reindex_matview;
 DROP TABLE concur_reindex_tab, concur_reindex_tab2, concur_reindex_tab3;
+-- Check handling of invalid indexes
+CREATE TABLE concur_reindex_tab4 (c1 int);
+INSERT INTO concur_reindex_tab4 VALUES (1), (1), (2);
+-- This trick creates an invalid index.
+CREATE UNIQUE INDEX CONCURRENTLY concur_reindex_ind5 ON concur_reindex_tab4 (c1);
+ERROR:  could not create unique index "concur_reindex_ind5"
+DETAIL:  Key (c1)=(1) is duplicated.
+-- Reindexing concurrently this index fails with the same failure.
+-- The extra index created is itself invalid, and can be dropped.
+REINDEX INDEX CONCURRENTLY concur_reindex_ind5;
+ERROR:  could not create unique index "concur_reindex_ind5_ccnew"
+DETAIL:  Key (c1)=(1) is duplicated.
+\d concur_reindex_tab4
+        Table "public.concur_reindex_tab4"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ c1     | integer |           |          | 
+Indexes:
+    "concur_reindex_ind5" UNIQUE, btree (c1) INVALID
+    "concur_reindex_ind5_ccnew" UNIQUE, btree (c1) INVALID
+
+DROP INDEX concur_reindex_ind5_ccnew;
+-- This makes the previous failure go away, so the index can become valid.
+DELETE FROM concur_reindex_tab4 WHERE c1 = 1;
+-- The invalid index is not processed when running REINDEX TABLE.
+REINDEX TABLE CONCURRENTLY concur_reindex_tab4;
+WARNING:  cannot reindex concurrently invalid index "public.concur_reindex_ind5", skipping
+NOTICE:  table "concur_reindex_tab4" has no indexes
+\d concur_reindex_tab4
+        Table "public.concur_reindex_tab4"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ c1     | integer |           |          | 
+Indexes:
+    "concur_reindex_ind5" UNIQUE, btree (c1) INVALID
+
+-- But it is fixed with REINDEX INDEX.
+REINDEX INDEX CONCURRENTLY concur_reindex_ind5;
+\d concur_reindex_tab4
+        Table "public.concur_reindex_tab4"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ c1     | integer |           |          | 
+Indexes:
+    "concur_reindex_ind5" UNIQUE, btree (c1)
+
+DROP TABLE concur_reindex_tab4;
 --
 -- REINDEX SCHEMA
 --

diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index f8141c0ce5158d38489e8626f09c952af1befe08..f29b8ca826fae03acc153ec97f1232a903b516e9 100644 (file)
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -849,6 +849,26 @@ REINDEX SCHEMA CONCURRENTLY pg_catalog;
 DROP MATERIALIZED VIEW concur_reindex_matview;
 DROP TABLE concur_reindex_tab, concur_reindex_tab2, concur_reindex_tab3;
 
+-- Check handling of invalid indexes
+CREATE TABLE concur_reindex_tab4 (c1 int);
+INSERT INTO concur_reindex_tab4 VALUES (1), (1), (2);
+-- This trick creates an invalid index.
+CREATE UNIQUE INDEX CONCURRENTLY concur_reindex_ind5 ON concur_reindex_tab4 (c1);
+-- Reindexing concurrently this index fails with the same failure.
+-- The extra index created is itself invalid, and can be dropped.
+REINDEX INDEX CONCURRENTLY concur_reindex_ind5;
+\d concur_reindex_tab4
+DROP INDEX concur_reindex_ind5_ccnew;
+-- This makes the previous failure go away, so the index can become valid.
+DELETE FROM concur_reindex_tab4 WHERE c1 = 1;
+-- The invalid index is not processed when running REINDEX TABLE.
+REINDEX TABLE CONCURRENTLY concur_reindex_tab4;
+\d concur_reindex_tab4
+-- But it is fixed with REINDEX INDEX.
+REINDEX INDEX CONCURRENTLY concur_reindex_ind5;
+\d concur_reindex_tab4
+DROP TABLE concur_reindex_tab4;
+
 --
 -- REINDEX SCHEMA
 --