Kohei KaiGai <kai...@kaigai.gr.jp> writes: > OK, Are you suggesting to add a "generic" comments such as "Generic > function to change the name of a given object, for simple cases ...", > not a list of OBJECT_* at the head of this function, aren't you?
Just something like * Most simple objects are covered by a generic function, the other * still need special processing. Mainly I was surprised to see collation not supported here, but I didn't take enough time to understand why that is the case. I will do that later in the review process. > The pain point is AlterObjectNamespace_internal is not invoked by > only ExecAlterObjectSchemaStmt(), but AlterObjectNamespace_oid() > also. I should remember better about that as the use case is extensions… > It is the reason why I had to put get_object_type() to find out OBJECT_* > from the supplied ObjectAddress, because this code path does not > available to pass down its ObjectType from the caller. If we really want to do that, I think I would only do that in AlterObjectNamespace_oid and add an argument to AlterObjectNamespace_internal, that you already have in ExecAlterObjectSchemaStmt(). But really, what about just removing that part of your patch instead: @@ -396,14 +614,23 @@ AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid) * Check for duplicate name (more friendly than unique-index failure). * Since this is just a friendliness check, we can just skip it in cases * where there isn't a suitable syscache available. + * + * XXX - the caller should check object-name duplication, if the supplied + * object type need to take object arguments for identification, such as + * functions. */ - if (nameCacheId >= 0 && - SearchSysCacheExists2(nameCacheId, name, ObjectIdGetDatum(nspOid))) - ereport(ERROR, - (errcode(ERRCODE_DUPLICATE_OBJECT), - errmsg("%s already exists in schema \"%s\"", - getObjectDescriptionOids(classId, objid), - get_namespace_name(nspOid)))); + if (get_object_catcache_name(classId) >= 0) + { + ObjectAddress address; + + address.classId = classId; + address.objectId = objid; + address.objectSubId = 0; + + objtype = get_object_type(&address); + check_duplicate_objectname(objtype, nspOid, + NameStr(*DatumGetName(name)), NIL); + } It would be much simpler to retain the old-style duplicate object check, and compared to doing extra cache lookups, it'd still be much cleaner in my view. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers