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