Skip to content

Commit 988cccc

Browse files
tglsfdctglsfdc
committed
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.
1 parent 6f1be5a commit 988cccc

14 files changed

+61
-36
lines changed

‎src/backend/catalog/heap.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1256,7 +1256,7 @@ heap_create_with_catalog(const char *relname,
12561256

12571257
recordDependencyOnOwner(RelationRelationId, relid, ownerid);
12581258

1259-
recordDependencyOnCurrentExtension(&myself);
1259+
recordDependencyOnCurrentExtension(&myself, false);
12601260

12611261
if (reloftypeid)
12621262
{

‎src/backend/catalog/pg_collation.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ CollationCreate(const char *collname, Oid collnamespace,
132132
collowner);
133133

134134
/* dependency on extension */
135-
recordDependencyOnCurrentExtension(&myself);
135+
recordDependencyOnCurrentExtension(&myself, false);
136136

137137
/* Post creation hook for new collation */
138138
InvokeObjectAccessHook(OAT_POST_CREATE,

‎src/backend/catalog/pg_conversion.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ ConversionCreate(const char *conname, Oid connamespace,
133133
conowner);
134134

135135
/* dependency on extension */
136-
recordDependencyOnCurrentExtension(&myself);
136+
recordDependencyOnCurrentExtension(&myself, false);
137137

138138
/* Post creation hook for new conversion */
139139
InvokeObjectAccessHook(OAT_POST_CREATE,

‎src/backend/catalog/pg_depend.c

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,14 +130,44 @@ recordMultipleDependencies(const ObjectAddress *depender,
130130
*
131131
* This must be called during creation of any user-definable object type
132132
* that could be a member of an extension.
133+
*
134+
* If isReplace is true, the object already existed (or might have already
135+
* existed), so we must check for a pre-existing extension membership entry.
136+
* Passing false is a guarantee that the object is newly created, and so
137+
* could not already be a member of any extension.
133138
*/
134139
void
135-
recordDependencyOnCurrentExtension(const ObjectAddress *object)
140+
recordDependencyOnCurrentExtension(const ObjectAddress *object,
141+
bool isReplace)
136142
{
143+
/* Only whole objects can be extension members */
144+
Assert(object->objectSubId == 0);
145+
137146
if (creating_extension)
138147
{
139148
ObjectAddress extension;
140149

150+
/* Only need to check for existing membership if isReplace */
151+
if (isReplace)
152+
{
153+
Oid oldext;
154+
155+
oldext = getExtensionOfObject(object->classId, object->objectId);
156+
if (OidIsValid(oldext))
157+
{
158+
/* If already a member of this extension, nothing to do */
159+
if (oldext == CurrentExtensionObject)
160+
return;
161+
/* Already a member of some other extension, so reject */
162+
ereport(ERROR,
163+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
164+
errmsg("%s is already a member of extension \"%s\"",
165+
getObjectDescription(object),
166+
get_extension_name(oldext))));
167+
}
168+
}
169+
170+
/* OK, record it as a member of CurrentExtensionObject */
141171
extension.classId = ExtensionRelationId;
142172
extension.objectId = CurrentExtensionObject;
143173
extension.objectSubId = 0;

‎src/backend/catalog/pg_namespace.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ NamespaceCreate(const char *nspName, Oid ownerId)
8383
recordDependencyOnOwner(NamespaceRelationId, nspoid, ownerId);
8484

8585
/* dependency on extension */
86-
recordDependencyOnCurrentExtension(&myself);
86+
recordDependencyOnCurrentExtension(&myself, false);
8787

8888
/* Post creation hook for new schema */
8989
InvokeObjectAccessHook(OAT_POST_CREATE, NamespaceRelationId, nspoid, 0);

‎src/backend/catalog/pg_operator.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -855,5 +855,5 @@ makeOperatorDependencies(HeapTuple tuple)
855855
oper->oprowner);
856856

857857
/* Dependency on extension */
858-
recordDependencyOnCurrentExtension(&myself);
858+
recordDependencyOnCurrentExtension(&myself, true);
859859
}

‎src/backend/catalog/pg_proc.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -564,8 +564,7 @@ ProcedureCreate(const char *procedureName,
564564
* Create dependencies for the new function. If we are updating an
565565
* existing function, first delete any existing pg_depend entries.
566566
* (However, since we are not changing ownership or permissions, the
567-
* shared dependencies do *not* need to change, and we leave them alone.
568-
* We also don't change any pre-existing extension-membership dependency.)
567+
* shared dependencies do *not* need to change, and we leave them alone.)
569568
*/
570569
if (is_update)
571570
deleteDependencyRecordsFor(ProcedureRelationId, retval, true);
@@ -619,8 +618,7 @@ ProcedureCreate(const char *procedureName,
619618
}
620619

621620
/* dependency on extension */
622-
if (!is_update)
623-
recordDependencyOnCurrentExtension(&myself);
621+
recordDependencyOnCurrentExtension(&myself, is_update);
624622

625623
heap_freetuple(tup);
626624

‎src/backend/catalog/pg_type.c

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ TypeCreate(Oid newTypeOid,
480480
*
481481
* If rebuild is true, we remove existing dependencies and rebuild them
482482
* from scratch. This is needed for ALTER TYPE, and also when replacing
483-
* a shell type. We don't remove/rebuild extension dependencies, though.
483+
* a shell type.
484484
*/
485485
void
486486
GenerateTypeDependencies(Oid typeNamespace,
@@ -521,7 +521,7 @@ GenerateTypeDependencies(Oid typeNamespace,
521521
* For a relation rowtype (that's not a composite type), we should skip
522522
* these because we'll depend on them indirectly through the pg_class
523523
* entry. Likewise, skip for implicit arrays since we'll depend on them
524-
* through the element type. The same goes for extension membership.
524+
* through the element type.
525525
*/
526526
if ((!OidIsValid(relationOid) || relationKind == RELKIND_COMPOSITE_TYPE) &&
527527
!isImplicitArray)
@@ -532,12 +532,11 @@ GenerateTypeDependencies(Oid typeNamespace,
532532
recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
533533

534534
recordDependencyOnOwner(TypeRelationId, typeObjectId, owner);
535-
536-
/* dependency on extension */
537-
if (!rebuild)
538-
recordDependencyOnCurrentExtension(&myself);
539535
}
540536

537+
/* dependency on extension */
538+
recordDependencyOnCurrentExtension(&myself, rebuild);
539+
541540
/* Normal dependencies on the I/O functions */
542541
if (OidIsValid(inputProcedure))
543542
{

‎src/backend/commands/foreigncmds.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,7 @@ CreateForeignDataWrapper(CreateFdwStmt *stmt)
515515
recordDependencyOnOwner(ForeignDataWrapperRelationId, fdwId, ownerId);
516516

517517
/* dependency on extension */
518-
recordDependencyOnCurrentExtension(&myself);
518+
recordDependencyOnCurrentExtension(&myself, false);
519519

520520
/* Post creation hook for new foreign data wrapper */
521521
InvokeObjectAccessHook(OAT_POST_CREATE,
@@ -857,7 +857,7 @@ CreateForeignServer(CreateForeignServerStmt *stmt)
857857
recordDependencyOnOwner(ForeignServerRelationId, srvId, ownerId);
858858

859859
/* dependency on extension */
860-
recordDependencyOnCurrentExtension(&myself);
860+
recordDependencyOnCurrentExtension(&myself, false);
861861

862862
/* Post creation hook for new foreign server */
863863
InvokeObjectAccessHook(OAT_POST_CREATE, ForeignServerRelationId, srvId, 0);
@@ -1137,7 +1137,7 @@ CreateUserMapping(CreateUserMappingStmt *stmt)
11371137
}
11381138

11391139
/* dependency on extension */
1140-
recordDependencyOnCurrentExtension(&myself);
1140+
recordDependencyOnCurrentExtension(&myself, false);
11411141

11421142
/* Post creation hook for new user mapping */
11431143
InvokeObjectAccessHook(OAT_POST_CREATE, UserMappingRelationId, umId, 0);

‎src/backend/commands/functioncmds.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1489,6 +1489,7 @@ CreateCast(CreateCastStmt *stmt)
14891489
char sourcetyptype;
14901490
char targettyptype;
14911491
Oid funcid;
1492+
Oid castid;
14921493
int nargs;
14931494
char castcontext;
14941495
char castmethod;
@@ -1734,13 +1735,13 @@ CreateCast(CreateCastStmt *stmt)
17341735

17351736
tuple = heap_form_tuple(RelationGetDescr(relation), values, nulls);
17361737

1737-
simple_heap_insert(relation, tuple);
1738+
castid = simple_heap_insert(relation, tuple);
17381739

17391740
CatalogUpdateIndexes(relation, tuple);
17401741

17411742
/* make dependency entries */
17421743
myself.classId = CastRelationId;
1743-
myself.objectId = HeapTupleGetOid(tuple);
1744+
myself.objectId = castid;
17441745
myself.objectSubId = 0;
17451746

17461747
/* dependency on source type */
@@ -1765,11 +1766,10 @@ CreateCast(CreateCastStmt *stmt)
17651766
}
17661767

17671768
/* dependency on extension */
1768-
recordDependencyOnCurrentExtension(&myself);
1769+
recordDependencyOnCurrentExtension(&myself, false);
17691770

17701771
/* Post creation hook for new cast */
1771-
InvokeObjectAccessHook(OAT_POST_CREATE,
1772-
CastRelationId, myself.objectId, 0);
1772+
InvokeObjectAccessHook(OAT_POST_CREATE, CastRelationId, castid, 0);
17731773

17741774
heap_freetuple(tuple);
17751775

‎src/backend/commands/opclasscmds.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ CreateOpFamily(char *amname, char *opfname, Oid namespaceoid, Oid amoid)
310310
recordDependencyOnOwner(OperatorFamilyRelationId, opfamilyoid, GetUserId());
311311

312312
/* dependency on extension */
313-
recordDependencyOnCurrentExtension(&myself);
313+
recordDependencyOnCurrentExtension(&myself, false);
314314

315315
/* Post creation hook for new operator family */
316316
InvokeObjectAccessHook(OAT_POST_CREATE,
@@ -713,7 +713,7 @@ DefineOpClass(CreateOpClassStmt *stmt)
713713
recordDependencyOnOwner(OperatorClassRelationId, opclassoid, GetUserId());
714714

715715
/* dependency on extension */
716-
recordDependencyOnCurrentExtension(&myself);
716+
recordDependencyOnCurrentExtension(&myself, false);
717717

718718
/* Post creation hook for new operator class */
719719
InvokeObjectAccessHook(OAT_POST_CREATE,

‎src/backend/commands/proclang.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -388,8 +388,7 @@ create_proc_lang(const char *languageName, bool replace,
388388
* Create dependencies for the new language. If we are updating an
389389
* existing language, first delete any existing pg_depend entries.
390390
* (However, since we are not changing ownership or permissions, the
391-
* shared dependencies do *not* need to change, and we leave them alone.
392-
* We also don't change any pre-existing extension-membership dependency.)
391+
* shared dependencies do *not* need to change, and we leave them alone.)
393392
*/
394393
myself.classId = LanguageRelationId;
395394
myself.objectId = HeapTupleGetOid(tup);
@@ -404,8 +403,7 @@ create_proc_lang(const char *languageName, bool replace,
404403
languageOwner);
405404

406405
/* dependency on extension */
407-
if (!is_update)
408-
recordDependencyOnCurrentExtension(&myself);
406+
recordDependencyOnCurrentExtension(&myself, is_update);
409407

410408
/* dependency on the PL handler function */
411409
referenced.classId = ProcedureRelationId;

‎src/backend/commands/tsearchcmds.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ makeParserDependencies(HeapTuple tuple)
142142
recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
143143

144144
/* dependency on extension */
145-
recordDependencyOnCurrentExtension(&myself);
145+
recordDependencyOnCurrentExtension(&myself, false);
146146

147147
/* dependencies on functions */
148148
referenced.classId = ProcedureRelationId;
@@ -479,7 +479,7 @@ makeDictionaryDependencies(HeapTuple tuple)
479479
recordDependencyOnOwner(myself.classId, myself.objectId, dict->dictowner);
480480

481481
/* dependency on extension */
482-
recordDependencyOnCurrentExtension(&myself);
482+
recordDependencyOnCurrentExtension(&myself, false);
483483

484484
/* dependency on template */
485485
referenced.classId = TSTemplateRelationId;
@@ -1069,7 +1069,7 @@ makeTSTemplateDependencies(HeapTuple tuple)
10691069
recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
10701070

10711071
/* dependency on extension */
1072-
recordDependencyOnCurrentExtension(&myself);
1072+
recordDependencyOnCurrentExtension(&myself, false);
10731073

10741074
/* dependencies on functions */
10751075
referenced.classId = ProcedureRelationId;
@@ -1417,8 +1417,7 @@ makeConfigurationDependencies(HeapTuple tuple, bool removeOld,
14171417
recordDependencyOnOwner(myself.classId, myself.objectId, cfg->cfgowner);
14181418

14191419
/* dependency on extension */
1420-
if (!removeOld)
1421-
recordDependencyOnCurrentExtension(&myself);
1420+
recordDependencyOnCurrentExtension(&myself, removeOld);
14221421

14231422
/* dependency on parser */
14241423
referenced.classId = TSParserRelationId;

‎src/include/catalog/dependency.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,8 @@ extern void recordMultipleDependencies(const ObjectAddress *depender,
201201
int nreferenced,
202202
DependencyType behavior);
203203

204-
extern void recordDependencyOnCurrentExtension(const ObjectAddress *object);
204+
extern void recordDependencyOnCurrentExtension(const ObjectAddress *object,
205+
bool isReplace);
205206

206207
extern long deleteDependencyRecordsFor(Oid classId, Oid objectId,
207208
bool skipExtensionDeps);

0 commit comments

Comments
 (0)