Skip to content

Commit e5bc945

Browse files
tglsfdctglsfdc
committed
Explicitly list dependent types as extension members in pg_depend.
Auto-generated array types, multirange types, and relation rowtypes are treated as dependent objects: they can't be dropped separately from the base object, nor can they have their own ownership or permissions. We previously felt that, for objects that are in an extension, only the base object needs to be listed as an extension member in pg_depend. While that's sufficient to prevent inappropriate drops, it results in undesirable answers if someone asks whether a dependent type belongs to the extension. It looks like the dependent type is just some random separately-created object that happens to depend on the base object. Notably, this results in postgres_fdw concluding that expressions involving an array type are not shippable to the remote server, even when the defining extension has been whitelisted. To fix, cause GenerateTypeDependencies to make extension dependencies for dependent types as well as their base objects, and adjust ExecAlterExtensionContentsStmt so that object addition and removal operations recurse to dependent types. The latter change means that pg_upgrade of a type-defining extension will end with the dependent type(s) now also listed as extension members, even if they were not that way in the source database. Normally we want pg_upgrade to precisely reproduce the source extension's state, but it seems desirable to make an exception here. This is arguably a bug fix, but we can't back-patch it since it causes changes in the expected contents of pg_depend. (Because it does, I've bumped catversion, even though there's no change in the immediate post-initdb catalog contents.) Tom Lane and David Geier Discussion: https://postgr.es/m/4a847c55-489f-4e8d-a664-fc6b1cbe306f@gmail.com
1 parent dc8f2d7 commit e5bc945

File tree

9 files changed

+265
-25
lines changed

9 files changed

+265
-25
lines changed

‎src/backend/catalog/pg_type.c

