Kohei KaiGai escribió:
> I'm probably saying same idea. It just adds invocation of external
> functions to check naming conflicts of functions or collation; that
> takes additional 4-lines for special case handling
> in AlterObjectNamespace_internal().
Okay, I can agree with this implementation plan. I didn't like your
patch very much though; I'm not sure that the handling of collations was
sane. And the names of the AlterFooNamespace_oid() functions is now
completely misleading, because they no longer do that. So I renamed
them, and while at it I noticed that the collation case can share code
with the RENAME code path; the function case could probably do likewise,
but it becomes uglier.
Note that after this patch, the error message now cites the function
signature, not just the name. This is more consistent with other cases.
9.2 and HEAD:
alvherre=# alter function foo.f(int,int) set schema bar;
ERROR: function "f" already exists in schema "bar"
(this error is misleading, because there can validly be a f(int)
function in schema bar and that wouldn't cause a problem for this SET
SCHEMA command).
After my patch:
alvherre=# alter function foo.f(int,int) set schema bar;
ERROR: function f(integer, integer) already exists in schema "bar"
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
*** a/src/backend/commands/alter.c
--- b/src/backend/commands/alter.c
***************
*** 19,27 ****
--- 19,29 ----
#include "catalog/dependency.h"
#include "catalog/indexing.h"
#include "catalog/namespace.h"
+ #include "catalog/pg_collation.h"
#include "catalog/pg_largeobject.h"
#include "catalog/pg_largeobject_metadata.h"
#include "catalog/pg_namespace.h"
+ #include "catalog/pg_proc.h"
#include "commands/alter.h"
#include "commands/collationcmds.h"
#include "commands/conversioncmds.h"
***************
*** 146,185 **** ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt)
{
switch (stmt->objectType)
{
- case OBJECT_AGGREGATE:
- return AlterFunctionNamespace(stmt->object, stmt->objarg, true,
- stmt->newschema);
-
- case OBJECT_COLLATION:
- return AlterCollationNamespace(stmt->object, stmt->newschema);
-
case OBJECT_EXTENSION:
return AlterExtensionNamespace(stmt->object, stmt->newschema);
! case OBJECT_FUNCTION:
! return AlterFunctionNamespace(stmt->object, stmt->objarg, false,
! stmt->newschema);
!
case OBJECT_SEQUENCE:
case OBJECT_TABLE:
case OBJECT_VIEW:
- case OBJECT_FOREIGN_TABLE:
return AlterTableNamespace(stmt);
- case OBJECT_TYPE:
case OBJECT_DOMAIN:
return AlterTypeNamespace(stmt->object, stmt->newschema,
stmt->objectType);
/* generic code path */
case OBJECT_CONVERSION:
case OBJECT_OPERATOR:
case OBJECT_OPCLASS:
case OBJECT_OPFAMILY:
! case OBJECT_TSPARSER:
case OBJECT_TSDICTIONARY:
case OBJECT_TSTEMPLATE:
- case OBJECT_TSCONFIGURATION:
{
Relation catalog;
Relation relation;
--- 148,179 ----
{
switch (stmt->objectType)
{
case OBJECT_EXTENSION:
return AlterExtensionNamespace(stmt->object, stmt->newschema);
! case OBJECT_FOREIGN_TABLE:
case OBJECT_SEQUENCE:
case OBJECT_TABLE:
case OBJECT_VIEW:
return AlterTableNamespace(stmt);
case OBJECT_DOMAIN:
+ case OBJECT_TYPE:
return AlterTypeNamespace(stmt->object, stmt->newschema,
stmt->objectType);
/* generic code path */
+ case OBJECT_AGGREGATE:
+ case OBJECT_COLLATION:
case OBJECT_CONVERSION:
+ case OBJECT_FUNCTION:
case OBJECT_OPERATOR:
case OBJECT_OPCLASS:
case OBJECT_OPFAMILY:
! case OBJECT_TSCONFIGURATION:
case OBJECT_TSDICTIONARY:
+ case OBJECT_TSPARSER:
case OBJECT_TSTEMPLATE:
{
Relation catalog;
Relation relation;
***************
*** 253,270 **** AlterObjectNamespace_oid(Oid classId, Oid objid, Oid nspOid,
break;
}
- case OCLASS_PROC:
- oldNspOid = AlterFunctionNamespace_oid(objid, nspOid);
- break;
-
case OCLASS_TYPE:
oldNspOid = AlterTypeNamespace_oid(objid, nspOid, objsMoved);
break;
case OCLASS_COLLATION:
- oldNspOid = AlterCollationNamespace_oid(objid, nspOid);
- break;
-
case OCLASS_CONVERSION:
case OCLASS_OPERATOR:
case OCLASS_OPCLASS:
--- 247,258 ----
break;
}
case OCLASS_TYPE:
oldNspOid = AlterTypeNamespace_oid(objid, nspOid, objsMoved);
break;
+ case OCLASS_PROC:
case OCLASS_COLLATION:
case OCLASS_CONVERSION:
case OCLASS_OPERATOR:
case OCLASS_OPCLASS:
***************
*** 380,385 **** AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid)
--- 368,385 ----
errmsg("%s already exists in schema \"%s\"",
getObjectDescriptionOids(classId, objid),
get_namespace_name(nspOid))));
+ else if (classId == ProcedureRelationId)
+ IsThereFunctionInNamespace(objid, nspOid);
+ else if (classId == CollationRelationId)
+ {
+ char *collname;
+
+ collname = get_collation_name(objid);
+ if (!collname)
+ elog(ERROR, "cache lookup failed for collation %u", objid);
+ IsThereCollationInNamespace(collname, nspOid);
+ pfree(collname);
+ }
/* Build modified tuple */
values = palloc0(RelationGetNumberOfAttributes(rel) * sizeof(Datum));
*** a/src/backend/commands/collationcmds.c
--- b/src/backend/commands/collationcmds.c
***************
*** 167,193 **** RenameCollation(List *name, const char *newname)
namespaceOid = ((Form_pg_collation) GETSTRUCT(tup))->collnamespace;
/* make sure the new name doesn't exist */
! if (SearchSysCacheExists3(COLLNAMEENCNSP,
! CStringGetDatum(newname),
! Int32GetDatum(GetDatabaseEncoding()),
! ObjectIdGetDatum(namespaceOid)))
! ereport(ERROR,
! (errcode(ERRCODE_DUPLICATE_OBJECT),
! errmsg("collation \"%s\" for encoding \"%s\" already exists in schema \"%s\"",
! newname,
! GetDatabaseEncodingName(),
! get_namespace_name(namespaceOid))));
!
! /* mustn't match an any-encoding entry, either */
! if (SearchSysCacheExists3(COLLNAMEENCNSP,
! CStringGetDatum(newname),
! Int32GetDatum(-1),
! ObjectIdGetDatum(namespaceOid)))
! ereport(ERROR,
! (errcode(ERRCODE_DUPLICATE_OBJECT),
! errmsg("collation \"%s\" already exists in schema \"%s\"",
! newname,
! get_namespace_name(namespaceOid))));
/* must be owner */
if (!pg_collation_ownercheck(collationOid, GetUserId()))
--- 167,173 ----
namespaceOid = ((Form_pg_collation) GETSTRUCT(tup))->collnamespace;
/* make sure the new name doesn't exist */
! IsThereCollationInNamespace(newname, namespaceOid);
/* must be owner */
if (!pg_collation_ownercheck(collationOid, GetUserId()))
***************
*** 213,283 **** RenameCollation(List *name, const char *newname)
}
/*
! * Execute ALTER COLLATION SET SCHEMA
! */
! Oid
! AlterCollationNamespace(List *name, const char *newschema)
! {
! Oid collOid,
! nspOid;
!
! collOid = get_collation_oid(name, false);
!
! nspOid = LookupCreationNamespace(newschema);
!
! AlterCollationNamespace_oid(collOid, nspOid);
!
! return collOid;
! }
!
! /*
! * Change collation schema, by oid
*/
! Oid
! AlterCollationNamespace_oid(Oid collOid, Oid newNspOid)
{
- Oid oldNspOid;
- Relation rel;
- char *collation_name;
-
- rel = heap_open(CollationRelationId, RowExclusiveLock);
-
- /*
- * We have to check for name collision ourselves, because
- * AlterObjectNamespace_internal doesn't know how to deal with the encoding
- * considerations.
- */
- collation_name = get_collation_name(collOid);
- if (!collation_name)
- elog(ERROR, "cache lookup failed for collation %u", collOid);
-
/* make sure the name doesn't already exist in new schema */
if (SearchSysCacheExists3(COLLNAMEENCNSP,
! CStringGetDatum(collation_name),
Int32GetDatum(GetDatabaseEncoding()),
! ObjectIdGetDatum(newNspOid)))
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_OBJECT),
errmsg("collation \"%s\" for encoding \"%s\" already exists in schema \"%s\"",
! collation_name,
! GetDatabaseEncodingName(),
! get_namespace_name(newNspOid))));
/* mustn't match an any-encoding entry, either */
if (SearchSysCacheExists3(COLLNAMEENCNSP,
! CStringGetDatum(collation_name),
Int32GetDatum(-1),
! ObjectIdGetDatum(newNspOid)))
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_OBJECT),
errmsg("collation \"%s\" already exists in schema \"%s\"",
! collation_name,
! get_namespace_name(newNspOid))));
!
! /* OK, do the work */
! oldNspOid = AlterObjectNamespace_internal(rel, collOid, newNspOid);
!
! heap_close(rel, RowExclusiveLock);
!
! return oldNspOid;
}
--- 193,224 ----
}
/*
! * Subroutine for ALTER COLLATION SET SCHEMA and RENAME
! *
! * Is there a collation with the same name of the given collation already in
! * the given namespace? If so, raise an appropriate error message.
*/
! void
! IsThereCollationInNamespace(const char *collname, Oid nspOid)
{
/* make sure the name doesn't already exist in new schema */
if (SearchSysCacheExists3(COLLNAMEENCNSP,
! CStringGetDatum(collname),
Int32GetDatum(GetDatabaseEncoding()),
! ObjectIdGetDatum(nspOid)))
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_OBJECT),
errmsg("collation \"%s\" for encoding \"%s\" already exists in schema \"%s\"",
! collname, GetDatabaseEncodingName(),
! get_namespace_name(nspOid))));
/* mustn't match an any-encoding entry, either */
if (SearchSysCacheExists3(COLLNAMEENCNSP,
! CStringGetDatum(collname),
Int32GetDatum(-1),
! ObjectIdGetDatum(nspOid)))
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_OBJECT),
errmsg("collation \"%s\" already exists in schema \"%s\"",
! collname, get_namespace_name(nspOid))));
}
*** a/src/backend/commands/functioncmds.c
--- b/src/backend/commands/functioncmds.c
***************
*** 1688,1733 **** DropCastById(Oid castOid)
}
/*
! * Execute ALTER FUNCTION/AGGREGATE SET SCHEMA
*
! * These commands are identical except for the lookup procedure, so share code.
*/
! Oid
! AlterFunctionNamespace(List *name, List *argtypes, bool isagg,
! const char *newschema)
! {
! Oid procOid;
! Oid nspOid;
!
! /* get function OID */
! if (isagg)
! procOid = LookupAggNameTypeNames(name, argtypes, false);
! else
! procOid = LookupFuncNameTypeNames(name, argtypes, false);
!
! /* get schema OID and check its permissions */
! nspOid = LookupCreationNamespace(newschema);
!
! AlterFunctionNamespace_oid(procOid, nspOid);
!
! return procOid;
! }
!
! Oid
! AlterFunctionNamespace_oid(Oid procOid, Oid nspOid)
{
- Oid oldNspOid;
HeapTuple tup;
- Relation procRel;
Form_pg_proc proc;
- procRel = heap_open(ProcedureRelationId, RowExclusiveLock);
-
- /*
- * We have to check for name collisions ourselves, because
- * AlterObjectNamespace_internal doesn't know how to deal with the
- * argument types.
- */
tup = SearchSysCacheCopy1(PROCOID, ObjectIdGetDatum(procOid));
if (!HeapTupleIsValid(tup))
elog(ERROR, "cache lookup failed for function %u", procOid);
--- 1688,1706 ----
}
/*
! * Subroutine for ALTER FUNCTION/AGGREGATE SET SCHEMA
*
! * Is there a function with the same name and signature of the given function
! * already in the given namespace? If so, raise an appropriate error message.
! *
! * XXX share code with RenameFunction?
*/
! void
! IsThereFunctionInNamespace(Oid procOid, Oid nspOid)
{
HeapTuple tup;
Form_pg_proc proc;
tup = SearchSysCacheCopy1(PROCOID, ObjectIdGetDatum(procOid));
if (!HeapTupleIsValid(tup))
elog(ERROR, "cache lookup failed for function %u", procOid);
***************
*** 1740,1758 **** AlterFunctionNamespace_oid(Oid procOid, Oid nspOid)
ObjectIdGetDatum(nspOid)))
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_FUNCTION),
! errmsg("function \"%s\" already exists in schema \"%s\"",
! NameStr(proc->proname),
get_namespace_name(nspOid))));
-
- /* OK, do the work */
- oldNspOid = AlterObjectNamespace_internal(procRel, procOid, nspOid);
-
- heap_close(procRel, RowExclusiveLock);
-
- return oldNspOid;
}
-
/*
* ExecuteDoStmt
* Execute inline procedural-language code
--- 1713,1726 ----
ObjectIdGetDatum(nspOid)))
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_FUNCTION),
! errmsg("function %s already exists in schema \"%s\"",
! funcname_signature_string(NameStr(proc->proname),
! proc->pronargs,
! NIL,
! proc->proargtypes.values),
get_namespace_name(nspOid))));
}
/*
* ExecuteDoStmt
* Execute inline procedural-language code
*** a/src/include/commands/collationcmds.h
--- b/src/include/commands/collationcmds.h
***************
*** 19,25 ****
extern Oid DefineCollation(List *names, List *parameters);
extern Oid RenameCollation(List *name, const char *newname);
! extern Oid AlterCollationNamespace(List *name, const char *newschema);
! extern Oid AlterCollationNamespace_oid(Oid collOid, Oid newNspOid);
#endif /* COLLATIONCMDS_H */
--- 19,24 ----
extern Oid DefineCollation(List *names, List *parameters);
extern Oid RenameCollation(List *name, const char *newname);
! extern void IsThereCollationInNamespace(const char *collname, Oid newNspOid);
#endif /* COLLATIONCMDS_H */
*** a/src/include/commands/defrem.h
--- b/src/include/commands/defrem.h
***************
*** 50,58 **** extern Oid RenameFunction(List *name, List *argtypes, const char *newname);
extern Oid AlterFunction(AlterFunctionStmt *stmt);
extern Oid CreateCast(CreateCastStmt *stmt);
extern void DropCastById(Oid castOid);
! extern Oid AlterFunctionNamespace(List *name, List *argtypes, bool isagg,
! const char *newschema);
! extern Oid AlterFunctionNamespace_oid(Oid procOid, Oid nspOid);
extern void ExecuteDoStmt(DoStmt *stmt);
extern Oid get_cast_oid(Oid sourcetypeid, Oid targettypeid, bool missing_ok);
--- 50,56 ----
extern Oid AlterFunction(AlterFunctionStmt *stmt);
extern Oid CreateCast(CreateCastStmt *stmt);
extern void DropCastById(Oid castOid);
! extern void IsThereFunctionInNamespace(Oid procOid, Oid nspOid);
extern void ExecuteDoStmt(DoStmt *stmt);
extern Oid get_cast_oid(Oid sourcetypeid, Oid targettypeid, bool missing_ok);
*** a/src/test/regress/expected/alter_generic.out
--- b/src/test/regress/expected/alter_generic.out
***************
*** 69,75 **** ERROR: must be member of role "regtest_alter_user3"
ALTER FUNCTION alt_func3(int) SET SCHEMA alt_nsp2; -- failed (not owner)
ERROR: must be owner of function alt_func3
ALTER FUNCTION alt_func2(int) SET SCHEMA alt_nsp2; -- failed (name conflicts)
! ERROR: function "alt_func2" already exists in schema "alt_nsp2"
ALTER AGGREGATE alt_agg3(int) RENAME TO alt_agg4; -- failed (not owner)
ERROR: must be owner of function alt_agg3
ALTER AGGREGATE alt_agg1(int) RENAME TO alt_agg4; -- OK
--- 69,75 ----
ALTER FUNCTION alt_func3(int) SET SCHEMA alt_nsp2; -- failed (not owner)
ERROR: must be owner of function alt_func3
ALTER FUNCTION alt_func2(int) SET SCHEMA alt_nsp2; -- failed (name conflicts)
! ERROR: function alt_func2(integer) already exists in schema "alt_nsp2"
ALTER AGGREGATE alt_agg3(int) RENAME TO alt_agg4; -- failed (not owner)
ERROR: must be owner of function alt_agg3
ALTER AGGREGATE alt_agg1(int) RENAME TO alt_agg4; -- OK
***************
*** 80,86 **** ERROR: must be member of role "regtest_alter_user3"
ALTER AGGREGATE alt_agg3(int) SET SCHEMA alt_nsp2; -- failed (not owner)
ERROR: must be owner of function alt_agg3
ALTER AGGREGATE alt_agg2(int) SET SCHEMA alt_nsp2; -- failed (name conflict)
! ERROR: function "alt_agg2" already exists in schema "alt_nsp2"
RESET SESSION AUTHORIZATION;
SELECT n.nspname, proname, prorettype::regtype, proisagg, a.rolname
FROM pg_proc p, pg_namespace n, pg_authid a
--- 80,86 ----
ALTER AGGREGATE alt_agg3(int) SET SCHEMA alt_nsp2; -- failed (not owner)
ERROR: must be owner of function alt_agg3
ALTER AGGREGATE alt_agg2(int) SET SCHEMA alt_nsp2; -- failed (name conflict)
! ERROR: function alt_agg2(integer) already exists in schema "alt_nsp2"
RESET SESSION AUTHORIZATION;
SELECT n.nspname, proname, prorettype::regtype, proisagg, a.rolname
FROM pg_proc p, pg_namespace n, pg_authid a
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
