From c3e94851f6620657ab90bd6f3f3466d862529d4e Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Fri, 28 Feb 2020 19:20:48 -0600
Subject: [PATCH v11 4/5] fixes

---
 src/backend/catalog/index.c               | 27 +++++++++-----
 src/backend/commands/cluster.c            | 21 ++++++++++-
 src/backend/commands/indexcmds.c          | 43 ++++++++++-------------
 src/backend/commands/vacuum.c             |  3 +-
 src/backend/nodes/copyfuncs.c             |  1 +
 src/backend/nodes/equalfuncs.c            |  1 +
 src/backend/parser/gram.y                 |  3 ++
 src/backend/postmaster/autovacuum.c       |  1 +
 src/test/regress/output/tablespace.source | 11 +++---
 9 files changed, 70 insertions(+), 41 deletions(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index e6c06e6a06..05c4b51165 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3434,6 +3434,7 @@ reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks,
 	volatile bool skipped_constraint = false;
 	PGRUsage	ru0;
 	bool		progress = (options & REINDEXOPT_REPORT_PROGRESS) != 0;
+	bool		valid_tablespace = OidIsValid(tablespaceOid);
 
 	pg_rusage_init(&ru0);
 
@@ -3473,14 +3474,19 @@ reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks,
 			 RelationGetRelationName(iRel));
 
 	/*
-	 * We cannot support moving mapped relations into different tablespaces.
-	 * (In particular this eliminates all shared catalogs.)
+	 * We don't support moving system relations into different tablespaces.
 	 */
-	if (OidIsValid(tablespaceOid) && RelationIsMapped(iRel))
+	if (OidIsValid(tablespaceOid) &&
+			((IsCatalogRelationOid(indexId) && !allowSystemTableMods)
+			|| RelationIsMapped(iRel)))
+	{
+		check_relation_is_movable(iRel, tablespaceOid);
 		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("cannot move system relation \"%s\"",
-						RelationGetRelationName(iRel))));
+				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+				 errmsg("permission denied: \"%s\" is a system catalog",
+					 RelationGetRelationName(iRel)),
+				 errdetail("cannot change tablespace of system catalog")));
+	}
 
 	/*
 	 * Don't allow reindex on temp tables of other backends ... their local
@@ -3508,11 +3514,14 @@ reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks,
 	 */
 	CheckTableNotInUse(iRel, "REINDEX INDEX");
 
+	tablespaceOid = (tablespaceOid == MyDatabaseTableSpace) ?
+		InvalidOid : tablespaceOid;
+
 	/*
 	 * Set the new tablespace for the relation.  Do that only in the
 	 * case where the reindex caller wishes to enforce a new tablespace.
 	 */
-	if (OidIsValid(tablespaceOid) &&
+	if (valid_tablespace &&
 		tablespaceOid != iRel->rd_rel->reltablespace)
 	{
 		Relation		pg_class;
@@ -3535,8 +3544,8 @@ reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks,
 		RelationDropStorage(iRel);
 
 		/* Update the pg_class row */
-		rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
-			InvalidOid : tablespaceOid;
+		rd_rel->reltablespace = tablespaceOid;
+
 		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
 
 		heap_freetuple(tuple);
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 794d4be30c..fdc62a1996 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -383,6 +383,18 @@ cluster_rel(Oid tableOid, Oid indexOid, Oid tableSpaceOid, int options)
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot cluster a shared catalog")));
 
+	if (!allowSystemTableMods && IsSystemRelation(OldHeap) &&
+		OidIsValid(tableSpaceOid))
+		// tableSpaceOid != OldHeap->rd_rel->reltablespace)
+	{
+		check_relation_is_movable(OldHeap, tableSpaceOid);
+		ereport(ERROR,
+				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+				 errmsg("permission denied: \"%s\" is a system catalog",
+					 RelationGetRelationName(OldHeap)),
+				errdetail("cannot change tablespace of system catalog")));
+	}
+
 	/*
 	 * Don't process temp tables of other backends ... their local buffer
 	 * manager is not going to cope.
@@ -603,7 +615,7 @@ rebuild_relation(Relation OldHeap, Oid indexOid, Oid NewTableSpaceOid, bool verb
 	TransactionId frozenXid;
 	MultiXactId cutoffMulti;
 
-	/* Use new TeableSpace if passed. */
+	/* Use new TableSpace if passed. */
 	if (OidIsValid(NewTableSpaceOid))
 		tableSpace = NewTableSpaceOid;
 
@@ -1051,6 +1063,13 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
 		 */
 		Assert(!target_is_pg_class);
 
+		if (!allowSystemTableMods && IsSystemClass(r1, relform1) &&
+				relform1->reltablespace != relform2->reltablespace)
+			ereport(ERROR,
+					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+					 errmsg("permission denied: \"%s\" is a system catalog",
+						 get_rel_name(r1))));
+
 		swaptemp = relform1->relfilenode;
 		relform1->relfilenode = relform2->relfilenode;
 		relform2->relfilenode = swaptemp;
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 1d394d5248..448e2f9054 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2521,7 +2521,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, char
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
-	bool		mapped_warning = false;
+	bool		tblspc_warning = false;
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
@@ -2562,17 +2562,8 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, char
 
 	/* Define new tablespaceOid if it is wanted by caller */
 	if (newTableSpaceName)
