Improve and fix some error handling for REINDEX INDEX/TABLE CONCURRENTLY

This improves the user experience when it comes to restrict several
flavors of REINDEX CONCURRENTLY.  First, for INDEX, remove a restriction
on shared relations as we already check after catalog relations.  Then,
for TABLE, add a proper error message when attempting to run the command
on system catalogs.  The code path of CREATE INDEX CONCURRENTLY already
complains about that, but if a REINDEX is issued then then the error
generated is confusing.

While on it, add more tests to check restrictions on catalog indexes and
on toast table/index for catalogs.  Some error messages are improved,
with wording suggestion coming from Tom Lane.

Reported-by: Tom Lane
Author: Michael Paquier
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/23694.1556806002@sss.pgh.pa.us

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index dab06e20a83ad7585b7e6c8a66d97cbd9076b904..7e7c03ef1247534c4e4cca4e1d02b493a6a59fa3 100644 (file)
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2743,6 +2743,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
 
                                MemoryContextSwitchTo(oldcontext);
 
+                               /* A system catalog cannot be reindexed concurrently */
+                               if (IsCatalogRelationOid(relationOid))
+                                       ereport(ERROR,
+                                                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                                        errmsg("cannot reindex a system catalog concurrently")));
+
                                /* Open relation to get its indexes */
                                heapRelation = table_open(relationOid, ShareUpdateExclusiveLock);
 
@@ -2756,13 +2762,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
                                        if (!indexRelation->rd_index->indisvalid)
                                                ereport(WARNING,
                                                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                                                                errmsg("cannot reindex concurrently invalid index \"%s.%s\", skipping",
+                                                                errmsg("cannot reindex invalid index \"%s.%s\" concurrently, skipping",
                                                                                get_namespace_name(get_rel_namespace(cellOid)),
                                                                                get_rel_name(cellOid))));
                                        else if (indexRelation->rd_index->indisexclusion)
                                                ereport(WARNING,
                                                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                                                                errmsg("cannot reindex concurrently exclusion constraint index \"%s.%s\", skipping",
+                                                                errmsg("cannot reindex exclusion constraint index \"%s.%s\" concurrently, skipping",
                                                                                get_namespace_name(get_rel_namespace(cellOid)),
                                                                                get_rel_name(cellOid))));
                                        else
@@ -2802,7 +2808,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
                                                if (!indexRelation->rd_index->indisvalid)
                                                        ereport(WARNING,
                                                                        (errcode(ERRCODE_INDEX_CORRUPTED),
-                                                                        errmsg("cannot reindex concurrently invalid index \"%s.%s\", skipping",
+                                                                        errmsg("cannot reindex invalid index \"%s.%s\" concurrently, skipping",
                                                                                        get_namespace_name(get_rel_namespace(cellOid)),
                                                                                        get_rel_name(cellOid))));
                                                else
@@ -2831,17 +2837,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
                        {
                                Oid                     heapId = IndexGetRelation(relationOid, false);
 
-                               /* A shared relation cannot be reindexed concurrently */
-                               if (IsSharedRelation(heapId))
-                                       ereport(ERROR,
-                                                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                                                        errmsg("concurrent reindex is not supported for shared relations")));
-
                                /* A system catalog cannot be reindexed concurrently */
                                if (IsCatalogRelationOid(heapId))
                                        ereport(ERROR,
                                                        (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                                                        errmsg("concurrent reindex is not supported for catalog relations")));
+                                                        errmsg("cannot reindex a system catalog concurrently")));
 
                                /* Save the list of relation OIDs in private context */
                                oldcontext = MemoryContextSwitchTo(private_context);
@@ -2869,7 +2869,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
                        /* Return error if type of relation is not supported */
                        ereport(ERROR,
                                        (errcode(ERRCODE_WRONG_OBJECT_TYPE),
-                                        errmsg("cannot reindex concurrently this type of relation")));
+                                        errmsg("cannot reindex this type of relation concurrently")));
                        break;
        }
 

diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 326dc4417768a10cd963ce0f1e9febfcab06dc88..c8bc6be06139d7068b9743c741aab2e63f0daff2 100644 (file)
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -1967,7 +1967,7 @@ INSERT INTO concur_reindex_tab3 VALUES  (3, '[1,2]');
 REINDEX INDEX CONCURRENTLY  concur_reindex_tab3_c2_excl;  -- error
 ERROR:  concurrent index creation for exclusion constraints is not supported
 REINDEX TABLE CONCURRENTLY concur_reindex_tab3;  -- succeeds with warning
-WARNING:  cannot reindex concurrently exclusion constraint index "public.concur_reindex_tab3_c2_excl", skipping
+WARNING:  cannot reindex exclusion constraint index "public.concur_reindex_tab3_c2_excl" concurrently, skipping
 INSERT INTO concur_reindex_tab3 VALUES  (4, '[2,4]');
 ERROR:  conflicting key value violates exclusion constraint "concur_reindex_tab3_c2_excl"
 DETAIL:  Key (c2)=([2,5)) conflicts with existing key (c2)=([1,3)).
@@ -2092,10 +2092,15 @@ BEGIN;
 REINDEX TABLE CONCURRENTLY concur_reindex_tab;
 ERROR:  REINDEX CONCURRENTLY cannot run inside a transaction block
 COMMIT;
-REINDEX TABLE CONCURRENTLY pg_database; -- no shared relation
-ERROR:  concurrent index creation on system catalog tables is not supported
-REINDEX TABLE CONCURRENTLY pg_class; -- no catalog relations
-ERROR:  concurrent index creation on system catalog tables is not supported
+REINDEX TABLE CONCURRENTLY pg_class; -- no catalog relation
+ERROR:  cannot reindex a system catalog concurrently
+REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- no catalog index
+ERROR:  cannot reindex a system catalog concurrently
+-- These are the toast table and index of pg_authid.
+REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1260; -- no catalog toast table
+ERROR:  cannot reindex a system catalog concurrently
+REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index
+ERROR:  cannot reindex a system catalog concurrently
 REINDEX SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM
 ERROR:  concurrent reindex of system catalogs is not supported
 -- Warns about catalog relations
@@ -2144,7 +2149,7 @@ DROP INDEX concur_reindex_ind5_ccnew;
 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
+WARNING:  cannot reindex invalid index "public.concur_reindex_ind5" concurrently, skipping
 NOTICE:  table "concur_reindex_tab4" has no indexes
 \d concur_reindex_tab4
         Table "public.concur_reindex_tab4"

diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index f29b8ca826fae03acc153ec97f1232a903b516e9..cd46f071bdc2b38ae5ab5394dbe36b9b0116fcd7 100644 (file)
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -838,8 +838,11 @@ DROP TABLE concur_reindex_part;
 BEGIN;
 REINDEX TABLE CONCURRENTLY concur_reindex_tab;
 COMMIT;
-REINDEX TABLE CONCURRENTLY pg_database; -- no shared relation
-REINDEX TABLE CONCURRENTLY pg_class; -- no catalog relations
+REINDEX TABLE CONCURRENTLY pg_class; -- no catalog relation
+REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- no catalog index
+-- These are the toast table and index of pg_authid.
+REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1260; -- no catalog toast table
+REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index
 REINDEX SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM
 -- Warns about catalog relations
 REINDEX SCHEMA CONCURRENTLY pg_catalog;