Prevent ALTER TYPE/DOMAIN/OPERATOR from changing extension membership.
authorTom Lane
Tue, 17 Aug 2021 18:29:22 +0000 (14:29 -0400)
committerTom Lane
Tue, 17 Aug 2021 18:29:22 +0000 (14:29 -0400)
If recordDependencyOnCurrentExtension is invoked on a pre-existing,
free-standing object during an extension update script, that object
will become owned by the extension.  In our current code this is
possible in three cases:

* Replacing a "shell" type or operator.
* CREATE OR REPLACE overwriting an existing object.
* ALTER TYPE SET, ALTER DOMAIN SET, and ALTER OPERATOR SET.

The first of these cases is intentional behavior, as noted by the
existing comments for GenerateTypeDependencies.  It seems like
appropriate behavior for CREATE OR REPLACE too; at least, the obvious
alternatives are not better.  However, the fact that it happens during
ALTER is an artifact of trying to share code (GenerateTypeDependencies
and makeOperatorDependencies) between the CREATE and ALTER cases.
Since an extension script would be unlikely to ALTER an object that
didn't already belong to the extension, this behavior is not very
troubling for the direct target object ... but ALTER TYPE SET will
recurse to dependent domains, and it is very uncool for those to
become owned by the extension if they were not already.

Let's fix this by redefining the ALTER cases to never change extension
membership, full stop.  We could minimize the behavioral change by
only changing the behavior when ALTER TYPE SET is recursing to a
domain, but that would complicate the code and it does not seem like
a better definition.

Per bug #17144 from Alex Kozhemyakin.  Back-patch to v13 where ALTER
TYPE SET was added.  (The other cases are older, but since they only
affect the directly-named object, there's not enough of a problem to
justify changing the behavior further back.)

Discussion: https://postgr.es/m/17144-e67d7a8f049de9af@postgresql.org

src/backend/catalog/pg_depend.c
src/backend/catalog/pg_operator.c
src/backend/catalog/pg_type.c
src/backend/commands/operatorcmds.c
src/backend/commands/typecmds.c
src/include/catalog/pg_operator.h
src/include/catalog/pg_type.h

index 21cfdcace9421f9e760cb4e1ae342b1e6ff73d98..8d9f1c8e8aba49c67b9801d288d430bb105c8e93 100644 (file)
@@ -133,6 +133,13 @@ recordMultipleDependencies(const ObjectAddress *depender,
  * existed), so we must check for a pre-existing extension membership entry.
  * Passing false is a guarantee that the object is newly created, and so
  * could not already be a member of any extension.
+ *
+ * Note: isReplace = true is typically used when updating a object in
+ * CREATE OR REPLACE and similar commands.  The net effect is that if an
+ * extension script uses such a command on a pre-existing free-standing
+ * object, the object will be absorbed into the extension.  If the object
+ * is already a member of some other extension, the command will fail.
+ * This behavior is desirable for cases such as replacing a shell type.
  */
 void
 recordDependencyOnCurrentExtension(const ObjectAddress *object,
index 340e284ae4b52df135ab6be80713220d810cd038..005c0d19f13e98aec5398a243564d2c59e6974f0 100644 (file)
@@ -268,7 +268,7 @@ OperatorShellMake(const char *operatorName,
    CatalogTupleInsert(pg_operator_desc, tup);
 
    /* Add dependencies for the entry */
-   makeOperatorDependencies(tup, false);
+   makeOperatorDependencies(tup, true, false);
 
    heap_freetuple(tup);
 
@@ -546,7 +546,7 @@ OperatorCreate(const char *operatorName,
    }
 
    /* Add dependencies for the entry */
-   address = makeOperatorDependencies(tup, isUpdate);
+   address = makeOperatorDependencies(tup, true, isUpdate);
 
    /* Post creation hook for new operator */
    InvokeObjectPostCreateHook(OperatorRelationId, operatorObjectId, 0);
@@ -766,11 +766,17 @@ OperatorUpd(Oid baseId, Oid commId, Oid negId, bool isDelete)
  * complete operator, a new shell operator, a just-updated shell,
  * or an operator that's being modified by ALTER OPERATOR).
  *
+ * makeExtensionDep should be true when making a new operator or
+ * replacing a shell, false for ALTER OPERATOR.  Passing false
+ * will prevent any change in the operator's extension membership.
+ *
  * NB: the OidIsValid tests in this routine are necessary, in case
  * the given operator is a shell.
  */
 ObjectAddress
-makeOperatorDependencies(HeapTuple tuple, bool isUpdate)
+makeOperatorDependencies(HeapTuple tuple,
+                        bool makeExtensionDep,
+                        bool isUpdate)
 {
    Form_pg_operator oper = (Form_pg_operator) GETSTRUCT(tuple);
    ObjectAddress myself,
@@ -867,7 +873,8 @@ makeOperatorDependencies(HeapTuple tuple, bool isUpdate)
                            oper->oprowner);
 
    /* Dependency on extension */
-   recordDependencyOnCurrentExtension(&myself, true);
+   if (makeExtensionDep)
+       recordDependencyOnCurrentExtension(&myself, true);
 
    return myself;
 }
index cd5671496890c8d147d8ced61e1ee74e2e2d5458..932e110938e8a3b5649d7c242b0dc589b028b345 100644 (file)
@@ -162,6 +162,7 @@ TypeShellMake(const char *typeName, Oid typeNamespace, Oid ownerId)
                                 0,
                                 false,
                                 false,
+                                true,  /* make extension dependency */
                                 false);
 
    /* Post creation hook for new shell type */
