ALTER EXTENSION SET SCHEMA versus dependent types

I happened to notice that the comment for AlterObjectNamespace_oid
claims that

 * ... it doesn't have to deal with certain special cases
 * such as not wanting to process array types --- those should never
 * be direct members of an extension anyway.

This struck me as probably broken in the wake of e5bc9454e
(Explicitly list dependent types as extension members in pg_depend),
and sure enough a moment's worth of testing showed it is:

regression=# create schema s1;
CREATE SCHEMA
regression=# create extension cube with schema s1;
CREATE EXTENSION
regression=# create schema s2;
CREATE SCHEMA
regression=# alter extension cube set schema s2;
ERROR:  cannot alter array type s1.cube[]
HINT:  You can alter type s1.cube, which will alter the array type as well.

So we need to do something about that; and the fact that this escaped
testing shows that our coverage for ALTER EXTENSION SET SCHEMA is
pretty lame.

The attached patch fixes up the code and adds a new test to
the test_extensions module.  The fix basically is to skip the
pg_depend entries for dependent types, assuming that they'll
get dealt with when we process their parent objects.

            regards, tom lane

diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
index 12802b9d3f..4f99ebb447 100644
--- a/src/backend/commands/alter.c
+++ b/src/backend/commands/alter.c
@@ -598,16 +598,16 @@ ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt,
 /*
  * Change an object's namespace given its classOid and object Oid.
  *
- * Objects that don't have a namespace should be ignored.
+ * Objects that don't have a namespace should be ignored, as should
+ * dependent types such as array types.
  *
  * This function is currently used only by ALTER EXTENSION SET SCHEMA,
- * so it only needs to cover object types that can be members of an
- * extension, and it doesn't have to deal with certain special cases
- * such as not wanting to process array types --- those should never
- * be direct members of an extension anyway.
+ * so it only needs to cover object kinds that can be members of an
+ * extension, and it can silently ignore dependent types --- we assume
+ * those will be moved when their parent object is moved.
  *
  * Returns the OID of the object's previous namespace, or InvalidOid if
- * object doesn't have a schema.
+ * object doesn't have a schema or was ignored due to being a dependent type.
  */
 Oid
 AlterObjectNamespace_oid(Oid classId, Oid objid, Oid nspOid,
@@ -631,7 +631,7 @@ AlterObjectNamespace_oid(Oid classId, Oid objid, Oid nspOid,
             }

         case TypeRelationId:
-            oldNspOid = AlterTypeNamespace_oid(objid, nspOid, objsMoved);
+            oldNspOid = AlterTypeNamespace_oid(objid, nspOid, true, objsMoved);
             break;

         case ProcedureRelationId:
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 77d8c9e186..1643c8c69a 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -2940,7 +2940,7 @@ AlterExtensionNamespace(const char *extensionName, const char *newschema, Oid *o

         /*
          * If not all the objects had the same old namespace (ignoring any
-         * that are not in namespaces), complain.
+         * that are not in namespaces or are dependent types), complain.
          */
         if (dep_oldNspOid != InvalidOid && dep_oldNspOid != oldNspOid)
             ereport(ERROR,
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 5bf5e69c5b..e5e67ed64c 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -18018,8 +18018,8 @@ AlterTableNamespaceInternal(Relation rel, Oid oldNspOid, Oid nspOid,

     /* Fix the table's row type too, if it has one */
     if (OidIsValid(rel->rd_rel->reltype))
-        AlterTypeNamespaceInternal(rel->rd_rel->reltype,
-                                   nspOid, false, false, objsMoved);
+        AlterTypeNamespaceInternal(rel->rd_rel->reltype, nspOid,
+                                   false, false, false, objsMoved);

     /* Fix other dependent stuff */
     AlterIndexNamespaces(classRel, rel, oldNspOid, nspOid, objsMoved);
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 315b0feb8e..bb023c7820 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -4068,7 +4068,7 @@ AlterTypeNamespace(List *names, const char *newschema, ObjectType objecttype,
     typename = makeTypeNameFromNameList(names);
     typeOid = typenameTypeId(NULL, typename);

-    /* Don't allow ALTER DOMAIN on a type */
+    /* Don't allow ALTER DOMAIN on a non-domain type */
     if (objecttype == OBJECT_DOMAIN && get_typtype(typeOid) != TYPTYPE_DOMAIN)
         ereport(ERROR,
                 (errcode(ERRCODE_WRONG_OBJECT_TYPE),
@@ -4079,7 +4079,7 @@ AlterTypeNamespace(List *names, const char *newschema, ObjectType objecttype,
     nspOid = LookupCreationNamespace(newschema);

     objsMoved = new_object_addresses();
-    oldNspOid = AlterTypeNamespace_oid(typeOid, nspOid, objsMoved);
+    oldNspOid = AlterTypeNamespace_oid(typeOid, nspOid, false, objsMoved);
     free_object_addresses(objsMoved);

     if (oldschema)
@@ -4090,8 +4090,21 @@ AlterTypeNamespace(List *names, const char *newschema, ObjectType objecttype,
     return myself;
 }

+/*
+ * ALTER TYPE SET SCHEMA, where the caller has already looked up the OIDs
+ * of the type and the target schema and checked the schema's privileges.
+ *
+ * If ignoreDependent is true, we silently ignore dependent types
+ * (array types and table rowtypes) rather than raising errors.
+ *
+ * This entry point is exported for use by AlterObjectNamespace_oid,
+ * which doesn't want errors when it passes OIDs of dependent types.
+ *
+ * Returns the type's old namespace OID, or InvalidOid if we did nothing.
+ */
 Oid
-AlterTypeNamespace_oid(Oid typeOid, Oid nspOid, ObjectAddresses *objsMoved)
+AlterTypeNamespace_oid(Oid typeOid, Oid nspOid, bool ignoreDependent,
+                       ObjectAddresses *objsMoved)
 {
     Oid            elemOid;

@@ -4102,15 +4115,21 @@ AlterTypeNamespace_oid(Oid typeOid, Oid nspOid, ObjectAddresses *objsMoved)
     /* don't allow direct alteration of array types */
     elemOid = get_element_type(typeOid);
     if (OidIsValid(elemOid) && get_array_type(elemOid) == typeOid)
+    {
+        if (ignoreDependent)
+            return InvalidOid;
         ereport(ERROR,
                 (errcode(ERRCODE_WRONG_OBJECT_TYPE),
                  errmsg("cannot alter array type %s",
                         format_type_be(typeOid)),
                  errhint("You can alter type %s, which will alter the array type as well.",
                          format_type_be(elemOid))));
+    }

     /* and do the work */
-    return AlterTypeNamespaceInternal(typeOid, nspOid, false, true, objsMoved);
+    return AlterTypeNamespaceInternal(typeOid, nspOid,
+                                      false, ignoreDependent, true,
+                                      objsMoved);
 }

 /*
@@ -4122,15 +4141,21 @@ AlterTypeNamespace_oid(Oid typeOid, Oid nspOid, ObjectAddresses *objsMoved)
  * if any.  isImplicitArray should be true only when doing this internal
  * recursion (outside callers must never try to move an array type directly).
  *
+ * If ignoreDependent is true, we silently don't process table types.
+ *
  * If errorOnTableType is true, the function errors out if the type is
  * a table type.  ALTER TABLE has to be used to move a table to a new
- * namespace.
+ * namespace.  (This flag is ignored if ignoreDependent is true.)
+ *
+ * We also do nothing if the type is already listed in *objsMoved.
+ * After a successful move, we add the type to *objsMoved.
  *
- * Returns the type's old namespace OID.
+ * Returns the type's old namespace OID, or InvalidOid if we did nothing.
  */
 Oid
 AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
                            bool isImplicitArray,
+                           bool ignoreDependent,
                            bool errorOnTableType,
                            ObjectAddresses *objsMoved)
 {
@@ -4185,15 +4210,21 @@ AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
          get_rel_relkind(typform->typrelid) == RELKIND_COMPOSITE_TYPE);

     /* Enforce not-table-type if requested */
-    if (typform->typtype == TYPTYPE_COMPOSITE && !isCompositeType &&
-        errorOnTableType)
-        ereport(ERROR,
-                (errcode(ERRCODE_WRONG_OBJECT_TYPE),
-                 errmsg("%s is a table's row type",
-                        format_type_be(typeOid)),
-        /* translator: %s is an SQL ALTER command */
-                 errhint("Use %s instead.",
-                         "ALTER TABLE")));
+    if (typform->typtype == TYPTYPE_COMPOSITE && !isCompositeType)
+    {
+        if (ignoreDependent)
+        {
+            table_close(rel, RowExclusiveLock);
+            return InvalidOid;
+        }
+        if (errorOnTableType)
+            ereport(ERROR,
+                    (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                     errmsg("%s is a table's row type",
+                            format_type_be(typeOid)),
+            /* translator: %s is an SQL ALTER command */
+                     errhint("Use %s instead.", "ALTER TABLE")));
+    }

     if (oldNspOid != nspOid)
     {
@@ -4260,7 +4291,8 @@ AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,

     /* Recursively alter the associated array type, if any */
     if (OidIsValid(arrayOid))
-        AlterTypeNamespaceInternal(arrayOid, nspOid, true, true, objsMoved);
+        AlterTypeNamespaceInternal(arrayOid, nspOid, true, false, true,
+                                   objsMoved);

     return oldNspOid;
 }
diff --git a/src/include/commands/typecmds.h b/src/include/commands/typecmds.h
index c378f9cd4f..e1b02927c4 100644
--- a/src/include/commands/typecmds.h
+++ b/src/include/commands/typecmds.h
@@ -50,9 +50,11 @@ extern void AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId);

 extern ObjectAddress AlterTypeNamespace(List *names, const char *newschema,
                                         ObjectType objecttype, Oid *oldschema);
-extern Oid    AlterTypeNamespace_oid(Oid typeOid, Oid nspOid, ObjectAddresses *objsMoved);
+extern Oid    AlterTypeNamespace_oid(Oid typeOid, Oid nspOid, bool ignoreDependent,
+                                   ObjectAddresses *objsMoved);
 extern Oid    AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
                                        bool isImplicitArray,
+                                       bool ignoreDependent,
                                        bool errorOnTableType,
                                        ObjectAddresses *objsMoved);

diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index 7d95d6b924..05272e6a40 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -8,6 +8,7 @@ EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
             test_ext_cyclic1 test_ext_cyclic2 \
             test_ext_extschema \
             test_ext_evttrig \
+            test_ext_set_schema \
             test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3

 DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
@@ -19,6 +20,7 @@ DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
        test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql \
        test_ext_extschema--1.0.sql \
        test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
+       test_ext_set_schema--1.0.sql \
        test_ext_req_schema1--1.0.sql \
        test_ext_req_schema2--1.0.sql \
        test_ext_req_schema3--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 a7ab244e87..f357cc21aa 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -464,6 +464,44 @@ CREATE EXTENSION test_ext_extschema SCHEMA has$dollar;
 ERROR:  invalid character in extension "test_ext_extschema" schema: must not contain any of ""$'\"
 CREATE EXTENSION test_ext_extschema SCHEMA "has space";
 --
+-- Test basic SET SCHEMA handling.
+--
+CREATE SCHEMA s1;
+CREATE SCHEMA s2;
+CREATE EXTENSION test_ext_set_schema SCHEMA s1;
+ALTER EXTENSION test_ext_set_schema SET SCHEMA s2;
+\dx+ test_ext_set_schema
+      Objects in extension "test_ext_set_schema"
+                  Object description
+-------------------------------------------------------
+ cast from s2.ess_range_type to s2.ess_multirange_type
+ function s2.ess_func(integer)
+ function s2.ess_multirange_type()
+ function s2.ess_multirange_type(s2.ess_range_type)
+ function s2.ess_multirange_type(s2.ess_range_type[])
+ function s2.ess_range_type(text,text)
+ function s2.ess_range_type(text,text,text)
+ table s2.ess_table
+ type s2.ess_composite_type
+ type s2.ess_composite_type[]
+ type s2.ess_multirange_type
+ type s2.ess_multirange_type[]
+ type s2.ess_range_type
+ type s2.ess_range_type[]
+ type s2.ess_table
+ type s2.ess_table[]
+(16 rows)
+
+\sf s2.ess_func(int)
+CREATE OR REPLACE FUNCTION s2.ess_func(integer)
+ RETURNS text
+ LANGUAGE sql
+BEGIN ATOMIC
+ SELECT ess_table.f3
+    FROM s2.ess_table
+   WHERE (ess_table.f1 = $1);
+END
+--
 -- Test extension with objects outside the extension's schema.
 --
 CREATE SCHEMA test_func_dep1;
diff --git a/src/test/modules/test_extensions/meson.build b/src/test/modules/test_extensions/meson.build
index 9b8d0a1016..c5f3424da5 100644
--- a/src/test/modules/test_extensions/meson.build
+++ b/src/test/modules/test_extensions/meson.build
@@ -40,6 +40,8 @@ test_install_data += files(
   'test_ext_req_schema2.control',
   'test_ext_req_schema3--1.0.sql',
   'test_ext_req_schema3.control',
+  'test_ext_set_schema--1.0.sql',
+  'test_ext_set_schema.control',
 )

 tests += {
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql
b/src/test/modules/test_extensions/sql/test_extensions.sql
index c5b64f47c6..642c82ff5d 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -232,6 +232,16 @@ CREATE SCHEMA "has space";
 CREATE EXTENSION test_ext_extschema SCHEMA has$dollar;
 CREATE EXTENSION test_ext_extschema SCHEMA "has space";

+--
+-- Test basic SET SCHEMA handling.
+--
+CREATE SCHEMA s1;
+CREATE SCHEMA s2;
+CREATE EXTENSION test_ext_set_schema SCHEMA s1;
+ALTER EXTENSION test_ext_set_schema SET SCHEMA s2;
+\dx+ test_ext_set_schema
+\sf s2.ess_func(int)
+
 --
 -- Test extension with objects outside the extension's schema.
 --
diff --git a/src/test/modules/test_extensions/test_ext_set_schema--1.0.sql
b/src/test/modules/test_extensions/test_ext_set_schema--1.0.sql
new file mode 100644
index 0000000000..66df583ca9
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_set_schema--1.0.sql
@@ -0,0 +1,17 @@
+/* src/test/modules/test_extensions/test_ext_set_schema--1.0.sql */
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_ext_set_schema" to load this file. \quit
+
+-- Create various object types that need extra handling by SET SCHEMA.
+
+CREATE TABLE ess_table (f1 int primary key, f2 int, f3 text,
+                        constraint ess_c check (f1 != f2));
+
+CREATE FUNCTION ess_func(int) RETURNS text
+BEGIN ATOMIC
+  SELECT f3 FROM ess_table WHERE f1 = $1;
+END;
+
+CREATE TYPE ess_range_type AS RANGE (subtype = text);
+
+CREATE TYPE ess_composite_type AS (f1 int, f2 ess_range_type);
diff --git a/src/test/modules/test_extensions/test_ext_set_schema.control
b/src/test/modules/test_extensions/test_ext_set_schema.control
new file mode 100644
index 0000000000..a9a236763e
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_set_schema.control
@@ -0,0 +1,3 @@
+comment = 'Test ALTER EXTENSION SET SCHEMA'
+default_version = '1.0'
+relocatable = true