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.

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)
 		{

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,

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,

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;

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);

src/backend/catalog/pg_operator.c

@@ -855,5 +855,5 @@ makeOperatorDependencies(HeapTuple tuple)
 							oper->oprowner);
 
 	/* Dependency on extension */
-	recordDependencyOnCurrentExtension(&myself);
+	recordDependencyOnCurrentExtension(&myself, true);
 }

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.
+	 * shared dependencies do *not* need to change, and we leave them alone.)
-	 * We also don't change any pre-existing extension-membership dependency.)
 	 */
 	if (is_update)
 		deleteDependencyRecordsFor(ProcedureRelationId, retval, true);
@@ -619,8 +618,7 @@ ProcedureCreate(const char *procedureName,
 	}
 
 	/* dependency on extension */
-	if (!is_update)
+	recordDependencyOnCurrentExtension(&myself, is_update);
-		recordDependencyOnCurrentExtension(&myself);
 
 	heap_freetuple(tup);
 

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,11 +532,10 @@ GenerateTypeDependencies(Oid typeNamespace,
 		recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
 
 		recordDependencyOnOwner(TypeRelationId, typeObjectId, owner);
+	}
 
 	/* dependency on extension */
-		if (!rebuild)
+	recordDependencyOnCurrentExtension(&myself, rebuild);
-			recordDependencyOnCurrentExtension(&myself);
-	}
 
 	/* Normal dependencies on the I/O functions */
 	if (OidIsValid(inputProcedure))

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);

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,
+	InvokeObjectAccessHook(OAT_POST_CREATE, CastRelationId, castid, 0);
-						   CastRelationId, myself.objectId, 0);
 
 	heap_freetuple(tuple);
 

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,

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.
+	 * shared dependencies do *not* need to change, and we leave them alone.)
-	 * We also don't change any pre-existing extension-membership dependency.)
 	 */
 	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, is_update);
-		recordDependencyOnCurrentExtension(&myself);
 
 	/* dependency on the PL handler function */
 	referenced.classId = ProcedureRelationId;

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, removeOld);
-		recordDependencyOnCurrentExtension(&myself);
 
 	/* dependency on parser */
 	referenced.classId = TSParserRelationId;

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);