From 988cccc620dd8c16d77f88ede167b22056176324 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 23 Jul 2011 16:59:39 -0400 Subject: [PATCH] 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 | 2 +- src/backend/catalog/pg_collation.c | 2 +- src/backend/catalog/pg_conversion.c | 2 +- src/backend/catalog/pg_depend.c | 32 ++++++++++++++++++++++++++++- src/backend/catalog/pg_namespace.c | 2 +- src/backend/catalog/pg_operator.c | 2 +- src/backend/catalog/pg_proc.c | 6 ++---- src/backend/catalog/pg_type.c | 11 +++++----- src/backend/commands/foreigncmds.c | 6 +++--- src/backend/commands/functioncmds.c | 10 ++++----- src/backend/commands/opclasscmds.c | 4 ++-- src/backend/commands/proclang.c | 6 ++---- src/backend/commands/tsearchcmds.c | 9 ++++---- src/include/catalog/dependency.h | 3 ++- 14 files changed, 61 insertions(+), 36 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 7cf44d1f25..4399493625 100644 --- 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 28a016b972..ce9bfae702 100644 --- 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 1ef6a9d24e..c84dbc6ae4 100644 --- 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 67aad86d4e..c1a4340190 100644 --- 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 172f99196c..ceebac2f7e 100644 --- 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 4d2d7b7e26..772e5d438d 100644 --- 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 ec229810ca..f2f0872159 100644 --- 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 f608229823..ea52f67dab 100644 --- 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 643ba91bfe..d16932ba6a 100644 --- 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 03da168ff2..92abd44a60 100644 --- 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 aff5ac6ec4..2bb0d4c3b5 100644 --- 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 b36f31ee6d..98770c5a61 100644 --- 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 3355eaafda..deac1062ef 100644 --- 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 a3bd729156..5dfb25fee7 100644 --- 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); -- 2.40.0