On Sat, May 04, 2019 at 11:00:11AM -0400, Tom Lane wrote: > I'm not excited about rewording longstanding errors. These two are > new though (aren't they?)
The message you are referring to in index_create() has been introduced as of e093dcdd with the introduction of CREATE INDEX CONCURRENTLY, and it can be perfectly hit without REINDEX: =# show allow_system_table_mods; allow_system_table_mods ------------------------- on (1 row) =# create index CONCURRENTLY popo on pg_class (relname); ERROR: 0A000: concurrent index creation on system catalog tables is not supported LOCATION: index_create, index.c:830 So I don't agree with switching the existing error message in index_create(). What we could do instead is to add a REINDEX-specific error in ReindexRelationConcurrently() as done for index relkinds, using your proposed wording. What do you think about something like the attached then? HEAD does not check after system indexes with REINDEX INDEX CONCURRENTLY, and I have moved all the catalog-related tests to reindex_catalog.sql. -- Michael
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index fabba3bacd..519673a2e3 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2729,6 +2729,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
/* Open relation to get its indexes */
heapRelation = table_open(relationOid, ShareUpdateExclusiveLock);
+ /* A system catalog cannot be reindexed concurrently */
+ if (IsSystemRelation(heapRelation))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot reindex a system catalog concurrently")));
+
/* Add all the valid indexes of relation to list */
foreach(lc, RelationGetIndexList(heapRelation))
{
@@ -2814,17 +2820,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 (IsSystemNamespace(get_rel_namespace(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);
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 326dc44177..ab8b1eb3fe 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2092,10 +2092,6 @@ 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 SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM
ERROR: concurrent reindex of system catalogs is not supported
-- Warns about catalog relations
diff --git a/src/test/regress/expected/reindex_catalog.out b/src/test/regress/expected/reindex_catalog.out
index 142616fccb..70a3f906da 100644
--- a/src/test/regress/expected/reindex_catalog.out
+++ b/src/test/regress/expected/reindex_catalog.out
@@ -31,3 +31,28 @@ REINDEX INDEX pg_index_indexrelid_index; -- non-mapped, non-shared, critical
REINDEX INDEX pg_index_indrelid_index; -- non-mapped, non-shared, non-critical
REINDEX INDEX pg_database_oid_index; -- mapped, shared, critical
REINDEX INDEX pg_shdescription_o_c_index; -- mapped, shared, non-critical
+-- Check that REINDEX CONCURRENTLY is forbidden with system catalogs.
+-- For whole tables.
+REINDEX TABLE CONCURRENTLY pg_class; -- mapped, non-shared, critical
+ERROR: cannot reindex a system catalog concurrently
+REINDEX TABLE CONCURRENTLY pg_index; -- non-mapped, non-shared, critical
+ERROR: cannot reindex a system catalog concurrently
+REINDEX TABLE CONCURRENTLY pg_operator; -- non-mapped, non-shared, critical
+ERROR: cannot reindex a system catalog concurrently
+REINDEX TABLE CONCURRENTLY pg_database; -- mapped, shared, critical
+ERROR: cannot reindex a system catalog concurrently
+REINDEX TABLE CONCURRENTLY pg_shdescription; -- mapped, shared non-critical
+ERROR: cannot reindex a system catalog concurrently
+-- For system indexes
+REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- mapped, non-shared, critical
+ERROR: cannot reindex a system catalog concurrently
+REINDEX INDEX CONCURRENTLY pg_class_relname_nsp_index; -- mapped, non-shared, non-critical
+ERROR: cannot reindex a system catalog concurrently
+REINDEX INDEX CONCURRENTLY pg_index_indexrelid_index; -- non-mapped, non-shared, critical
+ERROR: cannot reindex a system catalog concurrently
+REINDEX INDEX CONCURRENTLY pg_index_indrelid_index; -- non-mapped, non-shared, non-critical
+ERROR: cannot reindex a system catalog concurrently
+REINDEX INDEX CONCURRENTLY pg_database_oid_index; -- mapped, shared, critical
+ERROR: cannot reindex a system catalog concurrently
+REINDEX INDEX CONCURRENTLY pg_shdescription_o_c_index; -- mapped, shared, non-critical
+ERROR: cannot reindex a system catalog concurrently
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index f29b8ca826..7bcc79b458 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -838,8 +838,6 @@ 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 SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM
-- Warns about catalog relations
REINDEX SCHEMA CONCURRENTLY pg_catalog;
diff --git a/src/test/regress/sql/reindex_catalog.sql b/src/test/regress/sql/reindex_catalog.sql
index 2180ee5791..72d2996132 100644
--- a/src/test/regress/sql/reindex_catalog.sql
+++ b/src/test/regress/sql/reindex_catalog.sql
@@ -34,3 +34,18 @@ REINDEX INDEX pg_index_indexrelid_index; -- non-mapped, non-shared, critical
REINDEX INDEX pg_index_indrelid_index; -- non-mapped, non-shared, non-critical
REINDEX INDEX pg_database_oid_index; -- mapped, shared, critical
REINDEX INDEX pg_shdescription_o_c_index; -- mapped, shared, non-critical
+
+-- Check that REINDEX CONCURRENTLY is forbidden with system catalogs.
+-- For whole tables.
+REINDEX TABLE CONCURRENTLY pg_class; -- mapped, non-shared, critical
+REINDEX TABLE CONCURRENTLY pg_index; -- non-mapped, non-shared, critical
+REINDEX TABLE CONCURRENTLY pg_operator; -- non-mapped, non-shared, critical
+REINDEX TABLE CONCURRENTLY pg_database; -- mapped, shared, critical
+REINDEX TABLE CONCURRENTLY pg_shdescription; -- mapped, shared non-critical
+-- For system indexes
+REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- mapped, non-shared, critical
+REINDEX INDEX CONCURRENTLY pg_class_relname_nsp_index; -- mapped, non-shared, non-critical
+REINDEX INDEX CONCURRENTLY pg_index_indexrelid_index; -- non-mapped, non-shared, critical
+REINDEX INDEX CONCURRENTLY pg_index_indrelid_index; -- non-mapped, non-shared, non-critical
+REINDEX INDEX CONCURRENTLY pg_database_oid_index; -- mapped, shared, critical
+REINDEX INDEX CONCURRENTLY pg_shdescription_o_c_index; -- mapped, shared, non-critical
signature.asc
Description: PGP signature
