Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly
Justin Pryzby <pryzby@telsasoft.com>
From: Justin Pryzby <pryzby@telsasoft.com>
To: Alexey Kondratov <a.kondratov@postgrespro.ru>
Cc: Michael Paquier <michael@paquier.xyz>, Masahiko Sawada <masahiko.sawada@2ndquadrant.com>, Steve Singer <steve@ssinger.info>, pgsql-hackers@lists.postgresql.org, Alvaro Herrera <alvherre@2ndquadrant.com>, Robert Haas <robertmhaas@gmail.com>, Alexander Korotkov <a.korotkov@postgrespro.ru>, Masahiko Sawada <sawada.mshk@gmail.com>, Jose Luis Tallon <jltallon@adv-solutions.net>
Date: 2020-02-11T16:48:48Z
Lists: pgsql-hackers
For your v7 patch, which handles REINDEX to a new tablespace, I have a few minor comments: + * the relation will be rebuilt. If InvalidOid is used, the default => should say "currrent", not default ? +++ b/doc/src/sgml/ref/reindex.sgml + <term><literal>TABLESPACE</literal></term> ... + <term><replaceable class="parameter">new_tablespace</replaceable></term> => I saw you split the description of TABLESPACE from new_tablespace based on comment earlier in the thread, but I suggest that the descriptions for these should be merged, like: + <varlistentry> + <term><literal>TABLESPACE</literal><replaceable class="parameter">new_tablespace</replaceable></term> + <listitem> + <para> + Allow specification of a tablespace where all rebuilt indexes will be created. + Cannot be used with "mapped" relations. If <literal>SCHEMA</literal>, + <literal>DATABASE</literal> or <literal>SYSTEM</literal> are specified, then + all unsuitable relations will be skipped and a single <literal>WARNING</literal> + will be generated. + </para> + </listitem> + </varlistentry> The existing patch is very natural, especially the parts in the original patch handling vacuum full and cluster. Those were removed to concentrate on REINDEX, and based on comments that it might be nice if ALTER handled CLUSTER and VACUUM FULL. On a separate thread, I brought up the idea of ALTER using clustered order. Tom pointed out some issues with my implementation, but didn't like the idea, either. So I suggest to re-include the CLUSTER/VAC FULL parts as a separate 0002 patch, the same way they were originally implemented. BTW, I think if "ALTER" were updated to support REINDEX (to allow multiple operations at once), it might be either: |ALTER INDEX i SET TABLESPACE , REINDEX -- to reindex a single index on a given tlbspc or |ALTER TABLE tbl REINDEX USING INDEX TABLESPACE spc; -- to reindex all inds on table inds moved to a given tblspc "USING INDEX TABLESPACE" is already used for ALTER..ADD column/table CONSTRAINT. -- Justin
Commits
-
Add TABLESPACE option to REINDEX
- c5b286047cd6 14.0 landed
-
Refactor code in tablecmds.c to check and process tablespace moves
- 4c9c359d38ff 14.0 landed
-
Refactor option handling of CLUSTER, REINDEX and VACUUM
- a3dc926009be 14.0 landed
-
pg_dump: Don't use enums for defining bit mask values
- d2a2808eb444 14.0 landed
-
Refactor CLUSTER and REINDEX grammar to use DefElem for option lists
- b5913f612079 14.0 landed
-
Refactor parsing rules for option lists of EXPLAIN, VACUUM and ANALYZE
- 873ea9ee692e 14.0 landed
-
Improve tab completion of REINDEX in psql
- 1f75b454134c 14.0 landed
-
Fix possible crash during FATAL exit from reindexing.
- d12bdba77b0f 13.0 cited