Explicitly list dependent types as extension members in pg_depend.
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 4 Mar 2024 19:49:31 +0000 (14:49 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 4 Mar 2024 19:49:36 +0000 (14:49 -0500)
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
src/backend/commands/extension.c
src/include/catalog/catversion.h
src/test/modules/test_extensions/Makefile
src/test/modules/test_extensions/expected/test_extensions.out
src/test/modules/test_extensions/meson.build
src/test/modules/test_extensions/sql/test_extensions.sql
src/test/modules/test_extensions/test_ext9--1.0.sql [new file with mode: 0644]
src/test/modules/test_extensions/test_ext9.control [new file with mode: 0644]

index d1d8fa274e4d859fd64fb0fe81843c64a763a9fb..395dec8ed88e8906e72f8b805405bb0f27835c9b 100644 (file)
@@ -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))
    {
index af600d7c9acc6650d2f9607348711a9f04ae1290..77d8c9e1862f4fd9a649d38a1f570795af73b27c 100644 (file)
@@ -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);
+   }
 }
 
 /*
index 208ffbe2a3543ebd7ce75dede2e02786c0300459..fc48f871045eaa28ff37dd06b9a2e3a8b266a20f 100644 (file)
@@ -57,6 +57,6 @@
  */
 
 /*                         yyyymmddN */
-#define CATALOG_VERSION_NO 202403041
+#define CATALOG_VERSION_NO 202403042
 
 #endif
index 1388c0fb0bdca56f0d27d6c95ea8d9903132f39d..7d95d6b92450a608022f4f13f5ae6d2cb04ebe9b 100644 (file)
@@ -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 \
index 472627a232b523d70ee2c794f9e6af10a7e8e263..a7ab244e8750a4799412d4fc8f68f85d1afa1199 100644 (file)
@@ -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.
index aec99dc20d6d1c0a6a1c3204e9ec97aefc290e56..9b8d0a10168ee9649baee0dc5bb6eaffdd12854d 100644 (file)
@@ -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',
index 51327cc32176c50f5116b59d9996a46181fcbaa1..c5b64f47c6b7e49f2b27212c8c07e47effeabec9 100644 (file)
@@ -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.
 
diff --git a/src/test/modules/test_extensions/test_ext9--1.0.sql b/src/test/modules/test_extensions/test_ext9--1.0.sql
new file mode 100644 (file)
index 0000000..427070b
--- /dev/null
@@ -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);
diff --git a/src/test/modules/test_extensions/test_ext9.control b/src/test/modules/test_extensions/test_ext9.control
new file mode 100644 (file)
index 0000000..c36eddb
--- /dev/null
@@ -0,0 +1,3 @@
+comment = 'test_ext9'
+default_version = '1.0'
+relocatable = true