Refactor code in tablecmds.c to check and process tablespace moves

Two code paths of tablecmds.c (for relations with storage and without
storage) use the same logic to check if the move of a relation to a
new tablespace is allowed or not and to update pg_class.reltablespace
and pg_class.relfilenode.

A potential TABLESPACE clause for REINDEX, CLUSTER and VACUUM FULL needs
similar checks to make sure that nothing is moved around in illegal ways
(no mapped relations, shared relations only in pg_global, no move of
temp tables owned by other backends).

This reorganizes the existing code of ALTER TABLE so as all this logic
is controlled by two new routines that can be reused for the other
commands able to move relations across tablespaces, limiting the number
of code paths in need of the same protections.  This also removes some
code that was duplicated for tables with and without storage for ALTER
TABLE.

Author: Alexey Kondratov, Michael Paquier
Discussion: https://postgr.es/m/[email protected]

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8687e9a97c5bbbf92b39eca438856caa9b1d27b1..36747e7277da41af34a9c23817d47671a8cf91d6 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -3037,6 +3037,113 @@ SetRelationHasSubclass(Oid relationId, bool relhassubclass)
    table_close(relationRelation, RowExclusiveLock);
 }
 
+/*
+ * CheckRelationTableSpaceMove
+ *     Check if relation can be moved to new tablespace.
+ *
+ * NOTE: Caller must be holding an appropriate lock on the relation.
+ * ShareUpdateExclusiveLock is sufficient.
+ *
+ * Returns true if the relation can be moved to the new tablespace;
+ * false otherwise.
+ */
+bool
+CheckRelationTableSpaceMove(Relation rel, Oid newTableSpaceId)
+{
+   Oid         oldTableSpaceId;
+
+   /*
+    * No work if no change in tablespace.  Note that MyDatabaseTableSpace is
+    * stored as 0.
+    */
+   oldTableSpaceId = rel->rd_rel->reltablespace;
+   if (newTableSpaceId == oldTableSpaceId ||
+       (newTableSpaceId == MyDatabaseTableSpace && oldTableSpaceId == 0))
+       return false;
+
+   /*
+    * We cannot support moving mapped relations into different tablespaces.
+    * (In particular this eliminates all shared catalogs.)
+    */
+   if (RelationIsMapped(rel))
+       ereport(ERROR,
+               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                errmsg("cannot move system relation \"%s\"",
+                       RelationGetRelationName(rel))));
+
+   /* Cannot move a non-shared relation into pg_global */
+   if (newTableSpaceId == GLOBALTABLESPACE_OID)
+       ereport(ERROR,
+               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("only shared relations can be placed in pg_global tablespace")));
+
+   /*
+    * Do not allow moving temp tables of other backends ... their local
+    * buffer manager is not going to cope.
+    */
+   if (RELATION_IS_OTHER_TEMP(rel))
+       ereport(ERROR,
+               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                errmsg("cannot move temporary tables of other sessions")));
+
+   return true;
+}
+
+/*
+ * SetRelationTableSpace
+ *     Set new reltablespace and relfilenode in pg_class entry.
+ *
+ * newTableSpaceId is the new tablespace for the relation, and
+ * newRelFileNode its new filenode.  If newrelfilenode is InvalidOid,
+ * this field is not updated.
+ *
+ * NOTE: Caller must be holding an appropriate lock on the relation.
+ * ShareUpdateExclusiveLock is sufficient.
+ *
+ * The caller of this routine had better check if a relation can be
+ * moved to this new tablespace by calling CheckRelationTableSpaceMove()
+ * first, and is responsible for making the change visible with
+ * CommandCounterIncrement().
+ */
+void
+SetRelationTableSpace(Relation rel,
+                     Oid newTableSpaceId,
+                     Oid newRelFileNode)
+{
+   Relation    pg_class;
+   HeapTuple   tuple;
+   Form_pg_class rd_rel;
+   Oid         reloid = RelationGetRelid(rel);
+
+   Assert(CheckRelationTableSpaceMove(rel, newTableSpaceId));
+
+   /* Get a modifiable copy of the relation's pg_class row. */
+   pg_class = table_open(RelationRelationId, RowExclusiveLock);
+
+   tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
+   if (!HeapTupleIsValid(tuple))
+       elog(ERROR, "cache lookup failed for relation %u", reloid);
+   rd_rel = (Form_pg_class) GETSTRUCT(tuple);
+
+   /* Update the pg_class row. */
+   rd_rel->reltablespace = (newTableSpaceId == MyDatabaseTableSpace) ?
+       InvalidOid : newTableSpaceId;
+   if (OidIsValid(newRelFileNode))
+       rd_rel->relfilenode = newRelFileNode;
+   CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
+
+   /*
+    * Record dependency on tablespace.  This is only required for relations
+    * that have no physical storage.
+    */
+   if (!RELKIND_HAS_STORAGE(rel->rd_rel->relkind))
+       changeDependencyOnTablespace(RelationRelationId, reloid,
+                                    rd_rel->reltablespace);
+
+   heap_freetuple(tuple);
+   table_close(pg_class, RowExclusiveLock);
+}
+
 /*
  *     renameatt_check         - basic sanity checks before attribute rename
  */
