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

src/backend/catalog/pg_type.c

@@ -539,7 +539,7 @@ TypeCreate(Oid newTypeOid,
  * etc.
  *
  * We make an extension-membership dependency if we're in an extension
- * script and makeExtensionDep is true (and isDependentType isn't true).
+ * script and makeExtensionDep is true.
  * makeExtensionDep should be true when creating a new type or replacing a
  * shell type, but not for ALTER TYPE on an existing type.  Passing false
  * causes the type's extension membership to be left alone.
@@ -599,7 +599,7 @@ GenerateTypeDependencies(HeapTuple typeTuple,
 	ObjectAddressSet(myself, TypeRelationId, typeObjectId);
 
 	/*
-	 * Make dependencies on namespace, owner, ACL, extension.
+	 * Make dependencies on namespace, owner, ACL.
 	 *
 	 * Skip these for a dependent type, since it will have such dependencies
 	 * indirectly through its depended-on type or relation.  An exception is
@@ -624,11 +624,18 @@ GenerateTypeDependencies(HeapTuple typeTuple,
 
 		recordDependencyOnNewAcl(TypeRelationId, typeObjectId, 0,
 								 typeForm->typowner, typacl);
-
-		if (makeExtensionDep)
-			recordDependencyOnCurrentExtension(&myself, rebuild);
 	}
 
+	/*
+	 * Make extension dependency if requested.
+	 *
+	 * We used to skip this for dependent types, but it seems better to record
+	 * their extension membership explicitly; otherwise code such as
+	 * postgres_fdw's shippability test will be fooled.
+	 */
+	if (makeExtensionDep)
+		recordDependencyOnCurrentExtension(&myself, rebuild);
+
 	/* Normal dependencies on the I/O and support functions */
 	if (OidIsValid(typeForm->typinput))
 	{

src/backend/commands/extension.c

@@ -129,6 +129,9 @@ static void ApplyExtensionUpdates(Oid extensionOid,
 								  char *origSchemaName,
 								  bool cascade,
 								  bool is_create);
+static void ExecAlterExtensionContentsRecurse(AlterExtensionContentsStmt *stmt,
+											  ObjectAddress extension,
+											  ObjectAddress object);
 static char *read_whole_file(const char *filename, int *length);
 
 
@@ -3292,7 +3295,6 @@ ExecAlterExtensionContentsStmt(AlterExtensionContentsStmt *stmt,
 	ObjectAddress extension;
 	ObjectAddress object;
 	Relation	relation;
-	Oid			oldExtension;
 
 	switch (stmt->objtype)
 	{
@@ -3347,6 +3349,38 @@ ExecAlterExtensionContentsStmt(AlterExtensionContentsStmt *stmt,
 	check_object_ownership(GetUserId(), stmt->objtype, object,
 						   stmt->object, relation);
 
+	/* Do the update, recursing to any dependent objects */
+	ExecAlterExtensionContentsRecurse(stmt, extension, object);
+
+	/* Finish up */
+	InvokeObjectPostAlterHook(ExtensionRelationId, extension.objectId, 0);
+
+	/*
+	 * If get_object_address() opened the relation for us, we close it to keep
+	 * the reference count correct - but we retain any locks acquired by
+	 * get_object_address() until commit time, to guard against concurrent
+	 * activity.
+	 */
+	if (relation != NULL)
+		relation_close(relation, NoLock);
+
+	return extension;
+}
+
+/*
+ * ExecAlterExtensionContentsRecurse
+ *		Subroutine for ExecAlterExtensionContentsStmt
+ *
+ * Do the bare alteration of object's membership in extension,
+ * without permission checks.  Recurse to dependent objects, if any.
+ */
+static void
+ExecAlterExtensionContentsRecurse(AlterExtensionContentsStmt *stmt,
+								  ObjectAddress extension,
+								  ObjectAddress object)
+{
+	Oid			oldExtension;
+
 	/*
 	 * Check existing extension membership.
 	 */
@@ -3430,18 +3464,47 @@ ExecAlterExtensionContentsStmt(AlterExtensionContentsStmt *stmt,
 		removeExtObjInitPriv(object.objectId, object.classId);
 	}
 
-	InvokeObjectPostAlterHook(ExtensionRelationId, extension.objectId, 0);
-
 	/*
-	 * If get_object_address() opened the relation for us, we close it to keep
-	 * the reference count correct - but we retain any locks acquired by
-	 * get_object_address() until commit time, to guard against concurrent
-	 * activity.
+	 * Recurse to any dependent objects; currently, this includes the array
+	 * type of a base type, the multirange type associated with a range type,
+	 * and the rowtype of a table.
 	 */
-	if (relation != NULL)
-		relation_close(relation, NoLock);
+	if (object.classId == TypeRelationId)
+	{
+		ObjectAddress depobject;
 
-	return extension;
+		depobject.classId = TypeRelationId;
+		depobject.objectSubId = 0;
+
+		/* If it has an array type, update that too */
+		depobject.objectId = get_array_type(object.objectId);
+		if (OidIsValid(depobject.objectId))
+			ExecAlterExtensionContentsRecurse(stmt, extension, depobject);
+
+		/* If it is a range type, update the associated multirange too */
+		if (type_is_range(object.objectId))
+		{
+			depobject.objectId = get_range_multirange(object.objectId);
+			if (!OidIsValid(depobject.objectId))
+				ereport(ERROR,
+						(errcode(ERRCODE_UNDEFINED_OBJECT),
+						 errmsg("could not find multirange type for data type %s",
+								format_type_be(object.objectId))));
+			ExecAlterExtensionContentsRecurse(stmt, extension, depobject);
+		}
+	}
+	if (object.classId == RelationRelationId)
+	{
+		ObjectAddress depobject;
+
+		depobject.classId = TypeRelationId;
+		depobject.objectSubId = 0;
+
+		/* It might not have a rowtype, but if it does, update that */
+		depobject.objectId = get_rel_type_id(object.objectId);
+		if (OidIsValid(depobject.objectId))
+			ExecAlterExtensionContentsRecurse(stmt, extension, depobject);
+	}
 }
 
 /*

src/include/catalog/catversion.h

@@ -57,6 +57,6 @@
  */
 
 /*							yyyymmddN */
-#define CATALOG_VERSION_NO	202403041
+#define CATALOG_VERSION_NO	202403042
 
 #endif

src/test/modules/test_extensions/Makefile

@@ -4,7 +4,7 @@ MODULE = test_extensions
 PGFILEDESC = "test_extensions - regression testing for EXTENSION support"
 
 EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
-            test_ext7 test_ext8 test_ext_cine test_ext_cor \
+            test_ext7 test_ext8 test_ext9 test_ext_cine test_ext_cor \
             test_ext_cyclic1 test_ext_cyclic2 \
             test_ext_extschema \
             test_ext_evttrig \
@@ -13,6 +13,7 @@ EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
 DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
        test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
        test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
+       test_ext9--1.0.sql \
        test_ext_cine--1.0.sql test_ext_cine--1.0--1.1.sql \
        test_ext_cor--1.0.sql \
        test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql \

src/test/modules/test_extensions/expected/test_extensions.out

@@ -53,7 +53,13 @@ Objects in extension "test_ext7"
  table ext7_table1
  table ext7_table2
  table old_table1
-(6 rows)
+ type ext7_table1
+ type ext7_table1[]
+ type ext7_table2
+ type ext7_table2[]
+ type old_table1
+ type old_table1[]
+(12 rows)
 
 alter extension test_ext7 update to '2.0';
 \dx+ test_ext7
@@ -62,7 +68,9 @@ Objects in extension "test_ext7"
 -------------------------------
  sequence ext7_table2_col2_seq
  table ext7_table2
-(2 rows)
+ type ext7_table2
+ type ext7_table2[]
+(4 rows)
 
 -- test handling of temp objects created by extensions
 create extension test_ext8;
@@ -79,8 +87,13 @@ order by 1;
  function pg_temp.ext8_temp_even(posint)
  table ext8_table1
  table ext8_temp_table1
+ type ext8_table1
+ type ext8_table1[]
+ type ext8_temp_table1
+ type ext8_temp_table1[]
  type posint
-(5 rows)
+ type posint[]
+(10 rows)
 
 -- Should be possible to drop and recreate this extension
 drop extension test_ext8;
@@ -97,8 +110,13 @@ order by 1;
  function pg_temp.ext8_temp_even(posint)
  table ext8_table1
  table ext8_temp_table1
+ type ext8_table1
+ type ext8_table1[]
+ type ext8_temp_table1
+ type ext8_temp_table1[]
  type posint
-(5 rows)
+ type posint[]
+(10 rows)
 
 -- here we want to start a new session and wait till old one is gone
 select pg_backend_pid() as oldpid \gset
@@ -117,11 +135,119 @@ Objects in extension "test_ext8"
 ----------------------------
  function ext8_even(posint)
  table ext8_table1
+ type ext8_table1
+ type ext8_table1[]
  type posint
-(3 rows)
+ type posint[]
+(6 rows)
 
 -- dropping it should still work
 drop extension test_ext8;
+-- check handling of types as extension members
+create extension test_ext9;
+\dx+ test_ext9
+          Objects in extension "test_ext9"
+                 Object description                 
+----------------------------------------------------
+ cast from varbitrange to varbitmultirange
+ function varbitmultirange()
+ function varbitmultirange(varbitrange)
+ function varbitmultirange(varbitrange[])
+ function varbitrange(bit varying,bit varying)
+ function varbitrange(bit varying,bit varying,text)
+ table sometable
+ type somecomposite
+ type somecomposite[]
+ type sometable
+ type sometable[]
+ type varbitmultirange
+ type varbitmultirange[]
+ type varbitrange
+ type varbitrange[]
+(15 rows)
+
+alter extension test_ext9 drop type varbitrange;
+\dx+ test_ext9
+          Objects in extension "test_ext9"
+                 Object description                 
+----------------------------------------------------
+ cast from varbitrange to varbitmultirange
+ function varbitmultirange()
+ function varbitmultirange(varbitrange)
+ function varbitmultirange(varbitrange[])
+ function varbitrange(bit varying,bit varying)
+ function varbitrange(bit varying,bit varying,text)
+ table sometable
+ type somecomposite
+ type somecomposite[]
+ type sometable
+ type sometable[]
+(11 rows)
+
+alter extension test_ext9 add type varbitrange;
+\dx+ test_ext9
+          Objects in extension "test_ext9"
+                 Object description                 
+----------------------------------------------------
+ cast from varbitrange to varbitmultirange
+ function varbitmultirange()
+ function varbitmultirange(varbitrange)
+ function varbitmultirange(varbitrange[])
+ function varbitrange(bit varying,bit varying)
+ function varbitrange(bit varying,bit varying,text)
+ table sometable
+ type somecomposite
+ type somecomposite[]
+ type sometable
+ type sometable[]
+ type varbitmultirange
+ type varbitmultirange[]
+ type varbitrange
+ type varbitrange[]
+(15 rows)
+
+alter extension test_ext9 drop table sometable;
+\dx+ test_ext9
+          Objects in extension "test_ext9"
+                 Object description                 
+----------------------------------------------------
+ cast from varbitrange to varbitmultirange
+ function varbitmultirange()
+ function varbitmultirange(varbitrange)
+ function varbitmultirange(varbitrange[])
+ function varbitrange(bit varying,bit varying)
+ function varbitrange(bit varying,bit varying,text)
+ type somecomposite
+ type somecomposite[]
+ type varbitmultirange
+ type varbitmultirange[]
+ type varbitrange
+ type varbitrange[]
+(12 rows)
+
+alter extension test_ext9 add table sometable;
+\dx+ test_ext9
+          Objects in extension "test_ext9"
+                 Object description                 
+----------------------------------------------------
+ cast from varbitrange to varbitmultirange
+ function varbitmultirange()
+ function varbitmultirange(varbitrange)
+ function varbitmultirange(varbitrange[])
+ function varbitrange(bit varying,bit varying)
+ function varbitrange(bit varying,bit varying,text)
+ table sometable
+ type somecomposite
+ type somecomposite[]
+ type sometable
+ type sometable[]
+ type varbitmultirange
+ type varbitmultirange[]
+ type varbitrange
+ type varbitrange[]
+(15 rows)
+
+drop extension test_ext9;
 -- Test creation of extension in temporary schema with two-phase commit,
 -- which should not work.  This function wrapper is useful for portability.
 -- Avoid noise caused by CONTEXT and NOTICE messages including the temporary
@@ -237,9 +363,12 @@ Objects in extension "test_ext_cor"
 ------------------------------
  function ext_cor_func()
  operator <<@@(point,polygon)
+ type ext_cor_view
+ type ext_cor_view[]
  type test_ext_type
+ type test_ext_type[]
  view ext_cor_view
-(4 rows)
+(7 rows)
 
 --
 -- CREATE IF NOT EXISTS is an entirely unsound thing for an extension
@@ -295,7 +424,13 @@ Objects in extension "test_ext_cine"
  server ext_cine_srv
  table ext_cine_tab1
  table ext_cine_tab2
-(8 rows)
+ type ext_cine_mv
+ type ext_cine_mv[]
+ type ext_cine_tab1
+ type ext_cine_tab1[]
+ type ext_cine_tab2
+ type ext_cine_tab2[]
+(14 rows)
 
 ALTER EXTENSION test_ext_cine UPDATE TO '1.1';
 \dx+ test_ext_cine
@@ -311,7 +446,15 @@ Objects in extension "test_ext_cine"
  table ext_cine_tab1
  table ext_cine_tab2
  table ext_cine_tab3
-(9 rows)
+ type ext_cine_mv
+ type ext_cine_mv[]
+ type ext_cine_tab1
+ type ext_cine_tab1[]
+ type ext_cine_tab2
+ type ext_cine_tab2[]
+ type ext_cine_tab3
+ type ext_cine_tab3[]
+(17 rows)
 
 --
 -- Test @extschema@ syntax.

src/test/modules/test_extensions/meson.build

@@ -18,6 +18,8 @@ test_install_data += files(
   'test_ext7.control',
   'test_ext8--1.0.sql',
   'test_ext8.control',
+  'test_ext9--1.0.sql',
+  'test_ext9.control',
   'test_ext_cine--1.0.sql',
   'test_ext_cine--1.0--1.1.sql',
   'test_ext_cine.control',

src/test/modules/test_extensions/sql/test_extensions.sql

@@ -67,6 +67,19 @@ end';
 -- dropping it should still work
 drop extension test_ext8;
 
+-- check handling of types as extension members
+create extension test_ext9;
+\dx+ test_ext9
+alter extension test_ext9 drop type varbitrange;
+\dx+ test_ext9
+alter extension test_ext9 add type varbitrange;
+\dx+ test_ext9
+alter extension test_ext9 drop table sometable;
+\dx+ test_ext9
+alter extension test_ext9 add table sometable;
+\dx+ test_ext9
+drop extension test_ext9;
+
 -- Test creation of extension in temporary schema with two-phase commit,
 -- which should not work.  This function wrapper is useful for portability.
 

src/test/modules/test_extensions/test_ext9--1.0.sql

@@ -0,0 +1,8 @@
+/* src/test/modules/test_extensions/test_ext9--1.0.sql */
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_ext9" to load this file. \quit
+
+-- check handling of types as extension members
+create type varbitrange as range (subtype = varbit);
+create table sometable (f1 real, f2 real);
+create type somecomposite as (f1 float8, f2 float8);

src/test/modules/test_extensions/test_ext9.control

@@ -0,0 +1,3 @@
+comment = 'test_ext9'
+default_version = '1.0'
+relocatable = true