Fix some issues with temp/transient tables in extension scripts.

Phil Sorber reported that a rewriting ALTER TABLE within an extension
update script failed, because it creates and then drops a placeholder
table; the drop was being disallowed because the table was marked as an
extension member.  We could hack that specific case but it seems likely
that there might be related cases now or in the future, so the most
practical solution seems to be to create an exception to the general rule
that extension member objects can only be dropped by dropping the owning
extension.  To wit: if the DROP is issued within the extension's own
creation or update scripts, we'll allow it, implicitly performing an
"ALTER EXTENSION DROP object" first.  This will simplify cases such as
extension downgrade scripts anyway.

No docs change since we don't seem to have documented the idea that you
would need ALTER EXTENSION DROP for such an action to begin with.

Also, arrange for explicitly temporary tables to not get linked as
extension members in the first place, and the same for the magic
pg_temp_nnn schemas that are created to hold them.  This prevents assorted
unpleasant results if an extension script creates a temp table: the forced
drop at session end would either fail or remove the entire extension, and
neither of those outcomes is desirable.  Note that this doesn't fix the
ALTER TABLE scenario, since the placeholder table is not temp (unless the
table being rewritten is).

Back-patch to 9.1.

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 0c91536c6c366833dfa95c30cdac11b91724cd2d..3187c9ca5e7ed5f6415d76f84118865bbc883a8a 100644 (file)
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -549,17 +549,21 @@ findDependentObjects(const ObjectAddress *object,
                                 * another object, or is part of the extension that is the
                                 * other object.  We have three cases:
                                 *
-                                * 1. At the outermost recursion level, disallow the DROP. (We
-                                * just ereport here, rather than proceeding, since no other
-                                * dependencies are likely to be interesting.)  However, if
-                                * the owning object is listed in pendingObjects, just release
-                                * the caller's lock and return; we'll eventually complete the
-                                * DROP when we reach that entry in the pending list.
+                                * 1. At the outermost recursion level, we normally disallow
+                                * the DROP.  (We just ereport here, rather than proceeding,
+                                * since no other dependencies are likely to be interesting.)
+                                * However, there are exceptions.
                                 */
                                if (stack == NULL)
                                {
                                        char       *otherObjDesc;
 
+                                       /*
+                                        * Exception 1a: if the owning object is listed in
+                                        * pendingObjects, just release the caller's lock and
+                                        * return.  We'll eventually complete the DROP when we
+                                        * reach that entry in the pending list.
+                                        */
                                        if (pendingObjects &&
                                                object_address_present(&otherObject, pendingObjects))
                                        {
@@ -568,6 +572,21 @@ findDependentObjects(const ObjectAddress *object,
                                                ReleaseDeletionLock(object);
                                                return;
                                        }
+
+                                       /*
+                                        * Exception 1b: if the owning object is the extension
+                                        * currently being created/altered, it's okay to continue
+                                        * with the deletion.  This allows dropping of an
+                                        * extension's objects within the extension's scripts,
+                                        * as well as corner cases such as dropping a transient
+                                        * object created within such a script.
+                                        */
+                                       if (creating_extension &&
+                                               otherObject.classId == ExtensionRelationId &&
+                                               otherObject.objectId == CurrentExtensionObject)
+                                               break;
+
+                                       /* No exception applies, so throw the error */
                                        otherObjDesc = getObjectDescription(&otherObject);
                                        ereport(ERROR,
                                                        (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 2b77b84be0199bb7303eb160644f4324f41dc5a5..384e90de7c5ace7d89f7b6791f6e9da86559ce0e 100644 (file)
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -945,10 +945,12 @@ AddNewRelationType(const char *typeName,
  *     reltablespace: OID of tablespace it goes in
  *     relid: OID to assign to new rel, or InvalidOid to select a new OID
  *     reltypeid: OID to assign to rel's rowtype, or InvalidOid to select one
+ *     reloftypeid: if a typed table, OID of underlying type; else InvalidOid
  *     ownerid: OID of new rel's owner
  *     tupdesc: tuple descriptor (source of column definitions)
  *     cooked_constraints: list of precooked check constraints and defaults
  *     relkind: relkind for new rel
+ *     relpersistence: rel's persistence status (permanent, temp, or unlogged)
  *     shared_relation: TRUE if it's to be a shared relation
  *     mapped_relation: TRUE if the relation will use the relfilenode map
  *     oidislocal: TRUE if oid column (if any) should be marked attislocal
@@ -1222,6 +1224,10 @@ heap_create_with_catalog(const char *relname,
         * should they have any ACL entries.  The same applies for extension
         * dependencies.
         *
+        * If it's a temp table, we do not make it an extension member; this
+        * prevents the unintuitive result that deletion of the temp table at
+        * session end would make the whole extension go away.
+        *
         * Also, skip this in bootstrap mode, since we don't make dependencies
         * while bootstrapping.
         */
@@ -1242,7 +1248,8 @@ heap_create_with_catalog(const char *relname,
 
                recordDependencyOnOwner(RelationRelationId, relid, ownerid);
 
-               recordDependencyOnCurrentExtension(&myself, false);
+               if (relpersistence != RELPERSISTENCE_TEMP)
+                       recordDependencyOnCurrentExtension(&myself, false);
 
                if (reloftypeid)
                {

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 079a19379b643368125044bc63f7dfa4a7e9d628..b57c25595fc7ac0f0afb5d2e4a2da20b708db811 100644 (file)
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -3325,7 +3325,8 @@ InitTempTableNamespace(void)
                 * temp tables.  This works because the places that access the temp
                 * namespace for my own backend skip permissions checks on it.
                 */
-               namespaceId = NamespaceCreate(namespaceName, BOOTSTRAP_SUPERUSERID);
+               namespaceId = NamespaceCreate(namespaceName, BOOTSTRAP_SUPERUSERID,
+                                                                         true);
                /* Advance command counter to make namespace visible */
                CommandCounterIncrement();
        }
@@ -3349,7 +3350,8 @@ InitTempTableNamespace(void)
        toastspaceId = get_namespace_oid(namespaceName, true);
        if (!OidIsValid(toastspaceId))
        {
-               toastspaceId = NamespaceCreate(namespaceName, BOOTSTRAP_SUPERUSERID);
+               toastspaceId = NamespaceCreate(namespaceName, BOOTSTRAP_SUPERUSERID,
+                                                                          true);
                /* Advance command counter to make namespace visible */
                CommandCounterIncrement();
        }

diff --git a/src/backend/catalog/pg_namespace.c b/src/backend/catalog/pg_namespace.c
index ceebac2f7e1d2e2d6b6e8faf991f98d656b86664..45762c25b9cd6e806b3f92dbd78853547f0503ea 100644 (file)
--- a/src/backend/catalog/pg_namespace.c
+++ b/src/backend/catalog/pg_namespace.c
@@ -26,10 +26,18 @@
 
 /* ----------------
  * NamespaceCreate
+ *
+ * Create a namespace (schema) with the given name and owner OID.
+ *
+ * If isTemp is true, this schema is a per-backend schema for holding
+ * temporary tables.  Currently, the only effect of that is to prevent it
+ * from being linked as a member of any active extension.  (If someone
+ * does CREATE TEMP TABLE in an extension script, we don't want the temp
+ * schema to become part of the extension.)
  * ---------------
  */
 Oid
-NamespaceCreate(const char *nspName, Oid ownerId)
+NamespaceCreate(const char *nspName, Oid ownerId, bool isTemp)
 {
        Relation        nspdesc;
        HeapTuple       tup;
@@ -82,8 +90,9 @@ NamespaceCreate(const char *nspName, Oid ownerId)
        /* dependency on owner */
        recordDependencyOnOwner(NamespaceRelationId, nspoid, ownerId);
 
-       /* dependency on extension */
-       recordDependencyOnCurrentExtension(&myself, false);
+       /* dependency on extension ... but not for magic temp schemas */
+       if (!isTemp)
+               recordDependencyOnCurrentExtension(&myself, false);
 
        /* Post creation hook for new schema */
        InvokeObjectAccessHook(OAT_POST_CREATE, NamespaceRelationId, nspoid, 0);

diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c
index 82bbf8fa506f5b4b95a8ed0c5ed5aefcd77b4fed..671b17dba1f9655225950d36b10cb139d78aa13c 100644 (file)
--- a/src/backend/commands/schemacmds.c
+++ b/src/backend/commands/schemacmds.c
@@ -95,7 +95,7 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString)
                                                        save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
 
        /* Create the schema's namespace */
-       namespaceId = NamespaceCreate(schemaName, owner_uid);
+       namespaceId = NamespaceCreate(schemaName, owner_uid, false);
 
        /* Advance cmd counter to make the namespace visible */
        CommandCounterIncrement();

diff --git a/src/include/catalog/pg_namespace.h b/src/include/catalog/pg_namespace.h
index 680802d7fea7ed7c5d99324dbf2f865c1952dd9b..a21b9750738b13afcb68a899c0894e1feb23cd33 100644 (file)
--- a/src/include/catalog/pg_namespace.h
+++ b/src/include/catalog/pg_namespace.h
@@ -77,6 +77,6 @@ DESCR("standard public schema");
 /*
  * prototypes for functions in pg_namespace.c
  */
-extern Oid     NamespaceCreate(const char *nspName, Oid ownerId);
+extern Oid     NamespaceCreate(const char *nspName, Oid ownerId, bool isTemp);
 
 #endif   /* PG_NAMESPACE_H */