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

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

From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Michael Paquier <michael@paquier.xyz>
Cc: Justin Pryzby <pryzby@telsasoft.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.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-23T22:22:05Z
Lists: pgsql-hackers
On 2020-Dec-23, Michael Paquier wrote:

>  bool
> -reindex_relation(Oid relid, int flags, int options)
> +reindex_relation(Oid relid, int flags, ReindexOptions *options)
>  {
>  	Relation	rel;
>  	Oid			toast_relid;

Wait a minute.  reindex_relation has 'flags' and *also* 'options' with
an embedded 'flags' member?  Surely that's not right.  I see that they
carry orthogonal sets of options ... but why aren't they a single
bitmask instead of two separate ones?  This looks weird and confusing.


Also: it seems a bit weird to me to put the flags inside the options
struct.  I would keep them separate -- so initially the options struct
would only have the tablespace OID, on API cleanliness grounds:

struct ReindexOptions
{
	tablepaceOid	oid;
};
extern bool
reindex_relation(Oid relid, bits32 flags, ReindexOptions *options);

I guess you could argue that it's more performance to set up only two
arguments to the function call instead of three .. but I doubt that's
measurable for anything in DDL-land.

But also, are we really envisioning that these routines would have all
that many additional options?  Maybe it is sufficient to do just

extern bool
reindex_relation(Oid relid, bits32 flags, tablespaceOid Oid);



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.