Hello
I review code and documentation and i have few notes. Did you register this
patch in CF app?
I found one error in phase 4. Simple reproducer:
> create table test (i int);
> create index this_is_very_large_exactly_maxnamelen_index_name_wink_wink_wink
> on test (i);
> create index this_is_very_large_exactly_maxnamelen_index_name_wink_winkccold
> on test (i);
> reindex table CONCURRENTLY test;
This fails with error
> ERROR: duplicate key value violates unique constraint
> "pg_class_relname_nsp_index"
> DETAIL: Key (relname,
> relnamespace)=(this_is_very_large_exactly_maxnamelen_index_name_wink_win_ccold,
> 2200) already exists.
CommandCounterIncrement() in (or after) index_concurrently_swap will fix this
issue.
> ReindexPartitionedIndex(Relation parentIdx)
> ereport(ERROR,
> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> errmsg("REINDEX is not yet implemented for partitioned
> indexes")));
I think we need add errhint("you can REINDEX each partition separately") or
something similar.
Also can we omit this warning for reindex database? All partition must be in
same database and warning in such case is useless: we have warning, but doing
reindex for each partition => we reindex partitioned table correctly.
Another behavior issue i found with reindex (verbose) schema/database: INFO
ereport is printed twice for each table.
> INFO: relation "measurement_y2006m02" was reindexed
> DETAIL: CPU: user: 0.00 s, system: 0.00 s, elapsed: 0.07 s.
> INFO: table "public.measurement_y2006m02" was reindexed
One from ReindexMultipleTables and another (with pg_rusage_show) from
ReindexRelationConcurrently.
> ReindexRelationConcurrently
> if (!indexRelation->rd_index->indisvalid)
it is better use IndexIsValid macro here? And same question about added
indexform->indisvalid in src/backend/commands/tablecmds.c
> <para>
> An index build with the <literal>CONCURRENTLY</literal> option failed,
> leaving
> an <quote>invalid</quote> index. Such indexes are useless but it can be
>- convenient to use <command>REINDEX</command> to rebuild them. Note that
>- <command>REINDEX</command> will not perform a concurrent build. To
>build the
>- index without interfering with production you should drop the index and
>- reissue the <command>CREATE INDEX CONCURRENTLY</command> command.
>+ convenient to use <command>REINDEX</command> to rebuild them.
> </para>
This documentation change seems wrong for me: reindex concurrently does not
rebuild invalid indexes. To fix invalid indexes we still need reindex with lock
table or recreate this index concurrently.
> + A first pass to build the index is done for each new index entry.
> + Once the index is built, its flag <literal>pg_class.isready</literal>
> is
> + switched to <quote>true</quote>
> + At this point <literal>pg_class.indisvalid</literal> is switched to
> + <quote>true</quote> for the new index and to <quote>false</quote> for
> the old, and
> + Old indexes have <literal>pg_class.isready</literal> switched to
> <quote>false</quote>
Should be pg_index.indisvalid and pg_index.indisready, right?
regards, Sergei
