Prevent drop of tablespaces used by partitioned relations

When a tablespace is used in a partitioned relation (per commits
ca4103025dfe in pg12 for tables and 33e6c34c3267 in pg11 for indexes),
it is possible to drop the tablespace, potentially causing various
problems.  One such was reported in bug #16577, where a rewriting ALTER
TABLE causes a server crash.

Protect against this by using pg_shdepend to keep track of tablespaces
when used for relations that don't keep physical files; we now abort a
tablespace if we see that the tablespace is referenced from any
partitioned relations.

Backpatch this to 11, where this problem has been latent all along.  We
don't try to create pg_shdepend entries for existing partitioned
indexes/tables, but any ones that are modified going forward will be
protected.

Note slight behavior change: when trying to drop a tablespace that
contains both regular tables as well as partitioned ones, you'd
previously get ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE and now you'll
get ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST.  Arguably, the latter is more
correct.

It is possible to add protecting pg_shdepend entries for existing
tables/indexes, by doing
  ALTER TABLE ONLY some_partitioned_table SET TABLESPACE pg_default;
  ALTER TABLE ONLY some_partitioned_table SET TABLESPACE original_tablespace;
for each partitioned table/index that is not in the database default
tablespace.  Because these partitioned objects do not have storage, no
file needs to be actually moved, so it shouldn't take more time than
what's required to acquire locks.

This query can be used to search for such relations:
SELECT ... FROM pg_class WHERE relkind IN ('p', 'I') AND reltablespace <> 0

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/16577-881633a9f9894fd5@postgresql.org
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Michael Paquier <michael@paquier.xyz>

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 74c67a89b377727b9ee8212e2a64a8e1a5176509..1429b2d05c8a9e60b480f25a6ea892c7fc0ef7b3 100644 (file)
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -6175,10 +6175,21 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
       </para>
      </listitem>
     </varlistentry>
+
+    <varlistentry>
+     <term><symbol>SHARED_DEPENDENCY_TABLESPACE</symbol> (<literal>t</literal>)</term>
+     <listitem>
+      <para>
+       The referenced object (which must be a tablespace) is mentioned as
+       the tablespace for a relation that doesn't have storage.
+      </para>
+     </listitem>
+    </varlistentry>
    </variablelist>
 
    Other dependency flavors might be needed in future.  Note in particular
-   that the current definition only supports roles as referenced objects.
+   that the current definition only supports roles and tablespaces as referenced
+   objects.
   </para>
 
  </sect1>

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 4ac05adc73da82d53b2fec4111f28fb4d461659c..46a383294e9edba03dc51466db6180ba4b5093aa 100644 (file)
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -442,6 +442,15 @@ heap_create(const char *relname,
        }
    }
 
+   /*
+    * If a tablespace is specified, removal of that tablespace is normally
+    * protected by the existence of a physical file; but for relations with
+    * no files, add a pg_shdepend entry to account for that.
+    */
+   if (!create_storage && reltablespace != InvalidOid)
+       recordDependencyOnTablespace(RelationRelationId, relid,
+                                    reltablespace);
+
    return rel;
 }
 

diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.c
index e6fef9782dff04968d40241170b6451bb67632f8..076ba2d1afcdbcb5993f7418a02c5440d8bbdc2e 100644 (file)
--- a/src/backend/catalog/pg_shdepend.c
+++ b/src/backend/catalog/pg_shdepend.c
@@ -59,6 +59,7 @@
 #include "commands/schemacmds.h"
 #include "commands/subscriptioncmds.h"
 #include "commands/tablecmds.h"
+#include "commands/tablespace.h"
 #include "commands/typecmds.h"
 #include "storage/lmgr.h"
 #include "miscadmin.h"
@@ -187,11 +188,14 @@ recordDependencyOnOwner(Oid classId, Oid objectId, Oid owner)
  *
  * There must be no more than one existing entry for the given dependent
  * object and dependency type! So in practice this can only be used for
- * updating SHARED_DEPENDENCY_OWNER entries, which should have that property.
+ * updating SHARED_DEPENDENCY_OWNER and SHARED_DEPENDENCY_TABLESPACE
+ * entries, which should have that property.
  *
  * If there is no previous entry, we assume it was referencing a PINned
  * object, so we create a new entry.  If the new referenced object is
  * PINned, we don't create an entry (and drop the old one, if any).
