Skip to content

Commit bc4996e

Browse files
robertmhaasrobertmhaas
committed
Make ALTER .. SET SCHEMA do nothing, instead of throwing an ERROR.
This was already true for CREATE EXTENSION, but historically has not been true for other object types. Therefore, this is a backward incompatibility. Per discussion on pgsql-hackers, everyone seems to agree that the new behavior is better. Marti Raudsepp, reviewed by Haribabu Kommi and myself
1 parent f11c557 commit bc4996e

File tree

10 files changed

+61
-43
lines changed

10 files changed

+61
-43
lines changed

‎src/backend/catalog/namespace.c

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2769,24 +2769,13 @@ LookupCreationNamespace(const char *nspname)
27692769
/*
27702770
* Common checks on switching namespaces.
27712771
*
2772-
* We complain if (1) the old and new namespaces are the same, (2) either the
2773-
* old or new namespaces is a temporary schema (or temporary toast schema), or
2774-
* (3) either the old or new namespaces is the TOAST schema.
2772+
* We complain if (1) either the old or new namespaces is a temporary schema
2773+
* (or temporary toast schema), or (3) either the old or new namespaces is the
2774+
* TOAST schema.
27752775
*/
27762776
void
2777-
CheckSetNamespace(Oid oldNspOid, Oid nspOid, Oid classid, Oid objid)
2777+
CheckSetNamespace(Oid oldNspOid, Oid nspOid)
27782778
{
2779-
if (oldNspOid == nspOid)
2780-
ereport(ERROR,
2781-
(classid == RelationRelationId ?
2782-
errcode(ERRCODE_DUPLICATE_TABLE) :
2783-
classid == ProcedureRelationId ?
2784-
errcode(ERRCODE_DUPLICATE_FUNCTION) :
2785-
errcode(ERRCODE_DUPLICATE_OBJECT),
2786-
errmsg("%s is already in schema \"%s\"",
2787-
getObjectDescriptionOids(classid, objid),
2788-
get_namespace_name(nspOid))));
2789-
27902779
/* disallow renaming into or out of temp schemas */
27912780
if (isAnyTempNamespace(nspOid) || isAnyTempNamespace(oldNspOid))
27922781
ereport(ERROR,

‎src/backend/catalog/pg_constraint.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -726,7 +726,8 @@ AlterConstraintNamespaces(Oid ownerId, Oid oldNspId,
726726
if (object_address_present(&thisobj, objsMoved))
727727
continue;
728728

729-
if (conform->connamespace == oldNspId)
729+
/* Don't update if the object is already part of the namespace */
730+
if (conform->connamespace == oldNspId && oldNspId != newNspId)
730731
{
731732
tup = heap_copytuple(tup);
732733
conform = (Form_pg_constraint) GETSTRUCT(tup);

‎src/backend/commands/alter.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -592,8 +592,18 @@ AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid)
592592
Assert(!isnull);
593593
oldNspOid = DatumGetObjectId(namespace);
594594

595+
/*
596+
* If the object is already in the correct namespace, we don't need
597+
* to do anything except fire the object access hook.
598+
*/
599+
if (oldNspOid == nspOid)
600+
{
601+
InvokeObjectPostAlterHook(classId, objid, 0);
602+
return oldNspOid;
603+
}
604+
595605
/* Check basic namespace related issues */
596-
CheckSetNamespace(oldNspOid, nspOid, classId, objid);
606+
CheckSetNamespace(oldNspOid, nspOid);
597607

598608
/* Permission checks ... superusers can always do it */
599609
if (!superuser())

‎src/backend/commands/tablecmds.c

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11350,7 +11350,7 @@ AlterTableNamespace(AlterObjectSchemaStmt *stmt, Oid *oldschema)
1135011350
nspOid = RangeVarGetAndCheckCreationNamespace(newrv, NoLock, NULL);
1135111351

1135211352
/* common checks on switching namespaces */
11353-
CheckSetNamespace(oldNspOid, nspOid, RelationRelationId, relid);
11353+
CheckSetNamespace(oldNspOid, nspOid);
1135411354

1135511355
objsMoved = new_object_addresses();
1135611356
AlterTableNamespaceInternal(rel, oldNspOid, nspOid, objsMoved);
@@ -11418,6 +11418,7 @@ AlterRelationNamespaceInternal(Relation classRel, Oid relOid,
1141811418
HeapTuple classTup;
1141911419
Form_pg_class classForm;
1142011420
ObjectAddress thisobj;
11421+
bool already_done = false;
1142111422

1142211423
classTup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relOid));
1142311424
if (!HeapTupleIsValid(classTup))
@@ -11431,9 +11432,12 @@ AlterRelationNamespaceInternal(Relation classRel, Oid relOid,
1143111432
thisobj.objectSubId = 0;
1143211433

1143311434
/*
11434-
* Do nothing when there's nothing to do.
11435+
* If the object has already been moved, don't move it again. If it's
11436+
* already in the right place, don't move it, but still fire the object
11437+
* access hook.
1143511438
*/
11436-
if (!object_address_present(&thisobj, objsMoved))
11439+
already_done = object_address_present(&thisobj, objsMoved);
11440+
if (!already_done && oldNspOid != newNspOid)
1143711441
{
1143811442
/* check for duplicate name (more friendly than unique-index failure) */
1143911443
if (get_relname_relid(NameStr(classForm->relname),
@@ -11459,7 +11463,9 @@ AlterRelationNamespaceInternal(Relation classRel, Oid relOid,
1145911463
newNspOid) != 1)
1146011464
elog(ERROR, "failed to change schema dependency for relation \"%s\"",
1146111465
NameStr(classForm->relname));
11462-
11466+
}
11467+
if (!already_done)
11468+
{
1146311469
add_exact_object_address(&thisobj, objsMoved);
1146411470

1146511471
InvokeObjectPostAlterHook(RelationRelationId, relOid, 0);

‎src/backend/commands/typecmds.c

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3520,18 +3520,22 @@ AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
35203520
oldNspOid = typform->typnamespace;
35213521
arrayOid = typform->typarray;
35223522

3523-
/* common checks on switching namespaces */
3524-
CheckSetNamespace(oldNspOid, nspOid, TypeRelationId, typeOid);
3523+
/* If the type is already there, we scan skip these next few checks. */
3524+
if (oldNspOid != nspOid)
3525+
{
3526+
/* common checks on switching namespaces */
3527+
CheckSetNamespace(oldNspOid, nspOid);
35253528

3526-
/* check for duplicate name (more friendly than unique-index failure) */
3527-
if (SearchSysCacheExists2(TYPENAMENSP,
3528-
CStringGetDatum(NameStr(typform->typname)),
3529-
ObjectIdGetDatum(nspOid)))
3530-
ereport(ERROR,
3531-
(errcode(ERRCODE_DUPLICATE_OBJECT),
3532-
errmsg("type \"%s\" already exists in schema \"%s\"",
3533-
NameStr(typform->typname),
3534-
get_namespace_name(nspOid))));
3529+
/* check for duplicate name (more friendly than unique-index failure) */
3530+
if (SearchSysCacheExists2(TYPENAMENSP,
3531+
CStringGetDatum(NameStr(typform->typname)),
3532+
ObjectIdGetDatum(nspOid)))
3533+
ereport(ERROR,
3534+
(errcode(ERRCODE_DUPLICATE_OBJECT),
3535+
errmsg("type \"%s\" already exists in schema \"%s\"",
3536+
NameStr(typform->typname),
3537+
get_namespace_name(nspOid))));
3538+
}
35353539

35363540
/* Detect whether type is a composite type (but not a table rowtype) */
35373541
isCompositeType =
@@ -3547,13 +3551,16 @@ AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
35473551
format_type_be(typeOid)),
35483552
errhint("Use ALTER TABLE instead.")));
35493553

3550-
/* OK, modify the pg_type row */
3554+
if (oldNspOid != nspOid)
3555+
{
3556+
/* OK, modify the pg_type row */
35513557

3552-
/* tup is a copy, so we can scribble directly on it */
3553-
typform->typnamespace = nspOid;
3558+
/* tup is a copy, so we can scribble directly on it */
3559+
typform->typnamespace = nspOid;
35543560

3555-
simple_heap_update(rel, &tup->t_self, tup);
3556-
CatalogUpdateIndexes(rel, tup);
3561+
simple_heap_update(rel, &tup->t_self, tup);
3562+
CatalogUpdateIndexes(rel, tup);
3563+
}
35573564

35583565
/*
35593566
* Composite types have pg_class entries.
@@ -3592,7 +3599,8 @@ AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
35923599
* Update dependency on schema, if any --- a table rowtype has not got
35933600
* one, and neither does an implicit array.
35943601
*/
3595-
if ((isCompositeType || typform->typtype != TYPTYPE_COMPOSITE) &&
3602+
if (oldNspOid != nspOid &&
3603+
(isCompositeType || typform->typtype != TYPTYPE_COMPOSITE) &&
35963604
!isImplicitArray)
35973605
if (changeDependencyFor(TypeRelationId, typeOid,
35983606
NamespaceRelationId, oldNspOid, nspOid) != 1)

‎src/include/catalog/namespace.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,7 @@ extern Oid LookupExplicitNamespace(const char *nspname, bool missing_ok);
112112
extern Oid get_namespace_oid(const char *nspname, bool missing_ok);
113113

114114
extern Oid LookupCreationNamespace(const char *nspname);
115-
extern void CheckSetNamespace(Oid oldNspOid, Oid nspOid, Oid classid,
116-
Oid objid);
115+
extern void CheckSetNamespace(Oid oldNspOid, Oid nspOid);
117116
extern Oid QualifiedNameGetCreationNamespace(List *names, char **objname_p);
118117
extern RangeVar *makeRangeVarFromNameList(List *names);
119118
extern char *NameListToString(List *names);

‎src/test/regress/expected/alter_generic.out

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ ALTER FUNCTION alt_func1(int) RENAME TO alt_func3; -- OK
4040
ALTER FUNCTION alt_func2(int) OWNER TO regtest_alter_user2; -- failed (no role membership)
4141
ERROR: must be member of role "regtest_alter_user2"
4242
ALTER FUNCTION alt_func2(int) OWNER TO regtest_alter_user3; -- OK
43+
ALTER FUNCTION alt_func2(int) SET SCHEMA alt_nsp1; -- OK, already there
4344
ALTER FUNCTION alt_func2(int) SET SCHEMA alt_nsp2; -- OK
4445
ALTER AGGREGATE alt_agg1(int) RENAME TO alt_agg2; -- failed (name conflict)
4546
ERROR: function alt_agg2(integer) already exists in schema "alt_nsp1"

‎src/test/regress/expected/alter_table.out

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2133,6 +2133,7 @@ create text search template alter1.tmpl(init = dsimple_init, lexize = dsimple_le
21332133
create text search dictionary alter1.dict(template = alter1.tmpl);
21342134
insert into alter1.t1(f2) values(11);
21352135
insert into alter1.t1(f2) values(12);
2136+
alter table alter1.t1 set schema alter1; -- no-op, same schema
21362137
alter table alter1.t1 set schema alter2;
21372138
alter table alter1.v1 set schema alter2;
21382139
alter function alter1.plus1(int) set schema alter2;
@@ -2141,6 +2142,7 @@ alter operator class alter1.ctype_hash_ops using hash set schema alter2;
21412142
alter operator family alter1.ctype_hash_ops using hash set schema alter2;
21422143
alter operator alter1.=(alter1.ctype, alter1.ctype) set schema alter2;
21432144
alter function alter1.same(alter1.ctype, alter1.ctype) set schema alter2;
2145+
alter type alter1.ctype set schema alter1; -- no-op, same schema
21442146
alter type alter1.ctype set schema alter2;
21452147
alter conversion alter1.ascii_to_utf8 set schema alter2;
21462148
alter text search parser alter1.prs set schema alter2;
@@ -2567,9 +2569,8 @@ ALTER TABLE new_system_table SET SCHEMA pg_catalog;
25672569
-- XXX: it's currently impossible to move relations out of pg_catalog
25682570
ALTER TABLE new_system_table SET SCHEMA public;
25692571
ERROR: cannot remove dependency on schema pg_catalog because it is a system object
2570-
-- move back, will currently error out, already there
2572+
-- move back, will be ignored -- already there
25712573
ALTER TABLE new_system_table SET SCHEMA pg_catalog;
2572-
ERROR: table new_system_table is already in schema "pg_catalog"
25732574
ALTER TABLE new_system_table RENAME TO old_system_table;
25742575
CREATE INDEX old_system_table__othercol ON old_system_table (othercol);
25752576
INSERT INTO old_system_table(othercol) VALUES ('somedata'), ('otherdata');

‎src/test/regress/sql/alter_generic.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ ALTER FUNCTION alt_func1(int) RENAME TO alt_func2; -- failed (name conflict)
4444
ALTER FUNCTION alt_func1(int) RENAME TO alt_func3; -- OK
4545
ALTER FUNCTION alt_func2(int) OWNER TO regtest_alter_user2; -- failed (no role membership)
4646
ALTER FUNCTION alt_func2(int) OWNER TO regtest_alter_user3; -- OK
47+
ALTER FUNCTION alt_func2(int) SET SCHEMA alt_nsp1; -- OK, already there
4748
ALTER FUNCTION alt_func2(int) SET SCHEMA alt_nsp2; -- OK
4849

4950
ALTER AGGREGATE alt_agg1(int) RENAME TO alt_agg2; -- failed (name conflict)

‎src/test/regress/sql/alter_table.sql

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1465,6 +1465,7 @@ create text search dictionary alter1.dict(template = alter1.tmpl);
14651465
insert into alter1.t1(f2) values(11);
14661466
insert into alter1.t1(f2) values(12);
14671467

1468+
alter table alter1.t1 set schema alter1; -- no-op, same schema
14681469
alter table alter1.t1 set schema alter2;
14691470
alter table alter1.v1 set schema alter2;
14701471
alter function alter1.plus1(int) set schema alter2;
@@ -1473,6 +1474,7 @@ alter operator class alter1.ctype_hash_ops using hash set schema alter2;
14731474
alter operator family alter1.ctype_hash_ops using hash set schema alter2;
14741475
alter operator alter1.=(alter1.ctype, alter1.ctype) set schema alter2;
14751476
alter function alter1.same(alter1.ctype, alter1.ctype) set schema alter2;
1477+
alter type alter1.ctype set schema alter1; -- no-op, same schema
14761478
alter type alter1.ctype set schema alter2;
14771479
alter conversion alter1.ascii_to_utf8 set schema alter2;
14781480
alter text search parser alter1.prs set schema alter2;
@@ -1704,7 +1706,7 @@ ALTER TABLE new_system_table SET SCHEMA pg_catalog;
17041706

17051707
-- XXX: it's currently impossible to move relations out of pg_catalog
17061708
ALTER TABLE new_system_table SET SCHEMA public;
1707-
-- move back, will currently error out, already there
1709+
-- move back, will be ignored -- already there
17081710
ALTER TABLE new_system_table SET SCHEMA pg_catalog;
17091711
ALTER TABLE new_system_table RENAME TO old_system_table;
17101712
CREATE INDEX old_system_table__othercol ON old_system_table (othercol);

0 commit comments

Comments
 (0)