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

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index b16af64bb3d9d71221ca17b69b0ce0ccc19c14c5..8cf8b1764e1f1e4823785b834b0e45a46948ffd7 100644 (file)
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -2769,24 +2769,13 @@ LookupCreationNamespace(const char *nspname)
 /*
  * Common checks on switching namespaces.
  *
- * We complain if (1) the old and new namespaces are the same, (2) either the
- * old or new namespaces is a temporary schema (or temporary toast schema), or
- * (3) either the old or new namespaces is the TOAST schema.
+ * We complain if (1) either the old or new namespaces is a temporary schema
+ * (or temporary toast schema), or (3) either the old or new namespaces is the
+ * TOAST schema.
  */
 void
-CheckSetNamespace(Oid oldNspOid, Oid nspOid, Oid classid, Oid objid)
+CheckSetNamespace(Oid oldNspOid, Oid nspOid)
 {
-       if (oldNspOid == nspOid)
-               ereport(ERROR,
-                               (classid == RelationRelationId ?
-                                errcode(ERRCODE_DUPLICATE_TABLE) :
-                                classid == ProcedureRelationId ?
-                                errcode(ERRCODE_DUPLICATE_FUNCTION) :
-                                errcode(ERRCODE_DUPLICATE_OBJECT),
-                                errmsg("%s is already in schema \"%s\"",
-                                               getObjectDescriptionOids(classid, objid),
-                                               get_namespace_name(nspOid))));
-
        /* disallow renaming into or out of temp schemas */
        if (isAnyTempNamespace(nspOid) || isAnyTempNamespace(oldNspOid))
                ereport(ERROR,

diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 3c756f82278abea7eaf94820ef2486e3bcf80d37..a2c15a710fa5b08c9ce3ab4e7ac0b6fd9286f0d1 100644 (file)
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -726,7 +726,8 @@ AlterConstraintNamespaces(Oid ownerId, Oid oldNspId,
                if (object_address_present(&thisobj, objsMoved))
                        continue;
 
-               if (conform->connamespace == oldNspId)
+               /* Don't update if the object is already part of the namespace */
+               if (conform->connamespace == oldNspId && oldNspId != newNspId)
                {
                        tup = heap_copytuple(tup);
                        conform = (Form_pg_constraint) GETSTRUCT(tup);

diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
index d28758cf8b26176a71a34b2ad95755bba8875021..535741e92367871efe9ccf56e7a0381a66c2202b 100644 (file)
--- a/src/backend/commands/alter.c
+++ b/src/backend/commands/alter.c
@@ -592,8 +592,18 @@ AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid)
        Assert(!isnull);
        oldNspOid = DatumGetObjectId(namespace);
 
+       /*
+        * If the object is already in the correct namespace, we don't need
+        * to do anything except fire the object access hook.
+        */
+       if (oldNspOid == nspOid)
+       {
+               InvokeObjectPostAlterHook(classId, objid, 0);
+               return oldNspOid;
+       }
+
        /* Check basic namespace related issues */
-       CheckSetNamespace(oldNspOid, nspOid, classId, objid);
+       CheckSetNamespace(oldNspOid, nspOid);
 
        /* Permission checks ... superusers can always do it */
        if (!superuser())

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 44ea7311639a6c7823b8fc31f192666f3f56cbf1..b5d3708a6c127ff4a43e60789b738ac50e41bd2a 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -11350,7 +11350,7 @@ AlterTableNamespace(AlterObjectSchemaStmt *stmt, Oid *oldschema)
        nspOid = RangeVarGetAndCheckCreationNamespace(newrv, NoLock, NULL);
 
        /* common checks on switching namespaces */
-       CheckSetNamespace(oldNspOid, nspOid, RelationRelationId, relid);
+       CheckSetNamespace(oldNspOid, nspOid);
 
        objsMoved = new_object_addresses();
        AlterTableNamespaceInternal(rel, oldNspOid, nspOid, objsMoved);
@@ -11418,6 +11418,7 @@ AlterRelationNamespaceInternal(Relation classRel, Oid relOid,
        HeapTuple       classTup;
        Form_pg_class classForm;
        ObjectAddress thisobj;
+       bool            already_done = false;
 
        classTup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relOid));
        if (!HeapTupleIsValid(classTup))
@@ -11431,9 +11432,12 @@ AlterRelationNamespaceInternal(Relation classRel, Oid relOid,
        thisobj.objectSubId = 0;
 
        /*
-        * Do nothing when there's nothing to do.
+        * If the object has already been moved, don't move it again.  If it's
+        * already in the right place, don't move it, but still fire the object
+        * access hook.
         */
-       if (!object_address_present(&thisobj, objsMoved))
+       already_done = object_address_present(&thisobj, objsMoved);
+       if (!already_done && oldNspOid != newNspOid)
        {
                /* check for duplicate name (more friendly than unique-index failure) */
                if (get_relname_relid(NameStr(classForm->relname),
@@ -11459,7 +11463,9 @@ AlterRelationNamespaceInternal(Relation classRel, Oid relOid,
                                                                newNspOid) != 1)
                        elog(ERROR, "failed to change schema dependency for relation \"%s\"",
                                 NameStr(classForm->relname));
-
+       }
+       if (!already_done)
+       {
                add_exact_object_address(&thisobj, objsMoved);
 
                InvokeObjectPostAlterHook(RelationRelationId, relOid, 0);

diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index d2b3f2297b277dfec62b199484df2800bb0fec41..a126e666d8853ca1426f9815ccd2ca87b3b4b360 100644 (file)
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -3520,18 +3520,22 @@ AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
        oldNspOid = typform->typnamespace;
        arrayOid = typform->typarray;
 
-       /* common checks on switching namespaces */
-       CheckSetNamespace(oldNspOid, nspOid, TypeRelationId, typeOid);
+       /* If the type is already there, we scan skip these next few checks. */
+       if (oldNspOid != nspOid)
+       {
+               /* common checks on switching namespaces */
+               CheckSetNamespace(oldNspOid, nspOid);
 
-       /* check for duplicate name (more friendly than unique-index failure) */
-       if (SearchSysCacheExists2(TYPENAMENSP,
-                                                         CStringGetDatum(NameStr(typform->typname)),
-                                                         ObjectIdGetDatum(nspOid)))
-               ereport(ERROR,
-                               (errcode(ERRCODE_DUPLICATE_OBJECT),
-                                errmsg("type \"%s\" already exists in schema \"%s\"",
-                                               NameStr(typform->typname),
-                                               get_namespace_name(nspOid))));
+               /* check for duplicate name (more friendly than unique-index failure) */
+               if (SearchSysCacheExists2(TYPENAMENSP,
+                                                                 CStringGetDatum(NameStr(typform->typname)),
+                                                                 ObjectIdGetDatum(nspOid)))
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_DUPLICATE_OBJECT),
+                                        errmsg("type \"%s\" already exists in schema \"%s\"",
+                                                       NameStr(typform->typname),
+                                                       get_namespace_name(nspOid))));
+       }
 
        /* Detect whether type is a composite type (but not a table rowtype) */
        isCompositeType =
@@ -3547,13 +3551,16 @@ AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
                                                format_type_be(typeOid)),
                                 errhint("Use ALTER TABLE instead.")));
 