+ * (For tablespaces, we don't record dependencies in certain cases, so
+ * there are other possible reasons for entries to be missing.)
  *
  * sdepRel must be the pg_shdepend relation, already opened and suitably
  * locked.
@@ -345,6 +349,58 @@ changeDependencyOnOwner(Oid classId, Oid objectId, Oid newOwnerId)
    table_close(sdepRel, RowExclusiveLock);
 }
 
+/*
+ * recordDependencyOnTablespace
+ *
+ * A convenient wrapper of recordSharedDependencyOn -- register the specified
+ * tablespace as default for the given object.
+ *
+ * Note: it's the caller's responsibility to ensure that there isn't a
+ * tablespace entry for the object already.
+ */
+void
+recordDependencyOnTablespace(Oid classId, Oid objectId, Oid tablespace)
+{
+   ObjectAddress myself,
+                 referenced;
+
+   ObjectAddressSet(myself, classId, objectId);
+   ObjectAddressSet(referenced, TableSpaceRelationId, tablespace);
+
+   recordSharedDependencyOn(&myself, &referenced,
+                            SHARED_DEPENDENCY_TABLESPACE);
+}
+
+/*
+ * changeDependencyOnTablespace
+ *
+ * Update the shared dependencies to account for the new tablespace.
+ *
+ * Note: we don't need an objsubid argument because only whole objects
+ * have tablespaces.
+ */
+void
+changeDependencyOnTablespace(Oid classId, Oid objectId, Oid newTablespaceId)
+{
+   Relation    sdepRel;
+
+   sdepRel = table_open(SharedDependRelationId, RowExclusiveLock);
+
+   if (newTablespaceId != DEFAULTTABLESPACE_OID &&
+       newTablespaceId != InvalidOid)
+       shdepChangeDep(sdepRel,
+                      classId, objectId, 0,
+                      TableSpaceRelationId, newTablespaceId,
+                      SHARED_DEPENDENCY_TABLESPACE);
+   else
+       shdepDropDependency(sdepRel,
+                           classId, objectId, 0, true,
+                           InvalidOid, InvalidOid,
+                           SHARED_DEPENDENCY_INVALID);
+
+   table_close(sdepRel, RowExclusiveLock);
+}
+
 /*
  * getOidListDiff
  *     Helper for updateAclDependencies.
@@ -1084,13 +1140,6 @@ shdepLockAndCheckObject(Oid classId, Oid objectId)
                                objectId)));
            break;
 
-           /*
-            * Currently, this routine need not support any other shared
-            * object types besides roles.  If we wanted to record explicit
-            * dependencies on databases or tablespaces, we'd need code along
-            * these lines:
-            */
-#ifdef NOT_USED
        case TableSpaceRelationId:
            {
                /* For lack of a syscache on pg_tablespace, do this: */
@@ -1104,7 +1153,6 @@ shdepLockAndCheckObject(Oid classId, Oid objectId)
                pfree(tablespace);
                break;
            }
