From 9fb2558ecac23ec7b77ab75a6769abe1c2584104 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Fri, 28 Feb 2020 19:20:48 -0600 Subject: [PATCH v10 4/5] fixes --- src/backend/catalog/index.c | 17 +++++---- 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, 63 insertions(+), 38 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 3d98e9164a..4942a5513c 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -3473,14 +3473,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 diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index dcede5e908..dcb4576d9c 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 a0db21c1ea..31bd1b72e7 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)) @@ -3021,11 +3013,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 1a0ca44da8..a3e0e074ec 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 e3a43d3296..b6ae2d2804 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