On Sat, May 25, 2019 at 02:42:59PM +1200, David Rowley wrote: > Also, I think people probably will care more about the fact that > nothing was done for that table rather than if the table happens to > have no indexes. For the non-concurrently case, that just happened to > be the same thing.
This is equally confusing for plain REINDEX as well, no? Taking your previous example: =# REINDEX TABLE listp; WARNING: 0A000: REINDEX of partitioned tables is not yet implemented, skipping "listp" LOCATION: reindex_relation, index.c:3513 NOTICE: 00000: table "listp" has no indexes LOCATION: ReindexTable, indexcmds.c:2452 REINDEX In this case the relation has partitioned indexes, not indexes, so that's actually correct. Still it seems to me that some users could get confused by the current wording. For invalid indexes you would get that: =# create table aa (a int); CREATE TABLE =# insert into aa values (1),(1); INSERT 0 2 =# create unique index concurrently aai on aa(a); ERROR: 23505: could not create unique index "aai" DETAIL: Key (a)=(1) is duplicated. SCHEMA NAME: public TABLE NAME: aa CONSTRAINT NAME: aai LOCATION: comparetup_index_btree, tuplesort.c:405 =# reindex table concurrently aa; WARNING: 0A000: cannot reindex invalid index "public.aai" concurrently, skipping LOCATION: ReindexRelationConcurrently, indexcmds.c:2772 NOTICE: 00000: table "aa" has no indexes LOCATION: ReindexTable, indexcmds.c:2452 REINDEX As you mention for reindex_relation() no indexes <=> nothing to do, still let's not rely on that. Instead of making the error message specific to concurrent operations, I would suggest to change it to "table foo has no indexes to reindex". What do you think about the attached? -- Michael
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 40ea629ffe..ac4a32953a 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2447,7 +2447,7 @@ ReindexTable(RangeVar *relation, int options, bool concurrent)
if (!result)
ereport(NOTICE,
- (errmsg("table \"%s\" has no indexes",
+ (errmsg("table \"%s\" has no indexes to reindex",
relation->relname)));
return heapOid;
@@ -2676,6 +2676,9 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
* 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).
*/
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..4b62c9045a 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 to reindex
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 to reindex
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 to reindex
\d concur_reindex_tab4
Table "public.concur_reindex_tab4"
Column | Type | Collation | Nullable | Default
signature.asc
Description: PGP signature