-#endif
 
        case DatabaseRelationId:
            {
@@ -1164,6 +1212,8 @@ storeObjectDescription(StringInfo descs,
                appendStringInfo(descs, _("privileges for %s"), objdesc);
            else if (deptype == SHARED_DEPENDENCY_POLICY)
                appendStringInfo(descs, _("target of %s"), objdesc);
+           else if (deptype == SHARED_DEPENDENCY_TABLESPACE)
+               appendStringInfo(descs, _("tablespace for %s"), objdesc);
            else
                elog(ERROR, "unrecognized dependency type: %d",
                     (int) deptype);

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 933592b0fb30581fd4e03f75058f6524db4a8f04..6f4a3e70a409492a9b6520b42279753ecb2e5238 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -12839,6 +12839,10 @@ ATExecSetTableSpaceNoStorage(Relation rel, Oid newTableSpace)
    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);
 
    heap_freetuple(tuple);

diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 457948a296d1875a7aee0f39cee41a6dab9a219b..93d948f2ab4c167248631dd49a2b07b7f9475810 100644 (file)
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -421,6 +421,8 @@ DropTableSpace(DropTableSpaceStmt *stmt)
    Form_pg_tablespace spcform;
    ScanKeyData entry[1];
    Oid         tablespaceoid;
+   char       *detail;
+   char       *detail_log;
 
    /*
     * Find the target tuple
@@ -469,6 +471,16 @@ DropTableSpace(DropTableSpaceStmt *stmt)
        aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_TABLESPACE,
                       tablespacename);
 
+   /* Check for pg_shdepend entries depending on this tablespace */
+   if (checkSharedDependencies(TableSpaceRelationId, tablespaceoid,
+                               &detail, &detail_log))
+       ereport(ERROR,
+               (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
+                errmsg("tablespace \"%s\" cannot be dropped because some objects depend on it",
+                       tablespacename),
+                errdetail_internal("%s", detail),
+                errdetail_log("%s", detail_log)));
+
    /* DROP hook for the tablespace being removed */
    InvokeObjectDropHook(TableSpaceRelationId, tablespaceoid, 0);
 

diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index 7b1796ed09437a45dc3bb33f3fbf086766ac115d..8b1e3aacf23748a11397253ee696d95ee9f936af 100644 (file)
--- a/src/include/catalog/dependency.h
+++ b/src/include/catalog/dependency.h
@@ -67,6 +67,12 @@ typedef enum DependencyType
  * a role mentioned in a policy object.  The referenced object must be a
  * pg_authid entry.
  *
+ * (e) a SHARED_DEPENDENCY_TABLESPACE entry means that the referenced
+ * object is a tablespace mentioned in a relation without storage.  The
+ * referenced object must be a pg_tablespace entry.  (Relations that have
+ * storage don't need this: they are protected by the existence of a physical
+ * file in the tablespace.)
+ *
  * SHARED_DEPENDENCY_INVALID is a value used as a parameter in internal
  * routines, and is not valid in the catalog itself.
  */
@@ -76,6 +82,7 @@ typedef enum SharedDependencyType
    SHARED_DEPENDENCY_OWNER = 'o',
    SHARED_DEPENDENCY_ACL = 'a',
    SHARED_DEPENDENCY_POLICY = 'r',
+   SHARED_DEPENDENCY_TABLESPACE = 't',
    SHARED_DEPENDENCY_INVALID = 0
 } SharedDependencyType;
 
@@ -237,6 +244,12 @@ extern void recordDependencyOnOwner(Oid classId, Oid objectId, Oid owner);
 extern void changeDependencyOnOwner(Oid classId, Oid objectId,
                                    Oid newOwnerId);
 
+extern void recordDependencyOnTablespace(Oid classId, Oid objectId,
+                                        Oid tablespace);
+
+extern void changeDependencyOnTablespace(Oid classId, Oid objectId,
+                                        Oid newTablespaceId);
+
 extern void updateAclDependencies(Oid classId, Oid objectId, int32 objectSubId,
                                  Oid ownerId,
                                  int noldmembers, Oid *oldmembers,

diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source
index 8f012fcc339b3c1fda59d6fd47bfbf5a6095dea8..db6f04d7b850027ed21a3e2bc6d77921472af935 100644 (file)
--- a/src/test/regress/input/tablespace.source
+++ b/src/test/regress/input/tablespace.source
@@ -240,6 +240,9 @@ CREATE TABLESPACE regress_badspace LOCATION '/no/such/location';
 -- No such tablespace
 CREATE TABLE bar (i int) TABLESPACE regress_nosuchspace;
 
+-- Fail, in use for some partitioned object
+DROP TABLESPACE regress_tblspace;
+ALTER INDEX testschema.part_a_idx SET TABLESPACE pg_default;
 -- Fail, not empty
 DROP TABLESPACE regress_tblspace;
 

diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source
index 2ea68cabb0a3bc1a13d1cba64f32c2d5030bea5b..c2bc2fda10ebefe2af5f40c419d54b6f955d38b7 100644 (file)
--- a/src/test/regress/output/tablespace.source
+++ b/src/test/regress/output/tablespace.source
@@ -623,6 +623,11 @@ ERROR:  directory "/no/such/location" does not exist
 -- No such tablespace
 CREATE TABLE bar (i int) TABLESPACE regress_nosuchspace;
 ERROR:  tablespace "regress_nosuchspace" does not exist
+-- Fail, in use for some partitioned object
+DROP TABLESPACE regress_tblspace;
+ERROR:  tablespace "regress_tblspace" cannot be dropped because some objects depend on it
+DETAIL:  tablespace for index testschema.part_a_idx
+ALTER INDEX testschema.part_a_idx SET TABLESPACE pg_default;
 -- Fail, not empty
 DROP TABLESPACE regress_tblspace;
 ERROR:  tablespace "regress_tblspace" is not empty