+12-5
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ TypeCreate(Oid newTypeOid,
539539
* etc.
540540
*
541541
* We make an extension-membership dependency if we're in an extension
542-
* script and makeExtensionDep is true (and isDependentType isn't true).
542+
* script and makeExtensionDep is true.
543543
* makeExtensionDep should be true when creating a new type or replacing a
544544
* shell type, but not for ALTER TYPE on an existing type. Passing false
545545
* causes the type's extension membership to be left alone.
@@ -599,7 +599,7 @@ GenerateTypeDependencies(HeapTuple typeTuple,
599599
ObjectAddressSet(myself, TypeRelationId, typeObjectId);
600600

601601
/*
602-
* Make dependencies on namespace, owner, ACL, extension.
602+
* Make dependencies on namespace, owner, ACL.
603603
*
604604
* Skip these for a dependent type, since it will have such dependencies
605605
* indirectly through its depended-on type or relation. An exception is
@@ -624,11 +624,18 @@ GenerateTypeDependencies(HeapTuple typeTuple,
624624

625625
recordDependencyOnNewAcl(TypeRelationId, typeObjectId, 0,
626626
typeForm->typowner, typacl);
627-
628-
if (makeExtensionDep)
629-
recordDependencyOnCurrentExtension(&myself, rebuild);
630627
}
631628

629+
/*
630+
* Make extension dependency if requested.
631+
*
632+
* We used to skip this for dependent types, but it seems better to record
633+
* their extension membership explicitly; otherwise code such as
634+
* postgres_fdw's shippability test will be fooled.
635+
*/
636+
if (makeExtensionDep)
637+
recordDependencyOnCurrentExtension(&myself, rebuild);
638+
632639
/* Normal dependencies on the I/O and support functions */
633640
if (OidIsValid(typeForm->typinput))
634641
{

‎src/backend/commands/extension.c

+73-10
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,9 @@ static void ApplyExtensionUpdates(Oid extensionOid,
129129
char *origSchemaName,
130130
bool cascade,
131131
bool is_create);
132+
static void ExecAlterExtensionContentsRecurse(AlterExtensionContentsStmt *stmt,
133+
ObjectAddress extension,
134+
ObjectAddress object);
132135
static char *read_whole_file(const char *filename, int *length);
133136

134137

@@ -3292,7 +3295,6 @@ ExecAlterExtensionContentsStmt(AlterExtensionContentsStmt *stmt,
32923295
ObjectAddress extension;
32933296
ObjectAddress object;
32943297
Relation relation;
3295-
Oid oldExtension;
32963298

32973299
switch (stmt->objtype)
32983300
{
@@ -3347,6 +3349,38 @@ ExecAlterExtensionContentsStmt(AlterExtensionContentsStmt *stmt,
33473349
check_object_ownership(GetUserId(), stmt->objtype, object,
33483350
stmt->object, relation);
33493351

3352+
/* Do the update, recursing to any dependent objects */
3353+
ExecAlterExtensionContentsRecurse(stmt, extension, object);
3354+
3355+
/* Finish up */
3356+
InvokeObjectPostAlterHook(ExtensionRelationId, extension.objectId, 0);
3357+
3358+
/*
3359+
* If get_object_address() opened the relation for us, we close it to keep
3360+
* the reference count correct - but we retain any locks acquired by
3361+
* get_object_address() until commit time, to guard against concurrent
3362+
* activity.
3363+
*/
3364+
if (relation != NULL)
3365+
relation_close(relation, NoLock);
3366+
3367+
return extension;
3368+
}
3369+
3370+
/*
3371+
* ExecAlterExtensionContentsRecurse
3372+
* Subroutine for ExecAlterExtensionContentsStmt
3373+
*
3374+
* Do the bare alteration of object's membership in extension,
3375+
* without permission checks. Recurse to dependent objects, if any.
3376+
*/
3377+
static void
3378+
ExecAlterExtensionContentsRecurse(AlterExtensionContentsStmt *stmt,
3379+
ObjectAddress extension,
3380+
ObjectAddress object)
3381+
{
3382+
Oid oldExtension;
3383+
33503384
/*
33513385
* Check existing extension membership.
33523386
*/
@@ -3430,18 +3464,47 @@ ExecAlterExtensionContentsStmt(AlterExtensionContentsStmt *stmt,
34303464
removeExtObjInitPriv(object.objectId, object.classId);
34313465
}
34323466

3433-
InvokeObjectPostAlterHook(ExtensionRelationId, extension.objectId, 0);
3434-
34353467
/*
3436-
* If get_object_address() opened the relation for us, we close it to keep
3437-
* the reference count correct - but we retain any locks acquired by
3438-
* get_object_address() until commit time, to guard against concurrent
3439-
* activity.
3468+
* Recurse to any dependent objects; currently, this includes the array
3469+
* type of a base type, the multirange type associated with a range type,
3470+
* and the rowtype of a table.
34403471
*/
3441-
if (relation != NULL)
3442-
relation_close(relation, NoLock);
3472+
if (object.classId == TypeRelationId)
3473+
{
3474+
ObjectAddress depobject;
34433475

3444-
return extension;
3476+
depobject.classId = TypeRelationId;
3477+
depobject.objectSubId = 0;
3478+
3479+
/* If it has an array type, update that too */
3480+
depobject.objectId = get_array_type(object.objectId);
3481+
if (OidIsValid(depobject.objectId))
3482+
ExecAlterExtensionContentsRecurse(stmt, extension, depobject);
3483+
3484+
/* If it is a range type, update the associated multirange too */
3485+
if (type_is_range(object.objectId))
3486+
{
3487+
depobject.objectId = get_range_multirange(object.objectId);
3488+
if (!OidIsValid(depobject.objectId))
3489+
ereport(ERROR,
3490+
(errcode(ERRCODE_UNDEFINED_OBJECT),
3491+
errmsg("could not find multirange type for data type %s",
3492+
format_type_be(object.objectId))));
3493+
ExecAlterExtensionContentsRecurse(stmt, extension, depobject);
3494+
}
3495+
}
3496+
if (object.classId == RelationRelationId)
3497+
{
3498+
ObjectAddress depobject;
3499+
3500+
depobject.classId = TypeRelationId;
3501+
depobject.objectSubId = 0;
3502+
3503+
/* It might not have a rowtype, but if it does, update that */
3504+
depobject.objectId = get_rel_type_id(object.objectId);
3505+
if (OidIsValid(depobject.objectId))
3506+
ExecAlterExtensionContentsRecurse(stmt, extension, depobject);
3507+
}
34453508
}
34463509

34473510
/*

‎src/include/catalog/catversion.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,6 @@
5757
*/
5858

5959
/* yyyymmddN */
60-
#define CATALOG_VERSION_NO 202403041
60+
#define CATALOG_VERSION_NO 202403042
6161

6262
#endif

‎src/test/modules/test_extensions/Makefile

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ MODULE = test_extensions
44
PGFILEDESC = "test_extensions - regression testing for EXTENSION support"
55

66
EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
7-
test_ext7 test_ext8 test_ext_cine test_ext_cor \
7+
test_ext7 test_ext8 test_ext9 test_ext_cine test_ext_cor \
88
test_ext_cyclic1 test_ext_cyclic2 \
99
test_ext_extschema \
1010
test_ext_evttrig \
@@ -13,6 +13,7 @@ EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
1313
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
1414
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
1515
test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
16+
test_ext9--1.0.sql \
1617
test_ext_cine--1.0.sql test_ext_cine--1.0--1.1.sql \
1718
test_ext_cor--1.0.sql \
1819
test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql \

0 commit comments

Comments
 (0)