Skip to content

Commit 9effc46

Browse files
tglsfdctglsfdc
committed
Repair ALTER EXTENSION ... SET SCHEMA.
It turns out that we broke this in commit e5bc945, 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
1 parent d82ab9f commit 9effc46

File tree

11 files changed

+141
-27
lines changed

11 files changed

+141
-27
lines changed

‎src/backend/commands/alter.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -598,16 +598,16 @@ ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt,
598598
/*
599599
* Change an object's namespace given its classOid and object Oid.
600600
*
601-
* Objects that don't have a namespace should be ignored.
601+
* Objects that don't have a namespace should be ignored, as should
602+
* dependent types such as array types.
602603
*
603604
* This function is currently used only by ALTER EXTENSION SET SCHEMA,
604-
* so it only needs to cover object types that can be members of an
605-
* extension, and it doesn't have to deal with certain special cases
606-
* such as not wanting to process array types --- those should never
607-
* be direct members of an extension anyway.
605+
* so it only needs to cover object kinds that can be members of an
606+
* extension, and it can silently ignore dependent types --- we assume
607+
* those will be moved when their parent object is moved.
608608
*
609609
* Returns the OID of the object's previous namespace, or InvalidOid if
610-
* object doesn't have a schema.
610+
* object doesn't have a schema or was ignored due to being a dependent type.
611611
*/
612612
Oid
613613
AlterObjectNamespace_oid(Oid classId, Oid objid, Oid nspOid,
@@ -631,7 +631,7 @@ AlterObjectNamespace_oid(Oid classId, Oid objid, Oid nspOid,
631631
}
632632

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

637637
case ProcedureRelationId:

‎src/backend/commands/extension.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2940,7 +2940,7 @@ AlterExtensionNamespace(const char *extensionName, const char *newschema, Oid *o
29402940

29412941
/*
29422942
* If not all the objects had the same old namespace (ignoring any
2943-
* that are not in namespaces), complain.
2943+
* that are not in namespaces or are dependent types), complain.
29442944
*/
29452945
if (dep_oldNspOid != InvalidOid && dep_oldNspOid != oldNspOid)
29462946
ereport(ERROR,

‎src/backend/commands/tablecmds.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18017,8 +18017,11 @@ AlterTableNamespaceInternal(Relation rel, Oid oldNspOid, Oid nspOid,
1801718017

1801818018
/* Fix the table's row type too, if it has one */
1801918019
if (OidIsValid(rel->rd_rel->reltype))
18020-
AlterTypeNamespaceInternal(rel->rd_rel->reltype,
18021-
nspOid, false, false, objsMoved);
18020+
AlterTypeNamespaceInternal(rel->rd_rel->reltype, nspOid,
18021+
false, /* isImplicitArray */
18022+
false, /* ignoreDependent */
18023+
false, /* errorOnTableType */
18024+
objsMoved);
1802218025

1802318026
/* Fix other dependent stuff */
1802418027
AlterIndexNamespaces(classRel, rel, oldNspOid, nspOid, objsMoved);

‎src/backend/commands/typecmds.c

Lines changed: 53 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4068,7 +4068,7 @@ AlterTypeNamespace(List *names, const char *newschema, ObjectType objecttype,
40684068
typename = makeTypeNameFromNameList(names);
40694069
typeOid = typenameTypeId(NULL, typename);
40704070

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

40814081
objsMoved = new_object_addresses();
4082-
oldNspOid = AlterTypeNamespace_oid(typeOid, nspOid, objsMoved);
4082+
oldNspOid = AlterTypeNamespace_oid(typeOid, nspOid, false, objsMoved);
40834083
free_object_addresses(objsMoved);
40844084

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

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

@@ -4102,15 +4115,23 @@ AlterTypeNamespace_oid(Oid typeOid, Oid nspOid, ObjectAddresses *objsMoved)
41024115
/* don't allow direct alteration of array types */
41034116
elemOid = get_element_type(typeOid);
41044117
if (OidIsValid(elemOid) && get_array_type(elemOid) == typeOid)
4118+
{
4119+
if (ignoreDependent)
4120+
return InvalidOid;
41054121
ereport(ERROR,
41064122
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
41074123
errmsg("cannot alter array type %s",
41084124
format_type_be(typeOid)),
41094125
errhint("You can alter type %s, which will alter the array type as well.",
41104126
format_type_be(elemOid))));
4127+
}
41114128

41124129
/* and do the work */
4113-
return AlterTypeNamespaceInternal(typeOid, nspOid, false, true, objsMoved);
4130+
return AlterTypeNamespaceInternal(typeOid, nspOid,
4131+
false, /* isImplicitArray */
4132+
ignoreDependent, /* ignoreDependent */
4133+
true, /* errorOnTableType */
4134+
objsMoved);
41144135
}
41154136

41164137
/*
@@ -4122,15 +4143,21 @@ AlterTypeNamespace_oid(Oid typeOid, Oid nspOid, ObjectAddresses *objsMoved)
41224143
* if any. isImplicitArray should be true only when doing this internal
41234144
* recursion (outside callers must never try to move an array type directly).
41244145
*
4146+
* If ignoreDependent is true, we silently don't process table types.
4147+
*
41254148
* If errorOnTableType is true, the function errors out if the type is
41264149
* a table type. ALTER TABLE has to be used to move a table to a new
4127-
* namespace.
4150+
* namespace. (This flag is ignored if ignoreDependent is true.)
4151+
*
4152+
* We also do nothing if the type is already listed in *objsMoved.
4153+
* After a successful move, we add the type to *objsMoved.
41284154
*
4129-
* Returns the type's old namespace OID.
4155+
* Returns the type's old namespace OID, or InvalidOid if we did nothing.
41304156
*/
41314157
Oid
41324158
AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
41334159
bool isImplicitArray,
4160+
bool ignoreDependent,
41344161
bool errorOnTableType,
41354162
ObjectAddresses *objsMoved)
41364163
{
@@ -4185,15 +4212,21 @@ AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
41854212
get_rel_relkind(typform->typrelid) == RELKIND_COMPOSITE_TYPE);
41864213

41874214
/* Enforce not-table-type if requested */
4188-
if (typform->typtype == TYPTYPE_COMPOSITE && !isCompositeType &&
4189-
errorOnTableType)
4190-
ereport(ERROR,
4191-
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
4192-
errmsg("%s is a table's row type",
4193-
format_type_be(typeOid)),
4194-
/* translator: %s is an SQL ALTER command */
4195-
errhint("Use %s instead.",
4196-
"ALTER TABLE")));
4215+
if (typform->typtype == TYPTYPE_COMPOSITE && !isCompositeType)
4216+
{
4217+
if (ignoreDependent)
4218+
{
4219+
table_close(rel, RowExclusiveLock);
4220+
return InvalidOid;
4221+
}
4222+
if (errorOnTableType)
4223+
ereport(ERROR,
4224+
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
4225+
errmsg("%s is a table's row type",
4226+
format_type_be(typeOid)),
4227+
/* translator: %s is an SQL ALTER command */
4228+
errhint("Use %s instead.", "ALTER TABLE")));
4229+
}
41974230

