Horton

Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly

Alvaro Herrera <alvherre@alvh.no-ip.org>

View thread View on postgr.es
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Michael Paquier <michael@paquier.xyz>
Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Justin Pryzby <pryzby@telsasoft.com>, Alexey Kondratov <a.kondratov@postgrespro.ru>, Masahiko Sawada <masahiko.sawada@2ndquadrant.com>, Steve Singer <steve@ssinger.info>, pgsql-hackers@lists.postgresql.org, 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-12-04T19:28:26Z
Lists: pgsql-hackers 
On 2020-Dec-04, Michael Paquier wrote:

> VacuumOption does that since 6776142, and ClusterOption since 9ebe057,
> so switching ReindexOption to just match the two others still looks
> like the most consistent move.

9ebe057 goes to show why this is a bad idea, since it has this:

+typedef enum ClusterOption
+{
+   CLUOPT_RECHECK,             /* recheck relation state */
+   CLUOPT_VERBOSE              /* print progress info */
+} ClusterOption;

and then you do things like

+                   if ($2)
+                       n->options |= CLUOPT_VERBOSE;

and then tests like

+       if ((options & VACOPT_VERBOSE) != 0)

Now if you were to ever define third and fourth values in that enum,
this would immediately start malfunctioning.

FWIW I'm with Peter on this.

Commits

  1. Add TABLESPACE option to REINDEX

  2. Refactor code in tablecmds.c to check and process tablespace moves

  3. Refactor option handling of CLUSTER, REINDEX and VACUUM

  4. pg_dump: Don't use enums for defining bit mask values

  5. Refactor CLUSTER and REINDEX grammar to use DefElem for option lists

  6. Refactor parsing rules for option lists of EXPLAIN, VACUUM and ANALYZE

  7. Improve tab completion of REINDEX in psql

  8. Fix possible crash during FATAL exit from reindexing.