-	{
 		tablespaceOid = get_tablespace_oid(newTableSpaceName, false);
 
-		/* Can't move a non-shared relation into pg_global */
-		if (tablespaceOid == GLOBALTABLESPACE_OID)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("cannot move non-shared relation to tablespace \"%s\"",
-							newTableSpaceName)));
-	}
-
 	/*
 	 * Create a memory context that will survive forced transaction commits we
 	 * do below.  Since it is a child of PortalContext, it will go away
@@ -2664,18 +2655,18 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, char
 		}
 
 		/*
-		 * Skip all mapped relations if TABLESPACE is specified.
-		 * relfilenode == 0 checks after that, similarly to
-		 * RelationIsMapped().
+		 * Skip all system relations if TABLESPACE is specified.
+		 * Skip mapped relations (relfilenode=0) even if allowing system table mods.
 		 */
 		if (OidIsValid(tablespaceOid) &&
-			!OidIsValid(classtuple->relfilenode))
+			((!allowSystemTableMods && IsSystemClass(relid, classtuple)) ||
+			 !OidIsValid(classtuple->relfilenode)))
 		{
-			if (!mapped_warning)
+			if (!tblspc_warning)
 				ereport(WARNING,
 						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-						 errmsg("cannot change tablespace of indexes for mapped relations, skipping all")));
-			mapped_warning = true;
+						 errmsg("cannot change tablespace of indexes on system catalogs, skipping all")));
+			tblspc_warning = true;
 			continue;
 		}
 
@@ -2843,11 +2834,12 @@ ReindexRelationConcurrently(Oid relationOid, Oid tablespaceOid, int options)
 				/* Open relation to get its indexes */
 				heapRelation = table_open(relationOid, ShareUpdateExclusiveLock);
 
-				if (OidIsValid(tablespaceOid) && RelationIsMapped(heapRelation))
+				if (OidIsValid(tablespaceOid) && IsSystemRelation(heapRelation))
 					ereport(ERROR,
-							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-							 errmsg("cannot change tablespace of indexes for mapped relation \"%s\"",
-									RelationGetRelationName(heapRelation))));
+							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+							 errmsg("permission denied: \"%s\" is a system catalog",
+								 RelationGetRelationName(heapRelation)),
+							 errdetail("cannot change tablespace of system catalog")));
 
 				/* Add all the valid indexes of relation to list */
 				foreach(lc, RelationGetIndexList(heapRelation))
@@ -3031,11 +3023,12 @@ ReindexRelationConcurrently(Oid relationOid, Oid tablespaceOid, int options)
 		if (indexRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
 			elog(ERROR, "cannot reindex a temporary table concurrently");
 
-		if (OidIsValid(tablespaceOid) && RelationIsMapped(heapRel))
+		if (OidIsValid(tablespaceOid) && IsSystemRelation(heapRel))
 			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("cannot change tablespace of mapped relation \"%s\"",
-							RelationGetRelationName(heapRel))));
+					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+					 errmsg("permission denied: \"%s\" is a system catalog",
+						 RelationGetRelationName(heapRel)),
+						 errdetail("cannot change tablespace of system catalog")));
 
 		pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
 									  RelationGetRelid(heapRel));
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 6dfdce32eb..095dbbce96 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1827,7 +1827,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	/*
 	 * We cannot support moving system relations into different tablespaces.
 	 */
