Re: postgres_fdw fails to see that array type belongs to extension

I wrote:
> There's one big remaining to-do item, I think: experimentation with
> pg_upgrade proves that a binary upgrade fails to fix the extension
> membership of arrays/rowtypes.  That's because pg_dump needs to
> manually reconstruct the membership dependencies, and it thinks that
> it doesn't need to do anything for implicit arrays.  Normally the
> point of that is that we want to exactly reconstruct the extension's
> contents, but I think in this case we'd really like to add the
> additional pg_depend entries even if they weren't there before.
> Otherwise people wouldn't get the new behavior until they do a
> full dump/reload.

> I can see two ways we could do that:

> * add logic to pg_dump

> * modify ALTER EXTENSION ADD TYPE so that it automatically recurses
> from a base type to its array type (and I guess we'd need to add
> something for relation rowtypes and multiranges, too).

> I think I like the latter approach because it's like how we
> handle ownership: pg_dump doesn't emit any commands to explicitly
> change the ownership of dependent types, either.  (But see [1].)
> We could presumably steal some logic from ALTER TYPE OWNER.
> I've not tried to code that here, though.

Now that the multirange issue is fixed (3e8235ba4), here's a
new version that includes the needed recursion in ALTER EXTENSION.
I spent some more effort on a proper test case, too.

            regards, tom lane

diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
index fe47be38d0..1a0460b491 100644
--- a/src/backend/catalog/pg_type.c
+++ b/src/backend/catalog/pg_type.c
@@ -540,7 +540,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.
@@ -600,7 +600,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
@@ -625,11 +625,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))
     {
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 226f85d0e3..6772d6a8d2 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -131,6 +131,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);


@@ -3294,7 +3297,6 @@ ExecAlterExtensionContentsStmt(AlterExtensionContentsStmt *stmt,
     ObjectAddress extension;
     ObjectAddress object;
     Relation    relation;
-    Oid            oldExtension;

     switch (stmt->objtype)
     {
@@ -3349,6 +3351,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.
      */
@@ -3432,18 +3466,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);
+    }
 }

 /*
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index 1388c0fb0b..7d95d6b924 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/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 \
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out
b/src/test/modules/test_extensions/expected/test_extensions.out
index 472627a232..a7ab244e87 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/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.
diff --git a/src/test/modules/test_extensions/meson.build b/src/test/modules/test_extensions/meson.build
index aec99dc20d..9b8d0a1016 100644
--- a/src/test/modules/test_extensions/meson.build
+++ b/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',
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql
b/src/test/modules/test_extensions/sql/test_extensions.sql
index 51327cc321..c5b64f47c6 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/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.

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
index 0000000000..427070bece
--- /dev/null
+++ b/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);
diff --git a/src/test/modules/test_extensions/test_ext9.control b/src/test/modules/test_extensions/test_ext9.control
new file mode 100644
index 0000000000..c36eddb178
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext9.control
@@ -0,0 +1,3 @@
+comment = 'test_ext9'
+default_version = '1.0'
+relocatable = true