Skip to content

Commit eb15f26

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 16a6a70 commit eb15f26

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
@@ -1242,7 +1242,7 @@ heap_create_with_catalog(const char *relname,
12421242

12431243
recordDependencyOnOwner(RelationRelationId, relid, ownerid);
12441244

1245-
recordDependencyOnCurrentExtension(&myself);
1245+
recordDependencyOnCurrentExtension(&myself, false);
12461246

12471247
if (reloftypeid)
12481248
{

‎src/backend/catalog/pg_collation.c

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

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

136136
/* Post creation hook for new collation */
137137
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
@@ -857,5 +857,5 @@ makeOperatorDependencies(HeapTuple tuple)
857857
oper->oprowner);
858858

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

‎src/backend/catalog/pg_proc.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -562,8 +562,7 @@ ProcedureCreate(const char *procedureName,
562562
* Create dependencies for the new function. If we are updating an
563563
* existing function, first delete any existing pg_depend entries.
564564
* (However, since we are not changing ownership or permissions, the
565-
* shared dependencies do *not* need to change, and we leave them alone.
566-
* We also don't change any pre-existing extension-membership dependency.)
565+
* shared dependencies do *not* need to change, and we leave them alone.)
567566
*/
568567
if (is_update)
569568
deleteDependencyRecordsFor(ProcedureRelationId, retval, true);
@@ -617,8 +616,7 @@ ProcedureCreate(const char *procedureName,
617616
}
618617

619618
/* dependency on extension */
620-
if (!is_update)
621-
recordDependencyOnCurrentExtension(&myself);
619+
recordDependencyOnCurrentExtension(&myself, is_update);
622620

623621
heap_freetuple(tup);
624622

‎src/backend/catalog/pg_type.c

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

538538
recordDependencyOnOwner(TypeRelationId, typeObjectId, owner);
539-
540-
/* dependency on extension */
541-
if (!rebuild)
542-
recordDependencyOnCurrentExtension(&myself);
543539
}
544540

541+
/* dependency on extension */
542+
recordDependencyOnCurrentExtension(&myself, rebuild);
543+
545544
/* Normal dependencies on the I/O functions */
546545
if (OidIsValid(inputProcedure))
547546
{

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