-	if (params->options & VACOPT_FULL && OidIsValid(params->tablespace_oid) && IsSystemRelation(onerel))
+	if (params->options & VACOPT_FULL && OidIsValid(params->tablespace_oid)
+			&& IsSystemRelation(onerel))
 	{
 		ereport(WARNING,
 			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 7bf9aa7e75..891bc37b08 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3901,6 +3901,7 @@ _copyVacuumStmt(const VacuumStmt *from)
 	COPY_NODE_FIELD(options);
 	COPY_NODE_FIELD(rels);
 	COPY_SCALAR_FIELD(is_vacuumcmd);
+	COPY_STRING_FIELD(tablespacename);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 048c59fd68..da55563ca8 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1702,6 +1702,7 @@ _equalVacuumStmt(const VacuumStmt *a, const VacuumStmt *b)
 	COMPARE_NODE_FIELD(options);
 	COMPARE_NODE_FIELD(rels);
 	COMPARE_SCALAR_FIELD(is_vacuumcmd);
+	COMPARE_SCALAR_FIELD(tablespacename);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 1f388017be..300e6409f8 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -10615,6 +10615,7 @@ ClusterStmt:
 					n->options = 0;
 					if ($2)
 						n->options |= CLUOPT_VERBOSE;
+					n->tablespacename = NULL;
 					$$ = (Node*)n;
 				}
 			/* kept for pre-8.3 compatibility */
@@ -10718,6 +10719,7 @@ AnalyzeStmt: analyze_keyword opt_verbose opt_vacuum_relation_list
 											 makeDefElem("verbose", NULL, @2));
 					n->rels = $3;
 					n->is_vacuumcmd = false;
+					n->tablespacename = NULL;
 					$$ = (Node *)n;
 				}
 			| analyze_keyword '(' vac_analyze_option_list ')' opt_vacuum_relation_list
@@ -10726,6 +10728,7 @@ AnalyzeStmt: analyze_keyword opt_verbose opt_vacuum_relation_list
 					n->options = $3;
 					n->rels = $5;
 					n->is_vacuumcmd = false;
+					n->tablespacename = NULL;
 					$$ = (Node *) n;
 				}
 		;
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index da75e755f0..ad538a872f 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2895,6 +2895,7 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
 		tab->at_params.multixact_freeze_table_age = multixact_freeze_table_age;
 		tab->at_params.is_wraparound = wraparound;
 		tab->at_params.log_min_duration = log_min_duration;
+		tab->at_params.tablespace_oid = InvalidOid;
 		tab->at_vacuum_cost_limit = vac_cost_limit;
 		tab->at_vacuum_cost_delay = vac_cost_delay;
 		tab->at_relname = NULL;
diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source
index 5634cf20ff..0efe6cf4cc 100644
--- a/src/test/regress/output/tablespace.source
+++ b/src/test/regress/output/tablespace.source
@@ -33,7 +33,8 @@ REINDEX TABLE regress_tblspace_test_tbl TABLESPACE regress_tblspace;
 ROLLBACK;
 BEGIN;
 REINDEX TABLE pg_am TABLESPACE regress_tblspace;
-ERROR:  cannot move system relation "pg_am_name_index"
+ERROR:  permission denied: "pg_am_name_index" is a system catalog
+DETAIL:  cannot change tablespace of system catalog
 ROLLBACK;
 SELECT relname FROM pg_class
 WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
@@ -43,9 +44,9 @@ WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspa
 
 -- first, let us reindex and move the entire database, after that return everything back
 REINDEX DATABASE regression TABLESPACE regress_tblspace; -- ok with warning
-WARNING:  cannot change tablespace of indexes for mapped relations, skipping all
+WARNING:  cannot change tablespace of indexes on system catalogs, skipping all
 REINDEX DATABASE regression TABLESPACE pg_default; -- ok with warning
-WARNING:  cannot change tablespace of indexes for mapped relations, skipping all
+WARNING:  cannot change tablespace of indexes on system catalogs, skipping all
 SELECT relname FROM pg_class
 WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
  relname 
@@ -58,7 +59,7 @@ REINDEX TABLE regress_tblspace_test_tbl TABLESPACE regress_tblspace; -- ok
 REINDEX TABLE pg_authid TABLESPACE regress_tblspace; -- fail
 ERROR:  cannot move system relation "pg_authid_rolname_index"
 REINDEX SCHEMA pg_catalog TABLESPACE regress_tblspace; -- fail
-WARNING:  cannot change tablespace of indexes for mapped relations, skipping all
+WARNING:  cannot change tablespace of indexes on system catalogs, skipping all
 REINDEX DATABASE postgres TABLESPACE regress_tblspace; -- fail
 ERROR:  can only reindex the currently open database
 REINDEX SYSTEM postgres TABLESPACE regress_tblspace; -- fail
@@ -66,7 +67,7 @@ ERROR:  can only reindex the currently open database
 REINDEX INDEX regress_tblspace_test_tbl_idx TABLESPACE pg_global; -- fail
 ERROR:  cannot move non-shared relation to tablespace "pg_global"
 REINDEX SCHEMA pg_catalog TABLESPACE regress_tblspace; -- fail
-WARNING:  cannot change tablespace of indexes for mapped relations, skipping all
+WARNING:  cannot change tablespace of indexes on system catalogs, skipping all
 REINDEX DATABASE postgres TABLESPACE regress_tblspace; -- fail
 ERROR:  can only reindex the currently open database
 REINDEX SYSTEM postgres TABLESPACE regress_tblspace; -- fail
-- 
2.17.0

