Rethink behavior of CREATE OR REPLACE during CREATE EXTENSION.

The original implementation simply did nothing when replacing an existing
object during CREATE EXTENSION.  The folly of this was exposed by a report
from Marc Munro: if the existing object belongs to another extension, we
are left in an inconsistent state.  We should insist that the object does
not belong to another extension, and then add it to the current extension
if not already a member.

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 7cf44d1f256e55e9e794daad613c5a203e71917a..439949362570d65d63ccc5e04b2b8c5565bb313b 100644 (file)
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -1256,7 +1256,7 @@ heap_create_with_catalog(const char *relname,
 
                recordDependencyOnOwner(RelationRelationId, relid, ownerid);
 
-               recordDependencyOnCurrentExtension(&myself);
+               recordDependencyOnCurrentExtension(&myself, false);
 
                if (reloftypeid)
                {

diff --git a/src/backend/catalog/pg_collation.c b/src/backend/catalog/pg_collation.c
index 28a016b97260339d266243020eeb2680a81b2ecd..ce9bfae70259dee5277f2e4977dac33785812e4e 100644 (file)
--- a/src/backend/catalog/pg_collation.c
+++ b/src/backend/catalog/pg_collation.c
@@ -132,7 +132,7 @@ CollationCreate(const char *collname, Oid collnamespace,
                                                        collowner);
 
        /* dependency on extension */
-       recordDependencyOnCurrentExtension(&myself);
+       recordDependencyOnCurrentExtension(&myself, false);
 
        /* Post creation hook for new collation */
        InvokeObjectAccessHook(OAT_POST_CREATE,

diff --git a/src/backend/catalog/pg_conversion.c b/src/backend/catalog/pg_conversion.c
index 1ef6a9d24e2dca4bb9e3599d967d1d5fa3807c28..c84dbc6ae4cc9e2df7afd8a48f08b7f786a6eeb5 100644 (file)
--- a/src/backend/catalog/pg_conversion.c
+++ b/src/backend/catalog/pg_conversion.c
@@ -133,7 +133,7 @@ ConversionCreate(const char *conname, Oid connamespace,
                                                        conowner);
 
        /* dependency on extension */
-       recordDependencyOnCurrentExtension(&myself);
+       recordDependencyOnCurrentExtension(&myself, false);
 
        /* Post creation hook for new conversion */
        InvokeObjectAccessHook(OAT_POST_CREATE,

diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c
index 67aad86d4e705aa398c0f7a1f709aebef4a7da43..c1a4340190a5574f78badcf94333026af380a2f5 100644 (file)
--- a/src/backend/catalog/pg_depend.c
+++ b/src/backend/catalog/pg_depend.c
@@ -130,14 +130,44 @@ recordMultipleDependencies(const ObjectAddress *depender,
  *
  * This must be called during creation of any user-definable object type
  * that could be a member of an extension.
+ *
+ * If isReplace is true, the object already existed (or might have already
+ * 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.
  */
 void
-recordDependencyOnCurrentExtension(const ObjectAddress *object)
+recordDependencyOnCurrentExtension(const ObjectAddress *object,
+                                                                  bool isReplace)
 {
+       /* Only whole objects can be extension members */
+       Assert(object->objectSubId == 0);
+
        if (creating_extension)
        {
                ObjectAddress extension;
 
+               /* Only need to check for existing membership if isReplace */
+               if (isReplace)
+               {
+                       Oid             oldext;
+
+                       oldext = getExtensionOfObject(object->classId, object->objectId);
+                       if (OidIsValid(oldext))
+                       {
+                               /* If already a member of this extension, nothing to do */
+                               if (oldext == CurrentExtensionObject)
+                                       return;
+                               /* Already a member of some other extension, so reject */
+                               ereport(ERROR,
+                                               (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+                                                errmsg("%s is already a member of extension \"%s\"",
+                                                               getObjectDescription(object),
+                                                               get_extension_name(oldext))));
+                       }
+               }
+
+               /* OK, record it as a member of CurrentExtensionObject */
                extension.classId = ExtensionRelationId;
                extension.objectId = CurrentExtensionObject;
                extension.objectSubId = 0;

diff --git a/src/backend/catalog/pg_namespace.c b/src/backend/catalog/pg_namespace.c
index 172f99196cc371c2a90ae9469134842581fe51d9..ceebac2f7e1d2e2d6b6e8faf991f98d656b86664 100644 (file)
--- a/src/backend/catalog/pg_namespace.c
+++ b/src/backend/catalog/pg_namespace.c
@@ -83,7 +83,7 @@ NamespaceCreate(const char *nspName, Oid ownerId)
        recordDependencyOnOwner(NamespaceRelationId, nspoid, ownerId);
 
        /* dependency on extension */
-       recordDependencyOnCurrentExtension(&myself);
+       recordDependencyOnCurrentExtension(&myself, false);
 
        /* Post creation hook for new schema */
        InvokeObjectAccessHook(OAT_POST_CREATE, NamespaceRelationId, nspoid, 0);

diff --git a/src/backend/catalog/pg_operator.c b/src/backend/catalog/pg_operator.c
index 4d2d7b7e265611d395b2d13e25ab3b2c2171bc27..772e5d438d170428d32c15dacd2a6fe102536271 100644 (file)
--- a/src/backend/catalog/pg_operator.c
+++ b/src/backend/catalog/pg_operator.c
@@ -855,5 +855,5 @@ makeOperatorDependencies(HeapTuple tuple)
                                                        oper->oprowner);
 
        /* Dependency on extension */
-       recordDependencyOnCurrentExtension(&myself);
+       recordDependencyOnCurrentExtension(&myself, true);
 }

diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index ec229810cac63db9fb579b1f31ad679828c4b180..f2f08721599d4ae4ffa85543acd6c7f1f57f2d6e 100644 (file)
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -564,8 +564,7 @@ ProcedureCreate(const char *procedureName,
         * Create dependencies for the new function.  If we are updating an
         * existing function, first delete any existing pg_depend entries.
         * (However, since we are not changing ownership or permissions, the
-        * shared dependencies do *not* need to change, and we leave them alone.
-        * We also don't change any pre-existing extension-membership dependency.)
+        * shared dependencies do *not* need to change, and we leave them alone.)
         */
        if (is_update)
                deleteDependencyRecordsFor(ProcedureRelationId, retval, true);
@@ -619,8 +618,7 @@ ProcedureCreate(const char *procedureName,
        }
 
        /* dependency on extension */
-       if (!is_update)
-               recordDependencyOnCurrentExtension(&myself);
+       recordDependencyOnCurrentExtension(&myself, is_update);
 
        heap_freetuple(tup);
 

diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
index f60822982399db6c179495f1ccff9711b64c3991..ea52f67dabbe87b91042b9a9590908b219701dde 100644 (file)
--- a/src/backend/catalog/pg_type.c
+++ b/src/backend/catalog/pg_type.c
@@ -480,7 +480,7 @@ TypeCreate(Oid newTypeOid,
  *
  * 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/rebuild extension dependencies, though.
+ * a shell type.
  */
 void
 GenerateTypeDependencies(Oid typeNamespace,
@@ -521,7 +521,7 @@ GenerateTypeDependencies(Oid typeNamespace,
         * For a relation rowtype (that's not a composite type), we should skip
         * these because we'll depend on them indirectly through the pg_class
         * entry.  Likewise, skip for implicit arrays since we'll depend on them
-        * through the element type.  The same goes for extension membership.
+        * through the element type.
         */
        if ((!OidIsValid(relationOid) || relationKind == RELKIND_COMPOSITE_TYPE) &&
                !isImplicitArray)
@@ -532,12 +532,11 @@ GenerateTypeDependencies(Oid typeNamespace,
                recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
 
                recordDependencyOnOwner(TypeRelationId, typeObjectId, owner);
-
-               /* dependency on extension */
-               if (!rebuild)
-                       recordDependencyOnCurrentExtension(&myself);
        }
 
+       /* dependency on extension */
+       recordDependencyOnCurrentExtension(&myself, rebuild);
+
        /* Normal dependencies on the I/O functions */
        if (OidIsValid(inputProcedure))
        {

diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c
index 643ba91bfe1c50f048cf6f3364fb5f1e6dee8895..d16932ba6a025049f78235b13d528295f69671c5 100644 (file)
--- a/src/backend/commands/foreigncmds.c
+++ b/src/backend/commands/foreigncmds.c
@@ -515,7 +515,7 @@ CreateForeignDataWrapper(CreateFdwStmt *stmt)
        recordDependencyOnOwner(ForeignDataWrapperRelationId, fdwId, ownerId);
 
        /* dependency on extension */
-       recordDependencyOnCurrentExtension(&myself);
+       recordDependencyOnCurrentExtension(&myself, false);
 
        /* Post creation hook for new foreign data wrapper */
        InvokeObjectAccessHook(OAT_POST_CREATE,
@@ -857,7 +857,7 @@ CreateForeignServer(CreateForeignServerStmt *stmt)
        recordDependencyOnOwner(ForeignServerRelationId, srvId, ownerId);
 
        /* dependency on extension */
-       recordDependencyOnCurrentExtension(&myself);
+       recordDependencyOnCurrentExtension(&myself, false);
 
        /* Post creation hook for new foreign server */
        InvokeObjectAccessHook(OAT_POST_CREATE, ForeignServerRelationId, srvId, 0);
@@ -1137,7 +1137,7 @@ CreateUserMapping(CreateUserMappingStmt *stmt)
        }
 
        /* dependency on extension */
-       recordDependencyOnCurrentExtension(&myself);
+       recordDependencyOnCurrentExtension(&myself, false);
 
        /* Post creation hook for new user mapping */
        InvokeObjectAccessHook(OAT_POST_CREATE, UserMappingRelationId, umId, 0);

diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 03da168ff2c14d1ef1e414f7481ba7e72cd377b5..92abd44a600e4c8e034f6c9a9081309f61ef32f4 100644 (file)
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -1489,6 +1489,7 @@ CreateCast(CreateCastStmt *stmt)
        char            sourcetyptype;
        char            targettyptype;
        Oid                     funcid;
+       Oid                     castid;
        int                     nargs;
        char            castcontext;
        char            castmethod;
@@ -1734,13 +1735,13 @@ CreateCast(CreateCastStmt *stmt)
 
        tuple = heap_form_tuple(RelationGetDescr(relation), values, nulls);
 
-       simple_heap_insert(relation, tuple);
+       castid = simple_heap_insert(relation, tuple);
 
        CatalogUpdateIndexes(relation, tuple);
 
        /* make dependency entries */
        myself.classId = CastRelationId;
-       myself.objectId = HeapTupleGetOid(tuple);
+       myself.objectId = castid;
        myself.objectSubId = 0;
 
        /* dependency on source type */
@@ -1765,11 +1766,10 @@ CreateCast(CreateCastStmt *stmt)
        }
 
        /* dependency on extension */
-       recordDependencyOnCurrentExtension(&myself);
+       recordDependencyOnCurrentExtension(&myself, false);
 
        /* Post creation hook for new cast */
-       InvokeObjectAccessHook(OAT_POST_CREATE,
-                                                  CastRelationId, myself.objectId, 0);
+       InvokeObjectAccessHook(OAT_POST_CREATE, CastRelationId, castid, 0);
 
        heap_freetuple(tuple);
 

diff --git a/src/backend/commands/opclasscmds.c b/src/backend/commands/opclasscmds.c
index aff5ac6ec43ba15e02ac7450a8e76af3b5f25751..2bb0d4c3b5fc74076323b154155965e99443b6c8 100644 (file)
--- a/src/backend/commands/opclasscmds.c
+++ b/src/backend/commands/opclasscmds.c
@@ -310,7 +310,7 @@ CreateOpFamily(char *amname, char *opfname, Oid namespaceoid, Oid amoid)
        recordDependencyOnOwner(OperatorFamilyRelationId, opfamilyoid, GetUserId());
 
        /* dependency on extension */
-       recordDependencyOnCurrentExtension(&myself);
+       recordDependencyOnCurrentExtension(&myself, false);
 
        /* Post creation hook for new operator family */
        InvokeObjectAccessHook(OAT_POST_CREATE,
@@ -713,7 +713,7 @@ DefineOpClass(CreateOpClassStmt *stmt)
        recordDependencyOnOwner(OperatorClassRelationId, opclassoid, GetUserId());
 
        /* dependency on extension */
-       recordDependencyOnCurrentExtension(&myself);
+       recordDependencyOnCurrentExtension(&myself, false);
 
        /* Post creation hook for new operator class */
        InvokeObjectAccessHook(OAT_POST_CREATE,

diff --git a/src/backend/commands/proclang.c b/src/backend/commands/proclang.c
index b36f31ee6d52f5619fbb5eadea4fd9ad73c33a64..98770c5a61afc3b6d474feea50ed0ef947156a24 100644 (file)
--- a/src/backend/commands/proclang.c
+++ b/src/backend/commands/proclang.c
@@ -388,8 +388,7 @@ create_proc_lang(const char *languageName, bool replace,
         * Create dependencies for the new language.  If we are updating an
         * existing language, first delete any existing pg_depend entries.
         * (However, since we are not changing ownership or permissions, the
-        * shared dependencies do *not* need to change, and we leave them alone.
-        * We also don't change any pre-existing extension-membership dependency.)
+        * shared dependencies do *not* need to change, and we leave them alone.)
         */
        myself.classId = LanguageRelationId;
        myself.objectId = HeapTupleGetOid(tup);
@@ -404,8 +403,7 @@ create_proc_lang(const char *languageName, bool replace,
                                                                languageOwner);
 
        /* dependency on extension */
-       if (!is_update)
-               recordDependencyOnCurrentExtension(&myself);
+       recordDependencyOnCurrentExtension(&myself, is_update);
 
        /* dependency on the PL handler function */
        referenced.classId = ProcedureRelationId;

diff --git a/src/backend/commands/tsearchcmds.c b/src/backend/commands/tsearchcmds.c
index 3355eaafda26a6186b3163204f528c5948e678a4..deac1062ef9718655e1dd7b95d769fe456ac5f0c 100644 (file)
--- a/src/backend/commands/tsearchcmds.c
+++ b/src/backend/commands/tsearchcmds.c
@@ -142,7 +142,7 @@ makeParserDependencies(HeapTuple tuple)
        recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
 
        /* dependency on extension */
-       recordDependencyOnCurrentExtension(&myself);
+       recordDependencyOnCurrentExtension(&myself, false);
 
        /* dependencies on functions */
        referenced.classId = ProcedureRelationId;
@@ -479,7 +479,7 @@ makeDictionaryDependencies(HeapTuple tuple)
        recordDependencyOnOwner(myself.classId, myself.objectId, dict->dictowner);
 
        /* dependency on extension */
-       recordDependencyOnCurrentExtension(&myself);
+       recordDependencyOnCurrentExtension(&myself, false);
 
        /* dependency on template */
        referenced.classId = TSTemplateRelationId;
@@ -1069,7 +1069,7 @@ makeTSTemplateDependencies(HeapTuple tuple)
        recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
 
        /* dependency on extension */
-       recordDependencyOnCurrentExtension(&myself);
+       recordDependencyOnCurrentExtension(&myself, false);
 
        /* dependencies on functions */
        referenced.classId = ProcedureRelationId;
@@ -1417,8 +1417,7 @@ makeConfigurationDependencies(HeapTuple tuple, bool removeOld,
        recordDependencyOnOwner(myself.classId, myself.objectId, cfg->cfgowner);
 
        /* dependency on extension */
-       if (!removeOld)
-               recordDependencyOnCurrentExtension(&myself);
+       recordDependencyOnCurrentExtension(&myself, removeOld);
 
        /* dependency on parser */
        referenced.classId = TSParserRelationId;

diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index a3bd729156f796ec3cf544e6c4b728e448ba4e55..5dfb25fee74424c76dd61a2ae9437433806d5e1a 100644 (file)
--- a/src/include/catalog/dependency.h
+++ b/src/include/catalog/dependency.h
@@ -201,7 +201,8 @@ extern void recordMultipleDependencies(const ObjectAddress *depender,
                                                   int nreferenced,
                                                   DependencyType behavior);
 
-extern void recordDependencyOnCurrentExtension(const ObjectAddress *object);
+extern void recordDependencyOnCurrentExtension(const ObjectAddress *object,
+                                                                  bool isReplace);
 
 extern long deleteDependencyRecordsFor(Oid classId, Oid objectId,
                                                   bool skipExtensionDeps);