-       /* OK, modify the pg_type row */
+       if (oldNspOid != nspOid)
+       {
+               /* OK, modify the pg_type row */
 
-       /* tup is a copy, so we can scribble directly on it */
-       typform->typnamespace = nspOid;
+               /* tup is a copy, so we can scribble directly on it */
+               typform->typnamespace = nspOid;
 
-       simple_heap_update(rel, &tup->t_self, tup);
-       CatalogUpdateIndexes(rel, tup);
+               simple_heap_update(rel, &tup->t_self, tup);
+               CatalogUpdateIndexes(rel, tup);
+       }
 
        /*
         * Composite types have pg_class entries.
@@ -3592,7 +3599,8 @@ AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
         * Update dependency on schema, if any --- a table rowtype has not got
         * one, and neither does an implicit array.
         */
-       if ((isCompositeType || typform->typtype != TYPTYPE_COMPOSITE) &&
+       if (oldNspOid != nspOid &&
+               (isCompositeType || typform->typtype != TYPTYPE_COMPOSITE) &&
                !isImplicitArray)
                if (changeDependencyFor(TypeRelationId, typeOid,
                                                                NamespaceRelationId, oldNspOid, nspOid) != 1)

diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h
index f3b005fa9d80983551658168bc51230c1f439b8e..b6ad93406fd77bb11274122a803f16db19e7ae41 100644 (file)
--- a/src/include/catalog/namespace.h
+++ b/src/include/catalog/namespace.h
@@ -112,8 +112,7 @@ extern Oid  LookupExplicitNamespace(const char *nspname, bool missing_ok);
 extern Oid     get_namespace_oid(const char *nspname, bool missing_ok);
 
 extern Oid     LookupCreationNamespace(const char *nspname);
-extern void CheckSetNamespace(Oid oldNspOid, Oid nspOid, Oid classid,
-                                 Oid objid);
+extern void CheckSetNamespace(Oid oldNspOid, Oid nspOid);
 extern Oid     QualifiedNameGetCreationNamespace(List *names, char **objname_p);
 extern RangeVar *makeRangeVarFromNameList(List *names);
 extern char *NameListToString(List *names);

diff --git a/src/test/regress/expected/alter_generic.out b/src/test/regress/expected/alter_generic.out
index 4c3c8826b755c01299d622ae607236f8ccc8e225..43376eeafdd6ae8f56442d14c261282ccef75568 100644 (file)
--- a/src/test/regress/expected/alter_generic.out
+++ b/src/test/regress/expected/alter_generic.out
@@ -40,6 +40,7 @@ ALTER FUNCTION alt_func1(int) RENAME TO alt_func3;  -- OK
 ALTER FUNCTION alt_func2(int) OWNER TO regtest_alter_user2;  -- failed (no role membership)
 ERROR:  must be member of role "regtest_alter_user2"
 ALTER FUNCTION alt_func2(int) OWNER TO regtest_alter_user3;  -- OK
+ALTER FUNCTION alt_func2(int) SET SCHEMA alt_nsp1;  -- OK, already there
 ALTER FUNCTION alt_func2(int) SET SCHEMA alt_nsp2;  -- OK
 ALTER AGGREGATE alt_agg1(int) RENAME TO alt_agg2;   -- failed (name conflict)
 ERROR:  function alt_agg2(integer) already exists in schema "alt_nsp1"

diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 50b3c456f149004bd36d7a3bc08eadd946b59270..d2b6f2caceddf1a1cf191ec27fdeae44b7688880 100644 (file)
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2133,6 +2133,7 @@ create text search template alter1.tmpl(init = dsimple_init, lexize = dsimple_le
 create text search dictionary alter1.dict(template = alter1.tmpl);
 insert into alter1.t1(f2) values(11);
 insert into alter1.t1(f2) values(12);
+alter table alter1.t1 set schema alter1; -- no-op, same schema
 alter table alter1.t1 set schema alter2;
 alter table alter1.v1 set schema alter2;
 alter function alter1.plus1(int) set schema alter2;
@@ -2141,6 +2142,7 @@ alter operator class alter1.ctype_hash_ops using hash set schema alter2;
 alter operator family alter1.ctype_hash_ops using hash set schema alter2;
 alter operator alter1.=(alter1.ctype, alter1.ctype) set schema alter2;
 alter function alter1.same(alter1.ctype, alter1.ctype) set schema alter2;
+alter type alter1.ctype set schema alter1; -- no-op, same schema
 alter type alter1.ctype set schema alter2;
 alter conversion alter1.ascii_to_utf8 set schema alter2;
 alter text search parser alter1.prs set schema alter2;
@@ -2567,9 +2569,8 @@ ALTER TABLE new_system_table SET SCHEMA pg_catalog;
 -- XXX: it's currently impossible to move relations out of pg_catalog
 ALTER TABLE new_system_table SET SCHEMA public;
 ERROR:  cannot remove dependency on schema pg_catalog because it is a system object
--- move back, will currently error out, already there
+-- move back, will be ignored -- already there
 ALTER TABLE new_system_table SET SCHEMA pg_catalog;
-ERROR:  table new_system_table is already in schema "pg_catalog"
 ALTER TABLE new_system_table RENAME TO old_system_table;
 CREATE INDEX old_system_table__othercol ON old_system_table (othercol);
 INSERT INTO old_system_table(othercol) VALUES ('somedata'), ('otherdata');

diff --git a/src/test/regress/sql/alter_generic.sql b/src/test/regress/sql/alter_generic.sql
index ed4398b30a0ae257b3e277b321e6279e2266b2bc..8a811d47b3312ee322db4c5bf3d88fad9aace79b 100644 (file)
--- a/src/test/regress/sql/alter_generic.sql
+++ b/src/test/regress/sql/alter_generic.sql
@@ -44,6 +44,7 @@ ALTER FUNCTION alt_func1(int) RENAME TO alt_func2;  -- failed (name conflict)
 ALTER FUNCTION alt_func1(int) RENAME TO alt_func3;  -- OK
 ALTER FUNCTION alt_func2(int) OWNER TO regtest_alter_user2;  -- failed (no role membership)
 ALTER FUNCTION alt_func2(int) OWNER TO regtest_alter_user3;  -- OK
+ALTER FUNCTION alt_func2(int) SET SCHEMA alt_nsp1;  -- OK, already there
 ALTER FUNCTION alt_func2(int) SET SCHEMA alt_nsp2;  -- OK
 
 ALTER AGGREGATE alt_agg1(int) RENAME TO alt_agg2;   -- failed (name conflict)

diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 778791d9fd182a854bbc994524883834dc95ff77..6740c609c6bedf0a3f12ddd1ea0fbb34cff7cc49 100644 (file)
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1465,6 +1465,7 @@ create text search dictionary alter1.dict(template = alter1.tmpl);
 insert into alter1.t1(f2) values(11);
 insert into alter1.t1(f2) values(12);
 
+alter table alter1.t1 set schema alter1; -- no-op, same schema
 alter table alter1.t1 set schema alter2;
 alter table alter1.v1 set schema alter2;
 alter function alter1.plus1(int) set schema alter2;
@@ -1473,6 +1474,7 @@ alter operator class alter1.ctype_hash_ops using hash set schema alter2;
 alter operator family alter1.ctype_hash_ops using hash set schema alter2;
 alter operator alter1.=(alter1.ctype, alter1.ctype) set schema alter2;
 alter function alter1.same(alter1.ctype, alter1.ctype) set schema alter2;
+alter type alter1.ctype set schema alter1; -- no-op, same schema
 alter type alter1.ctype set schema alter2;
 alter conversion alter1.ascii_to_utf8 set schema alter2;
 alter text search parser alter1.prs set schema alter2;
@@ -1704,7 +1706,7 @@ ALTER TABLE new_system_table SET SCHEMA pg_catalog;
 
 -- XXX: it's currently impossible to move relations out of pg_catalog
 ALTER TABLE new_system_table SET SCHEMA public;
--- move back, will currently error out, already there
+-- move back, will be ignored -- already there
 ALTER TABLE new_system_table SET SCHEMA pg_catalog;
 ALTER TABLE new_system_table RENAME TO old_system_table;
 CREATE INDEX old_system_table__othercol ON old_system_table (othercol);