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 3d2b183a4ee3bf1f5ba4b5686d22f3d76944c8cc..2b77b84be0199bb7303eb160644f4324f41dc5a5 100644 (file)
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -1242,7 +1242,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 5b92a4c0c2816d29d6e3379e6422b20a29b8743d..383f36ef1d50c7e00e55d3e0f6444f75bd8357b1 100644 (file)
--- a/src/backend/catalog/pg_collation.c
+++ b/src/backend/catalog/pg_collation.c
@@ -131,7 +131,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 ccd0fe1997def001bb782f1f819d682657d76a64..013707495d11c91460490ad6b40c9d54fc392667 100644 (file)
--- a/src/backend/catalog/pg_operator.c
+++ b/src/backend/catalog/pg_operator.c
@@ -857,5 +857,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 6250b0735c1d0303c7a34c575a37fdb0950f80ec..18a2c1a3c081c69ad638b691ad09f2274382770b 100644 (file)
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -562,8 +562,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);
@@ -617,8 +616,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 de5c63defe5f292d08145fdb543e93ee4acced4b..c31f6e9582b69c2e32c8dfc1cfb8e3acc4719a2f 100644 (file)
--- a/src/backend/catalog/pg_type.c
+++ b/src/backend/catalog/pg_type.c
@@ -484,7 +484,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,
@@ -525,7 +525,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)
@@ -536,12 +536,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);