@@ -13160,13 +13267,9 @@ static void
 ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
 {
    Relation    rel;
-   Oid         oldTableSpace;
    Oid         reltoastrelid;
    Oid         newrelfilenode;
    RelFileNode newrnode;
-   Relation    pg_class;
-   HeapTuple   tuple;
-   Form_pg_class rd_rel;
    List       *reltoastidxids = NIL;
    ListCell   *lc;
 
@@ -13175,45 +13278,15 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
     */
    rel = relation_open(tableOid, lockmode);
 
-   /*
-    * No work if no change in tablespace.
-    */
-   oldTableSpace = rel->rd_rel->reltablespace;
-   if (newTableSpace == oldTableSpace ||
-       (newTableSpace == MyDatabaseTableSpace && oldTableSpace == 0))
+   /* Check first if relation can be moved to new tablespace */
+   if (!CheckRelationTableSpaceMove(rel, newTableSpace))
    {
        InvokeObjectPostAlterHook(RelationRelationId,
                                  RelationGetRelid(rel), 0);
-
        relation_close(rel, NoLock);
        return;
    }
 
-   /*
-    * We cannot support moving mapped relations into different tablespaces.
-    * (In particular this eliminates all shared catalogs.)
-    */
-   if (RelationIsMapped(rel))
-       ereport(ERROR,
-               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                errmsg("cannot move system relation \"%s\"",
-                       RelationGetRelationName(rel))));
-
-   /* Can't move a non-shared relation into pg_global */
-   if (newTableSpace == GLOBALTABLESPACE_OID)
-       ereport(ERROR,
-               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                errmsg("only shared relations can be placed in pg_global tablespace")));
-
-   /*
-    * Don't allow moving temp tables of other backends ... their local buffer
-    * manager is not going to cope.
-    */
-   if (RELATION_IS_OTHER_TEMP(rel))
-       ereport(ERROR,
-               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                errmsg("cannot move temporary tables of other sessions")));
-
    reltoastrelid = rel->rd_rel->reltoastrelid;
    /* Fetch the list of indexes on toast relation if necessary */
    if (OidIsValid(reltoastrelid))
@@ -13224,14 +13297,6 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
        relation_close(toastRel, lockmode);
    }
 
-   /* Get a modifiable copy of the relation's pg_class row */
-   pg_class = table_open(RelationRelationId, RowExclusiveLock);
-
-   tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(tableOid));
-   if (!HeapTupleIsValid(tuple))
-       elog(ERROR, "cache lookup failed for relation %u", tableOid);
-   rd_rel = (Form_pg_class) GETSTRUCT(tuple);
-
    /*
     * Relfilenodes are not unique in databases across tablespaces, so we need
     * to allocate a new one in the new tablespace.
@@ -13262,18 +13327,13 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
     *
     * NB: This wouldn't work if ATExecSetTableSpace() were allowed to be
     * executed on pg_class or its indexes (the above copy wouldn't contain
-    * the updated pg_class entry), but that's forbidden above.
+    * the updated pg_class entry), but that's forbidden with
+    * CheckRelationTableSpaceMove().
     */
