From e24a815c1c8550fcba5cc5aeb0d130db46570872 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Wed, 5 Jun 2019 21:05:41 +1200 Subject: [PATCH] Fix confusing NOTICE text in REINDEX CONCURRENTLY When performing REINDEX TABLE CONCURRENTLY, if all of the table's indexes could not be reindexed, a NOTICE message claimed that the table had no indexes. This was confusing, so let's change the NOTICE text to something less confusing. In passing, also mention in the comment before ReindexRelationConcurrently that materialized views are supported too and also explain what the return value of the function means. Author: Ashwin Agrawal Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/CALfoeithHvi13p_VyR8kt9o6Pa7Z=Smi6Nfc2anHnQx5Lj8bTQ@mail.gmail.com --- src/backend/commands/indexcmds.c | 35 +++++++++++++++------- src/test/regress/expected/create_index.out | 10 +++---- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 4d76da8293..d05d2fd3d5 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -2444,17 +2444,25 @@ ReindexTable(RangeVar *relation, int options, bool concurrent) RangeVarCallbackOwnsTable, NULL); if (concurrent) + { result = ReindexRelationConcurrently(heapOid, options); + + if (!result) + ereport(NOTICE, + (errmsg("table \"%s\" has no indexes that can be reindexed concurrently", + relation->relname))); + } else + { result = reindex_relation(heapOid, REINDEX_REL_PROCESS_TOAST | REINDEX_REL_CHECK_CONSTRAINTS, options); - - if (!result) - ereport(NOTICE, - (errmsg("table \"%s\" has no indexes", - relation->relname))); + if (!result) + ereport(NOTICE, + (errmsg("table \"%s\" has no indexes to reindex", + relation->relname))); + } return heapOid; } @@ -2636,7 +2644,6 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, foreach(l, relids) { Oid relid = lfirst_oid(l); - bool result; StartTransactionCommand(); /* functions in indexes may want a snapshot set */ @@ -2644,11 +2651,13 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, if (concurrent) { - result = ReindexRelationConcurrently(relid, options); + (void) ReindexRelationConcurrently(relid, options); /* ReindexRelationConcurrently() does the verbose output */ } else { + bool result; + result = reindex_relation(relid, REINDEX_REL_PROCESS_TOAST | REINDEX_REL_CHECK_CONSTRAINTS, @@ -2675,13 +2684,19 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, * ReindexRelationConcurrently - process REINDEX CONCURRENTLY for given * relation OID * - * The relation can be either an index or a table. If it is a table, all its - * valid indexes will be rebuilt, including its associated toast table - * indexes. If it is an index, this index itself will be rebuilt. + * 'relationOid' can either belong to an index, a table or a materialized + * view. For tables and materialized views, all its indexes will be rebuilt, + * excluding invalid indexes and any indexes used in exclusion constraints, + * but including its associated toast table indexes. For indexes, the index + * itself will be rebuilt. If 'relationOid' belongs to a partitioned table + * then we issue a warning to mention these are not yet supported. * * The locks taken on parent tables and involved indexes are kept until the * transaction is committed, at which point a session lock is taken on each * relation. Both of these protect against concurrent schema changes. + * + * Returns true if any indexes have been rebuilt (including toast table's + * indexes, when relevant), otherwise returns false. */ static bool ReindexRelationConcurrently(Oid relationOid, int options) diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index c8bc6be061..c30e6738ba 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -1944,9 +1944,9 @@ DROP TABLE reindex_verbose; CREATE TABLE concur_reindex_tab (c1 int); -- REINDEX REINDEX TABLE concur_reindex_tab; -- notice -NOTICE: table "concur_reindex_tab" has no indexes +NOTICE: table "concur_reindex_tab" has no indexes to reindex REINDEX TABLE CONCURRENTLY concur_reindex_tab; -- notice -NOTICE: table "concur_reindex_tab" has no indexes +NOTICE: table "concur_reindex_tab" has no indexes that can be reindexed concurrently ALTER TABLE concur_reindex_tab ADD COLUMN c2 text; -- add toast index -- Normal index with integer column CREATE UNIQUE INDEX concur_reindex_ind1 ON concur_reindex_tab(c1); @@ -2043,10 +2043,10 @@ ERROR: REINDEX is not yet implemented for partitioned indexes -- REINDEX is a no-op for partitioned tables REINDEX TABLE concur_reindex_part_10; WARNING: REINDEX of partitioned tables is not yet implemented, skipping "concur_reindex_part_10" -NOTICE: table "concur_reindex_part_10" has no indexes +NOTICE: table "concur_reindex_part_10" has no indexes to reindex REINDEX TABLE CONCURRENTLY concur_reindex_part_10; WARNING: REINDEX of partitioned tables is not yet implemented, skipping "concur_reindex_part_10" -NOTICE: table "concur_reindex_part_10" has no indexes +NOTICE: table "concur_reindex_part_10" has no indexes that can be reindexed concurrently SELECT relid, parentrelid, level FROM pg_partition_tree('concur_reindex_part_index') ORDER BY relid, level; relid | parentrelid | level @@ -2150,7 +2150,7 @@ 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 invalid index "public.concur_reindex_ind5" concurrently, skipping -NOTICE: table "concur_reindex_tab4" has no indexes +NOTICE: table "concur_reindex_tab4" has no indexes that can be reindexed concurrently \d concur_reindex_tab4 Table "public.concur_reindex_tab4" Column | Type | Collation | Nullable | Default