@@ -497,6 +498,7 @@ TypeCreate(Oid newTypeOid,
                                 relationKind,
                                 isImplicitArray,
                                 isDependentType,
+                                true,  /* make extension dependency */
                                 rebuildDeps);
 
    /* Post creation hook for new type */
@@ -530,13 +532,17 @@ TypeCreate(Oid newTypeOid,
  * isDependentType is true if this is an implicit array or relation rowtype;
  * that means it doesn't need its own dependencies on owner etc.
  *
- * If rebuild is true, we remove existing dependencies and rebuild them
- * from scratch.  This is needed for ALTER TYPE, and also when replacing
- * a shell type.  We don't remove an existing extension dependency, though.
- * (That means an extension can't absorb a shell type created in another
- * extension, nor ALTER a type created by another extension.  Also, if it
- * replaces a free-standing shell type or ALTERs a free-standing type,
- * that type will become a member of the extension.)
+ * We make an extension-membership dependency if we're in an extension
+ * script and makeExtensionDep is true (and isDependentType isn't true).
+ * makeExtensionDep should be true when creating a new type or replacing a
+ * shell type, but not for ALTER TYPE on an existing type.  Passing false
+ * causes the type's extension membership to be left alone.
+ *
+ * rebuild should be true if this is a pre-existing type.  We will remove
+ * existing dependencies and rebuild them from scratch.  This is needed for
+ * ALTER TYPE, and also when replacing a shell type.  We don't remove any
+ * existing extension dependency, though (hence, if makeExtensionDep is also
+ * true and the type belongs to some other extension, an error will occur).
  */
 void
 GenerateTypeDependencies(HeapTuple typeTuple,
@@ -546,6 +552,7 @@ GenerateTypeDependencies(HeapTuple typeTuple,
                         char relationKind, /* only for relation rowtypes */
                         bool isImplicitArray,
                         bool isDependentType,
+                        bool makeExtensionDep,
                         bool rebuild)
 {
    Form_pg_type typeForm = (Form_pg_type) GETSTRUCT(typeTuple);
@@ -602,7 +609,8 @@ GenerateTypeDependencies(HeapTuple typeTuple,
        recordDependencyOnNewAcl(TypeRelationId, typeObjectId, 0,
                                 typeForm->typowner, typacl);
 
-       recordDependencyOnCurrentExtension(&myself, rebuild);
+       if (makeExtensionDep)
+           recordDependencyOnCurrentExtension(&myself, rebuild);
    }
 
    /* Normal dependencies on the I/O functions */
index bf23937849c9fbd37bb5289dccfc6b1b84fbeff6..640b22ad93693e3637992c09e7bb67f2b052951e 100644 (file)
@@ -530,7 +530,7 @@ AlterOperator(AlterOperatorStmt *stmt)
 
    CatalogTupleUpdate(catalog, &tup->t_self, tup);
 
-   address = makeOperatorDependencies(tup, true);
+   address = makeOperatorDependencies(tup, false, true);
 
    InvokeObjectPostAlterHook(OperatorRelationId, oprId, 0);
 
index 05158246fba58fa05cf62c6900fc465156b64157..11a46ab7bb1b6e8e4d0acde34ebffd375a2f6478 100644 (file)
@@ -2253,6 +2253,7 @@ AlterDomainDefault(List *names, Node *defaultRaw)
                             0, /* relation kind is n/a */
                             false, /* a domain isn't an implicit array */
                             false, /* nor is it any kind of dependent type */
+                            false, /* don't touch extension membership */
                             true); /* We do need to rebuild dependencies */
 
    InvokeObjectPostAlterHook(TypeRelationId, domainoid, 0);
@@ -3984,6 +3985,7 @@ AlterTypeRecurse(Oid typeOid, bool isImplicitArray,
                             0, /* we rejected composite types above */
                             isImplicitArray,   /* it might be an array */
                             isImplicitArray,   /* dependent iff it's array */
+                            false, /* don't touch extension membership */
                             true);
 
    InvokeObjectPostAlterHook(TypeRelationId, typeOid, 0);
index 1daa2638520f685bad06ca474fb72f07ff58a060..5aaa02e8a9f56b246e82dfddc5770c33b31e1873 100644 (file)
@@ -95,7 +95,9 @@ extern ObjectAddress OperatorCreate(const char *operatorName,
                                    bool canMerge,
                                    bool canHash);
 
-extern ObjectAddress makeOperatorDependencies(HeapTuple tuple, bool isUpdate);
+extern ObjectAddress makeOperatorDependencies(HeapTuple tuple,
+                                             bool makeExtensionDep,
+                                             bool isUpdate);
 
 extern void OperatorUpd(Oid baseId, Oid commId, Oid negId, bool isDelete);
 
index 7b375626484e85dbf4c77865838e9c067973b987..ebde49dafb4b10a1ec373df8b35df0795d9ccee3 100644 (file)
@@ -359,6 +359,7 @@ extern void GenerateTypeDependencies(HeapTuple typeTuple,
                                                         * rowtypes */
                                     bool isImplicitArray,
                                     bool isDependentType,
+                                    bool makeExtensionDep,
                                     bool rebuild);
 
 extern void RenameTypeInternal(Oid typeOid, const char *newTypeName,