Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly
Peter Eisentraut <peter.eisentraut@enterprisedb.com>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
To: Michael Paquier <michael@paquier.xyz>,
Justin Pryzby <pryzby@telsasoft.com>
Cc: Alexey Kondratov <a.kondratov@postgrespro.ru>,
Alvaro Herrera <alvherre@2ndquadrant.com>,
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-03T19:46:09Z
Lists: pgsql-hackers
A side comment on this patch: I think using enums as bit mask values is
bad style. So changing this:
-/* Reindex options */
-#define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
-#define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
-#define REINDEXOPT_MISSING_OK (1 << 2) /* skip missing relations */
-#define REINDEXOPT_CONCURRENTLY (1 << 3) /* concurrent mode */
to this:
+typedef enum ReindexOption
+{
+ REINDEXOPT_VERBOSE = 1 << 0, /* print progress info */
+ REINDEXOPT_REPORT_PROGRESS = 1 << 1, /* report pgstat progress */
+ REINDEXOPT_MISSING_OK = 1 << 2, /* skip missing relations */
+ REINDEXOPT_CONCURRENTLY = 1 << 3 /* concurrent mode */
+} ReindexOption;
seems wrong.
There are a couple of more places like this, including the existing
ClusterOption that this patched moved around, but we should be removing
those.
My reasoning is that if you look at an enum value of this type, either
say in a switch statement or a debugger, the enum value might not be any
of the defined symbols. So that way you lose all the type checking that
an enum might give you.
Let's just keep the #define's like it is done in almost all other places.
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