-   rd_rel->reltablespace = (newTableSpace == MyDatabaseTableSpace) ? InvalidOid : newTableSpace;
-   rd_rel->relfilenode = newrelfilenode;
-   CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
+   SetRelationTableSpace(rel, newTableSpace, newrelfilenode);
 
    InvokeObjectPostAlterHook(RelationRelationId, RelationGetRelid(rel), 0);
 
-   heap_freetuple(tuple);
-
-   table_close(pg_class, RowExclusiveLock);
-
    RelationAssumeNewRelfilenode(rel);
 
    relation_close(rel, NoLock);
@@ -13301,56 +13361,25 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
 static void
 ATExecSetTableSpaceNoStorage(Relation rel, Oid newTableSpace)
 {
-   HeapTuple   tuple;
-   Oid         oldTableSpace;
-   Relation    pg_class;
-   Form_pg_class rd_rel;
-   Oid         reloid = RelationGetRelid(rel);
-
    /*
     * Shouldn't be called on relations having storage; these are processed in
     * phase 3.
     */
    Assert(!RELKIND_HAS_STORAGE(rel->rd_rel->relkind));
 
-   /* Can't allow a non-shared relation in pg_global */
-   if (newTableSpace == GLOBALTABLESPACE_OID)
-       ereport(ERROR,
-               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                errmsg("only shared relations can be placed in pg_global tablespace")));
-
-   /*
-    * No work if no change in tablespace.
-    */
-   oldTableSpace = rel->rd_rel->reltablespace;
-   if (newTableSpace == oldTableSpace ||
-       (newTableSpace == MyDatabaseTableSpace && oldTableSpace == 0))
+   /* check if relation can be moved to its new tablespace */
+   if (!CheckRelationTableSpaceMove(rel, newTableSpace))
    {
-       InvokeObjectPostAlterHook(RelationRelationId, reloid, 0);
+       InvokeObjectPostAlterHook(RelationRelationId,
+                                 RelationGetRelid(rel),
+                                 0);
        return;
    }
 
-   /* Get a modifiable copy of the relation's pg_class row */
-   pg_class = table_open(RelationRelationId, RowExclusiveLock);
-
-   tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
-   if (!HeapTupleIsValid(tuple))
-       elog(ERROR, "cache lookup failed for relation %u", reloid);
-   rd_rel = (Form_pg_class) GETSTRUCT(tuple);
-
-   /* update the pg_class row */
-   rd_rel->reltablespace = (newTableSpace == MyDatabaseTableSpace) ? InvalidOid : newTableSpace;
-   CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
-
-   /* Record dependency on tablespace */
-   changeDependencyOnTablespace(RelationRelationId,
-                                reloid, rd_rel->reltablespace);
-
-   InvokeObjectPostAlterHook(RelationRelationId, reloid, 0);
+   /* Update can be done, so change reltablespace */
+   SetRelationTableSpace(rel, newTableSpace, InvalidOid);
 
-   heap_freetuple(tuple);
-
-   table_close(pg_class, RowExclusiveLock);
+   InvokeObjectPostAlterHook(RelationRelationId, RelationGetRelid(rel), 0);
 
    /* Make sure the reltablespace change is visible */
    CommandCounterIncrement();

diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index 08c463d3c431de282ab01926d6dc3421d0f3f3af..b3d30acc3596d236ee42b8695fc22072266743fa 100644 (file)
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -61,6 +61,10 @@ extern void ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_
 
 extern void SetRelationHasSubclass(Oid relationId, bool relhassubclass);
 
+extern bool CheckRelationTableSpaceMove(Relation rel, Oid newTableSpaceId);
+extern void SetRelationTableSpace(Relation rel, Oid newTableSpaceId,
+                                 Oid newRelFileNode);
+
 extern ObjectAddress renameatt(RenameStmt *stmt);
 
 extern ObjectAddress RenameConstraint(RenameStmt *stmt);