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

On Thu, Feb 04, 2021 at 03:38:39PM +0900, Michael Paquier wrote:
> 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.

> 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.

Isn't this dead code ?

postgres=# REINDEX (CONCURRENTLY, TABLESPACE pg_global) TABLE pg_class;
ERROR:  0A000: cannot reindex system catalogs concurrently
LOCATION:  ReindexRelationConcurrently, indexcmds.c:3276

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 127ba7835d..c77a9b2563 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -3260,73 +3260,66 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 			{
 				if (IsCatalogRelationOid(relationOid))
 					ereport(ERROR,
 							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 							 errmsg("cannot reindex system catalogs concurrently")));
 
...
 
-				if (OidIsValid(params->tablespaceOid) &&
-					IsSystemRelation(heapRelation))
-					ereport(ERROR,
-							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-							 errmsg("cannot move system relation \"%s\"",
-									RelationGetRelationName(heapRelation))));
-
@@ -3404,73 +3397,66 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
                                if (IsCatalogRelationOid(heapId))
 					ereport(ERROR,
 							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 							 errmsg("cannot reindex system catalogs concurrently")));
... 
 
-				if (OidIsValid(params->tablespaceOid) &&
-					IsSystemRelation(heapRelation))
-					ereport(ERROR,
-							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-							 errmsg("cannot move system relation \"%s\"",
-									get_rel_name(relationOid))));
-