41984231
if (oldNspOid != nspOid)
41994232
{
@@ -4260,7 +4293,11 @@ AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
42604293

42614294
/* Recursively alter the associated array type, if any */
42624295
if (OidIsValid(arrayOid))
4263-
AlterTypeNamespaceInternal(arrayOid, nspOid, true, true, objsMoved);
4296+
AlterTypeNamespaceInternal(arrayOid, nspOid,
4297+
true, /* isImplicitArray */
4298+
false, /* ignoreDependent */
4299+
true, /* errorOnTableType */
4300+
objsMoved);
42644301

42654302
return oldNspOid;
42664303
}

‎src/include/commands/typecmds.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,11 @@ extern void AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId);
5050

5151
extern ObjectAddress AlterTypeNamespace(List *names, const char *newschema,
5252
ObjectType objecttype, Oid *oldschema);
53-
extern Oid AlterTypeNamespace_oid(Oid typeOid, Oid nspOid, ObjectAddresses *objsMoved);
53+
extern Oid AlterTypeNamespace_oid(Oid typeOid, Oid nspOid, bool ignoreDependent,
54+
ObjectAddresses *objsMoved);
5455
extern Oid AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
5556
bool isImplicitArray,
57+
bool ignoreDependent,
5658
bool errorOnTableType,
5759
ObjectAddresses *objsMoved);
5860

‎src/test/modules/test_extensions/Makefile

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
88
test_ext_cyclic1 test_ext_cyclic2 \
99
test_ext_extschema \
1010
test_ext_evttrig \
11+
test_ext_set_schema \
1112
test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3
1213

1314
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 \
1920
test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql \
2021
test_ext_extschema--1.0.sql \
2122
test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql \
23+
test_ext_set_schema--1.0.sql \
2224
test_ext_req_schema1--1.0.sql \
2325
test_ext_req_schema2--1.0.sql \
2426
test_ext_req_schema3--1.0.sql

