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

File tree

14 files changed

+61
-36
lines changed

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

0 commit comments

Comments
 (0)