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

On Thu, Jan 21, 2021 at 11:48:08PM +0300, Alexey Kondratov wrote:
> Attached is a new patch set of first two patches, that should resolve all
> the issues raised before (ACL, docs, tests) excepting TOAST. Double thanks
> for suggestion to add more tests with nested partitioning. I have found and
> squashed a huge bug related to the returning back to the default tablespace
> using newly added tests.
>
> Regarding TOAST. Now we skip moving toast indexes or throw error if someone
> wants to move TOAST index directly. I had a look on ALTER TABLE SET
> TABLESPACE and it has a bit complicated logic:
>
> 1) You cannot move TOAST table directly.
> 2) But if you move basic relation that TOAST table belongs to, then they are
> moved altogether.
> 3) Same logic as 2) happens if one does ALTER TABLE ALL IN TABLESPACE ...
>
> That way, ALTER TABLE allows moving TOAST tables (with indexes) implicitly,
> but does not allow doing that explicitly. In the same time I found docs to
> be vague about such behavior it only says:
>
> All tables in the current database in a tablespace can be moved
> by using the ALL IN TABLESPACE ... Note that system catalogs are
> not moved by this command
>
> Changing any part of a system catalog table is not permitted.
>
> So actually ALTER TABLE treats TOAST relations as system sometimes, but
> sometimes not.
>
> From the end user perspective it makes sense to move TOAST with main table
> when doing ALTER TABLE SET TABLESPACE. But should we touch indexes on TOAST
> table with REINDEX? We cannot move TOAST relation itself, since we are doing
> only a reindex, so we end up in the state when TOAST table and its index are
> placed in the different tablespaces. This state is not reachable with ALTER
> TABLE/INDEX, so it seem we should not allow it with REINDEX as well, should
> we?

> + * Even if a table's indexes were moved to a new tablespace, the index
> + * on its toast table is not normally moved.
> */
> ReindexParams newparams = *params;
>
> newparams.options &= ~(REINDEXOPT_MISSING_OK);
> + if (!allowSystemTableMods)
> + newparams.tablespaceOid = InvalidOid;

I think you're right. So actually TOAST should never move, even if
allowSystemTableMods, right ?

> @@ -292,7 +315,11 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN
> with <command>REINDEX INDEX</command> or <command>REINDEX TABLE</command>,
> respectively. Each partition of the specified partitioned relation is
> reindexed in a separate transaction. Those commands cannot be used inside
> - a transaction block when working on a partitioned table or index.
> + a transaction block when working on a partitioned table or index. If
> + <command>REINDEX</command> with <literal>TABLESPACE</literal> executed
> + on partitioned relation fails it may have moved some partitions to the new
> + tablespace. Repeated command will still reindex all partitions even if they
> + are already in the new tablespace.

Minor corrections here:

If a <command>REINDEX</command> command fails when run on a partitioned
relation, and <literal>TABLESPACE</literal> was specified, then it may have
moved indexes on some partitions to the new tablespace. Re-running the command
will reindex all partitions and move previously-unprocessed indexes to the new
tablespace.

--
Justin