‎src/test/modules/test_extensions/expected/test_extensions.out

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,44 @@ CREATE EXTENSION test_ext_extschema SCHEMA has$dollar;
464464
ERROR: invalid character in extension "test_ext_extschema" schema: must not contain any of ""$'\"
465465
CREATE EXTENSION test_ext_extschema SCHEMA "has space";
466466
--
467+
-- Test basic SET SCHEMA handling.
468+
--
469+
CREATE SCHEMA s1;
470+
CREATE SCHEMA s2;
471+
CREATE EXTENSION test_ext_set_schema SCHEMA s1;
472+
ALTER EXTENSION test_ext_set_schema SET SCHEMA s2;
473+
\dx+ test_ext_set_schema
474+
Objects in extension "test_ext_set_schema"
475+
Object description
476+
-------------------------------------------------------
477+
cast from s2.ess_range_type to s2.ess_multirange_type
478+
function s2.ess_func(integer)
479+
function s2.ess_multirange_type()
480+
function s2.ess_multirange_type(s2.ess_range_type)
481+
function s2.ess_multirange_type(s2.ess_range_type[])
482+
function s2.ess_range_type(text,text)
483+
function s2.ess_range_type(text,text,text)
484+
table s2.ess_table
485+
type s2.ess_composite_type
486+
type s2.ess_composite_type[]
487+
type s2.ess_multirange_type
488+
type s2.ess_multirange_type[]
489+
type s2.ess_range_type
490+
type s2.ess_range_type[]
491+
type s2.ess_table
492+
type s2.ess_table[]
493+
(16 rows)
494+
495+
\sf s2.ess_func(int)
496+
CREATE OR REPLACE FUNCTION s2.ess_func(integer)
497+
RETURNS text
498+
LANGUAGE sql
499+
BEGIN ATOMIC
500+
SELECT ess_table.f3
501+
FROM s2.ess_table
502+
WHERE (ess_table.f1 = $1);
503+
END
504+
--
467505
-- Test extension with objects outside the extension's schema.
468506
--
469507
CREATE SCHEMA test_func_dep1;

‎src/test/modules/test_extensions/meson.build

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ test_install_data += files(
4040
'test_ext_req_schema2.control',
4141
'test_ext_req_schema3--1.0.sql',
4242
'test_ext_req_schema3.control',
43+
'test_ext_set_schema--1.0.sql',
44+
'test_ext_set_schema.control',
4345
)
4446

4547
tests += {

‎src/test/modules/test_extensions/sql/test_extensions.sql

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,16 @@ CREATE SCHEMA "has space";
232232
CREATE EXTENSION test_ext_extschema SCHEMA has$dollar;
233233
CREATE EXTENSION test_ext_extschema SCHEMA "has space";
234234

235+
--
236+
-- Test basic SET SCHEMA handling.
237+
--
238+
CREATE SCHEMA s1;
239+
CREATE SCHEMA s2;
240+
CREATE EXTENSION test_ext_set_schema SCHEMA s1;
241+
ALTER EXTENSION test_ext_set_schema SET SCHEMA s2;
242+
\dx+ test_ext_set_schema
243+
\sf s2.ess_func(int)
244+
235245
--
236246
-- Test extension with objects outside the extension's schema.
237247
--

‎src/test/modules/test_extensions/test_ext_set_schema--1.0.sql

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/* src/test/modules/test_extensions/test_ext_set_schema--1.0.sql */
2+
-- complain if script is sourced in psql, rather than via CREATE EXTENSION
3+
\echo Use "CREATE EXTENSION test_ext_set_schema" to load this file. \quit
4+
5+
-- Create various object types that need extra handling by SET SCHEMA.
6+
7+
CREATE TABLE ess_table (f1 int primary key, f2 int, f3 text,
8+
constraint ess_c check (f1 != f2));
9+
10+
CREATE FUNCTION ess_func(int) RETURNS text
11+
BEGIN ATOMIC
12+
SELECT f3 FROM ess_table WHERE f1 = $1;
13+
END;
14+
15+
CREATE TYPE ess_range_type AS RANGE (subtype = text);
16+
17+
CREATE TYPE ess_composite_type AS (f1 int, f2 ess_range_type);

‎src/test/modules/test_extensions/test_ext_set_schema.control

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
comment = 'Test ALTER EXTENSION SET SCHEMA'
2+
default_version = '1.0'
3+
relocatable = true

0 commit comments

Comments
 (0)