Repair ALTER EXTENSION ... SET SCHEMA.
It turns out that we broke this in commit e5bc9454e
, because
the code was assuming that no dependent types would appear
among the extension's direct dependencies, and now they do.
This isn't terribly hard to fix: just skip dependent types,
expecting that we will recurse to them when we process the parent
object (which should also be among the direct dependencies).
But a little bit of refactoring is needed so that we can avoid
duplicating logic about what is a dependent type.
Although there is some testing of ALTER EXTENSION SET SCHEMA,
it failed to cover interesting cases, so add more tests.
Discussion: https://postgr.es/m/930191.1715205151@sss.pgh.pa.us
This commit is contained in:
parent
d82ab9fc31
commit
9effc4608e
|
@ -598,16 +598,16 @@ ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt,
|
||||||
/*
|
/*
|
||||||
* Change an object's namespace given its classOid and object Oid.
|
* 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,
|
* 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
|
* so it only needs to cover object kinds that can be members of an
|
||||||
* extension, and it doesn't have to deal with certain special cases
|
* extension, and it can silently ignore dependent types --- we assume
|
||||||
* such as not wanting to process array types --- those should never
|
* those will be moved when their parent object is moved.
|
||||||
* be direct members of an extension anyway.
|
|
||||||
*
|
*
|
||||||
* Returns the OID of the object's previous namespace, or InvalidOid if
|
* 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
|
Oid
|
||||||
AlterObjectNamespace_oid(Oid classId, Oid objid, Oid nspOid,
|
AlterObjectNamespace_oid(Oid classId, Oid objid, Oid nspOid,
|
||||||
|
@ -631,7 +631,7 @@ AlterObjectNamespace_oid(Oid classId, Oid objid, Oid nspOid,
|
||||||
}
|
}
|
||||||
|
|
||||||
case TypeRelationId:
|
case TypeRelationId:
|
||||||
oldNspOid = AlterTypeNamespace_oid(objid, nspOid, objsMoved);
|
oldNspOid = AlterTypeNamespace_oid(objid, nspOid, true, objsMoved);
|
||||||
break;
|
break;
|
||||||
|
|
||||||
case ProcedureRelationId:
|
case ProcedureRelationId:
|
||||||
|
|
|
@ -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
|
* 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)
|
if (dep_oldNspOid != InvalidOid && dep_oldNspOid != oldNspOid)
|
||||||
ereport(ERROR,
|
ereport(ERROR,
|
||||||
|
|
|
@ -18017,8 +18017,11 @@ AlterTableNamespaceInternal(Relation rel, Oid oldNspOid, Oid nspOid,
|
||||||
|
|
||||||
/* Fix the table's row type too, if it has one */
|
/* Fix the table's row type too, if it has one */
|
||||||
if (OidIsValid(rel->rd_rel->reltype))
|
if (OidIsValid(rel->rd_rel->reltype))
|
||||||
AlterTypeNamespaceInternal(rel->rd_rel->reltype,
|
AlterTypeNamespaceInternal(rel->rd_rel->reltype, nspOid,
|
||||||
nspOid, false, false, objsMoved);
|
false, /* isImplicitArray */
|
||||||
|
false, /* ignoreDependent */
|
||||||
|
false, /* errorOnTableType */
|
||||||
|
objsMoved);
|
||||||
|
|
||||||
/* Fix other dependent stuff */
|
/* Fix other dependent stuff */
|
||||||
AlterIndexNamespaces(classRel, rel, oldNspOid, nspOid, objsMoved);
|
AlterIndexNamespaces(classRel, rel, oldNspOid, nspOid, objsMoved);
|
||||||
|
|
|
@ -4068,7 +4068,7 @@ AlterTypeNamespace(List *names, const char *newschema, ObjectType objecttype,
|
||||||
typename = makeTypeNameFromNameList(names);
|
typename = makeTypeNameFromNameList(names);
|
||||||
typeOid = typenameTypeId(NULL, typename);
|
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)
|
if (objecttype == OBJECT_DOMAIN && get_typtype(typeOid) != TYPTYPE_DOMAIN)
|
||||||
ereport(ERROR,
|
ereport(ERROR,
|
||||||
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
|
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
|
||||||
|
@ -4079,7 +4079,7 @@ AlterTypeNamespace(List *names, const char *newschema, ObjectType objecttype,
|
||||||
nspOid = LookupCreationNamespace(newschema);
|
nspOid = LookupCreationNamespace(newschema);
|
||||||
|
|
||||||
objsMoved = new_object_addresses();
|
objsMoved = new_object_addresses();
|
||||||
oldNspOid = AlterTypeNamespace_oid(typeOid, nspOid, objsMoved);
|
oldNspOid = AlterTypeNamespace_oid(typeOid, nspOid, false, objsMoved);
|
||||||
free_object_addresses(objsMoved);
|
free_object_addresses(objsMoved);
|
||||||
|
|
||||||
if (oldschema)
|
if (oldschema)
|
||||||
|
@ -4090,8 +4090,21 @@ AlterTypeNamespace(List *names, const char *newschema, ObjectType objecttype,
|
||||||
return myself;
|
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
|
Oid
|
||||||
AlterTypeNamespace_oid(Oid typeOid, Oid nspOid, ObjectAddresses *objsMoved)
|
AlterTypeNamespace_oid(Oid typeOid, Oid nspOid, bool ignoreDependent,
|
||||||
|
ObjectAddresses *objsMoved)
|
||||||
{
|
{
|
||||||
Oid elemOid;
|
Oid elemOid;
|
||||||
|
|
||||||
|
@ -4102,15 +4115,23 @@ AlterTypeNamespace_oid(Oid typeOid, Oid nspOid, ObjectAddresses *objsMoved)
|
||||||
/* don't allow direct alteration of array types */
|
/* don't allow direct alteration of array types */
|
||||||
elemOid = get_element_type(typeOid);
|
elemOid = get_element_type(typeOid);
|
||||||
if (OidIsValid(elemOid) && get_array_type(elemOid) == typeOid)
|
if (OidIsValid(elemOid) && get_array_type(elemOid) == typeOid)
|
||||||
|
{
|
||||||
|
if (ignoreDependent)
|
||||||
|
return InvalidOid;
|
||||||
ereport(ERROR,
|
ereport(ERROR,
|
||||||
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
|
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
|
||||||
errmsg("cannot alter array type %s",
|
errmsg("cannot alter array type %s",
|
||||||
format_type_be(typeOid)),
|
format_type_be(typeOid)),
|
||||||
errhint("You can alter type %s, which will alter the array type as well.",
|
errhint("You can alter type %s, which will alter the array type as well.",
|
||||||
format_type_be(elemOid))));
|
format_type_be(elemOid))));
|
||||||
|
}
|
||||||
|
|
||||||
/* and do the work */
|
/* and do the work */
|
||||||
return AlterTypeNamespaceInternal(typeOid, nspOid, false, true, objsMoved);
|
return AlterTypeNamespaceInternal(typeOid, nspOid,
|
||||||
|
false, /* isImplicitArray */
|
||||||
|
ignoreDependent, /* ignoreDependent */
|
||||||
|
true, /* errorOnTableType */
|
||||||
|
objsMoved);
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
@ -4122,15 +4143,21 @@ AlterTypeNamespace_oid(Oid typeOid, Oid nspOid, ObjectAddresses *objsMoved)
|
||||||
* if any. isImplicitArray should be true only when doing this internal
|
* if any. isImplicitArray should be true only when doing this internal
|
||||||
* recursion (outside callers must never try to move an array type directly).
|
* 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
|
* 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
|
* 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.)
|
||||||
*
|
*
|
||||||
* Returns the type's old namespace OID.
|
* 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, or InvalidOid if we did nothing.
|
||||||
*/
|
*/
|
||||||
Oid
|
Oid
|
||||||
AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
|
AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
|
||||||
bool isImplicitArray,
|
bool isImplicitArray,
|
||||||
|
bool ignoreDependent,
|
||||||
bool errorOnTableType,
|
bool errorOnTableType,
|
||||||
ObjectAddresses *objsMoved)
|
ObjectAddresses *objsMoved)
|
||||||
{
|
{
|
||||||
|
@ -4185,15 +4212,21 @@ AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
|
||||||
get_rel_relkind(typform->typrelid) == RELKIND_COMPOSITE_TYPE);
|
get_rel_relkind(typform->typrelid) == RELKIND_COMPOSITE_TYPE);
|
||||||
|
|
||||||
/* Enforce not-table-type if requested */
|
/* Enforce not-table-type if requested */
|
||||||
if (typform->typtype == TYPTYPE_COMPOSITE && !isCompositeType &&
|
if (typform->typtype == TYPTYPE_COMPOSITE && !isCompositeType)
|
||||||
errorOnTableType)
|
{
|
||||||
|
if (ignoreDependent)
|
||||||
|
{
|
||||||
|
table_close(rel, RowExclusiveLock);
|
||||||
|
return InvalidOid;
|
||||||
|
}
|
||||||
|
if (errorOnTableType)
|
||||||
ereport(ERROR,
|
ereport(ERROR,
|
||||||
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
|
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
|
||||||
errmsg("%s is a table's row type",
|
errmsg("%s is a table's row type",
|
||||||
format_type_be(typeOid)),
|
format_type_be(typeOid)),
|
||||||
/* translator: %s is an SQL ALTER command */
|
/* translator: %s is an SQL ALTER command */
|
||||||
errhint("Use %s instead.",
|
errhint("Use %s instead.", "ALTER TABLE")));
|
||||||
"ALTER TABLE")));
|
}
|
||||||
|
|
||||||
if (oldNspOid != nspOid)
|
if (oldNspOid != nspOid)
|
||||||
{
|
{
|
||||||
|
@ -4260,7 +4293,11 @@ AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
|
||||||
|
|
||||||
/* Recursively alter the associated array type, if any */
|
/* Recursively alter the associated array type, if any */
|
||||||
if (OidIsValid(arrayOid))
|
if (OidIsValid(arrayOid))
|
||||||
AlterTypeNamespaceInternal(arrayOid, nspOid, true, true, objsMoved);
|
AlterTypeNamespaceInternal(arrayOid, nspOid,
|
||||||
|
true, /* isImplicitArray */
|
||||||
|
false, /* ignoreDependent */
|
||||||
|
true, /* errorOnTableType */
|
||||||
|
objsMoved);
|
||||||
|
|
||||||
return oldNspOid;
|
return oldNspOid;
|
||||||
}
|
}
|
||||||
|
|
|
@ -50,9 +50,11 @@ extern void AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId);
|
||||||
|
|
||||||
extern ObjectAddress AlterTypeNamespace(List *names, const char *newschema,
|
extern ObjectAddress AlterTypeNamespace(List *names, const char *newschema,
|
||||||
ObjectType objecttype, Oid *oldschema);
|
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,
|
extern Oid AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
|
||||||
bool isImplicitArray,
|
bool isImplicitArray,
|
||||||
|
bool ignoreDependent,
|
||||||
bool errorOnTableType,
|
bool errorOnTableType,
|
||||||
ObjectAddresses *objsMoved);
|
ObjectAddresses *objsMoved);
|
||||||
|
|
||||||
|
|
|
@ -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_cyclic1 test_ext_cyclic2 \
|
||||||
test_ext_extschema \
|
test_ext_extschema \
|
||||||
test_ext_evttrig \
|
test_ext_evttrig \
|
||||||
|
test_ext_set_schema \
|
||||||
test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3
|
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 \
|
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_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql \
|
||||||
test_ext_extschema--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_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_schema1--1.0.sql \
|
||||||
test_ext_req_schema2--1.0.sql \
|
test_ext_req_schema2--1.0.sql \
|
||||||
test_ext_req_schema3--1.0.sql
|
test_ext_req_schema3--1.0.sql
|
||||||
|
|
|
@ -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 ""$'\"
|
ERROR: invalid character in extension "test_ext_extschema" schema: must not contain any of ""$'\"
|
||||||
CREATE EXTENSION test_ext_extschema SCHEMA "has space";
|
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.
|
-- Test extension with objects outside the extension's schema.
|
||||||
--
|
--
|
||||||
CREATE SCHEMA test_func_dep1;
|
CREATE SCHEMA test_func_dep1;
|
||||||
|
|
|
@ -40,6 +40,8 @@ test_install_data += files(
|
||||||
'test_ext_req_schema2.control',
|
'test_ext_req_schema2.control',
|
||||||
'test_ext_req_schema3--1.0.sql',
|
'test_ext_req_schema3--1.0.sql',
|
||||||
'test_ext_req_schema3.control',
|
'test_ext_req_schema3.control',
|
||||||
|
'test_ext_set_schema--1.0.sql',
|
||||||
|
'test_ext_set_schema.control',
|
||||||
)
|
)
|
||||||
|
|
||||||
tests += {
|
tests += {
|
||||||
|
|
|
@ -232,6 +232,16 @@ CREATE SCHEMA "has space";
|
||||||
CREATE EXTENSION test_ext_extschema SCHEMA has$dollar;
|
CREATE EXTENSION test_ext_extschema SCHEMA has$dollar;
|
||||||
CREATE EXTENSION test_ext_extschema SCHEMA "has space";
|
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.
|
-- Test extension with objects outside the extension's schema.
|
||||||
--
|
--
|
||||||
|
|
|
@ -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);
|
|
@ -0,0 +1,3 @@
|
||||||
|
comment = 'Test ALTER EXTENSION SET SCHEMA'
|
||||||
|
default_version = '1.0'
|
||||||
|
relocatable = true
|
Loading…
Reference in New Issue