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

On Wed, Feb 03, 2021 at 07:54:42PM +0900, Michael Paquier wrote:
> Not sure I like that. Here is a proposal:
> "it is recommended to separately use ALTER TABLE ONLY on them so as
> any new partitions attached inherit the new tablespace value."

So, I have done more work on this stuff today, and applied that as of
c5b2860. While reviewing my changes, I have noticed that I have
managed to break ALTER TABLE SET TABLESPACE which would have failed
when cascading to a toast relation, the extra check placed previously
in CheckRelationTableSpaceMove() being incorrect. The most surprising
part was that we had zero in-core tests to catch this mistake, so I
have added an extra test to cover this scenario while on it.

A second thing I have come back to is allow_system_table_mods for
toast relations, and decided to just forbid TABLESPACE if attempting
to use it directly on a system table even if allow_system_table_mods
is true. This was leading to inconsistent behaviors and weirdness in
the concurrent case because all the indexes are processed in series
after building a list. As we want to ignore the move of toast indexes
when moving the indexes of the parent table, this was leading to extra
conditions that are not really worth supporting after thinking about
it. One other issue was the lack of consistency when using pg_global
that was a no-op for the concurrent case but failed in the
non-concurrent case. I have put in place more regression tests for
all that.

Regarding the VACUUM and CLUSTER cases, I am not completely sure if
going through these for a tablespace case is the best move we can do,
as ALTER TABLE is able to mix multiple operations and all of them
require already an AEL to work. REINDEX was different thanks to the
case of CONCURRENTLY. Anyway, as a lot of work has been done here
already, I would recommend to create new threads about those two
topics. I am also closing this patch in the CF app.
--
Michael