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
