Horton

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

Justin Pryzby <pryzby@telsasoft.com>

View thread View on postgr.es
From: Justin Pryzby <pryzby@telsasoft.com>
To: Zhihong Yu <zyu@yugabyte.com>
Cc: Michael Paquier <michael@paquier.xyz>, Alvaro Herrera <alvherre@alvh.no-ip.org>, 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-23T05:22:00Z
Lists: pgsql-hackers 
On Tue, Dec 22, 2020 at 03:22:19PM -0800, Zhihong Yu wrote:
> Justin:
> For reindex_index() :
> 
> +   if (options->tablespaceOid == MyDatabaseTableSpace)
> +       options->tablespaceOid = InvalidOid;
> ...
> +   oldTablespaceOid = iRel->rd_rel->reltablespace;
> +   if (set_tablespace &&
> +       (options->tablespaceOid != oldTablespaceOid ||
> +       (options->tablespaceOid == MyDatabaseTableSpace &&
> OidIsValid(oldTablespaceOid))))
> 
> I wonder why the options->tablespaceOid == MyDatabaseTableSpace clause
> appears again in the second if statement.
> Since the first if statement would assign InvalidOid
> to options->tablespaceOid when the first if condition is satisfied.

Good question.  Alexey mentioned on Sept 23 that he added the first stanza.  to
avoid storing the DB's tablespace OID (rather than InvalidOid).

I think the 2nd half of the "or" is unnecessary since that was added setting to
options->tablespaceOid = InvalidOid.
If requesting to move to the DB's default tablespace, it'll now hit the first
part of the OR:

> +       (options->tablespaceOid != oldTablespaceOid ||

Without the first stanza setting, it would've hit the 2nd condition:

> +       (options->tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid))))

which means: "user requested to move a table to the DB's default tblspace, and
it was previously on a nondefault space".

So I think we can drop the 2nd half of the OR.  Thanks for noticing.

-- 
Justin

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.