From: Tom Lane Date: Sat, 12 May 2007 00:55:00 +0000 (+0000) Subject: Fix the problem that creating a user-defined type named _foo, followed by one X-Git-Tag: REL9_0_0~5562 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=9aa3c782c939f29bd562a22030fb3a64e6f18365;p=pg-rex%2Fsyncrep.git Fix the problem that creating a user-defined type named _foo, followed by one named foo, would work but the other ordering would not. If a user-specified type or table name collides with an existing auto-generated array name, just rename the array type out of the way by prepending more underscores. This should not create any backward-compatibility issues, since the cases in which this will happen would have failed outright in prior releases. Also fix an oversight in the arrays-of-composites patch: ALTER TABLE RENAME renamed the table's rowtype but not its array type. --- diff --git a/doc/src/sgml/ref/create_type.sgml b/doc/src/sgml/ref/create_type.sgml index 08f4c1464c..038cf8c256 100644 --- a/doc/src/sgml/ref/create_type.sgml +++ b/doc/src/sgml/ref/create_type.sgml @@ -1,5 +1,5 @@ @@ -530,18 +530,6 @@ CREATE TYPE name Notes - It is best to avoid using type names that begin with the underscore - character (_). PostgreSQL - forms the name of an array type by prepending one or more underscores - to the element type's name, and these names may collide with user-defined - type names that begin with underscore. While the system will modify - generated array type names to avoid collisions, this does not help if the - conflicting array type already exists when you try to create your type. - Also, various old client software may assume that names beginning with - underscores always represent arrays. - - - Because there are no restrictions on use of a data type once it's been created, creating a base type is tantamount to granting public execute permission on the functions mentioned in the type definition. (The creator @@ -553,6 +541,27 @@ CREATE TYPE name + Before PostgreSQL version 8.3, the name of + a generated array type was always exactly the element type's name with one + underscore character (_) prepended. (Type names were + therefore restricted in length to one less character than other names.) + While this is still usually the case, the array type name may vary from + this in case of maximum-length names or collisions with user type names + that begin with underscore. Writing code that depends on this convention + is therefore deprecated. Instead, use + pg_type.typarray to locate the array type + associated with a given type. + + + + It may be advisable to avoid using type and table names that begin with + underscore. While the server will change generated array type names to + avoid collisions with user-given names, there is still risk of confusion, + particularly with old client software that may assume that type names + beginning with underscores always represent arrays. + + + Before PostgreSQL version 8.2, the syntax CREATE TYPE name did not exist. The way to create a new base type was to create its input function first. diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 8b34d685d8..43b493767d 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.319 2007/05/11 17:57:11 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.320 2007/05/12 00:54:59 tgl Exp $ * * * INTERFACE ROUTINES @@ -797,6 +797,7 @@ heap_create_with_catalog(const char *relname, { Relation pg_class_desc; Relation new_rel_desc; + Oid old_type_oid; Oid new_type_oid; Oid new_array_oid = InvalidOid; @@ -815,6 +816,27 @@ heap_create_with_catalog(const char *relname, errmsg("relation \"%s\" already exists", relname))); /* + * Since we are going to create a rowtype as well, also check for + * collision with an existing type name. If there is one and it's + * an autogenerated array, we can rename it out of the way; otherwise + * we can at least give a good error message. + */ + old_type_oid = GetSysCacheOid(TYPENAMENSP, + CStringGetDatum(relname), + ObjectIdGetDatum(relnamespace), + 0, 0); + if (OidIsValid(old_type_oid)) + { + if (!moveArrayTypeName(old_type_oid, relname, relnamespace)) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("type \"%s\" already exists", relname), + errhint("A relation has an associated type of the same name, " + "so you must use a name that doesn't conflict " + "with any existing type."))); + } + + /* * Allocate an OID for the relation, unless we were told what to use. * * The OID will be the relfilenode as well, so make sure it doesn't @@ -861,8 +883,9 @@ heap_create_with_catalog(const char *relname, * Since defining a relation also defines a complex type, we add a new * system type corresponding to the new relation. * - * NOTE: we could get a unique-index failure here, in case the same name - * has already been used for a type. + * NOTE: we could get a unique-index failure here, in case someone else is + * creating the same type name in parallel but hadn't committed yet + * when we checked for a duplicate name above. */ new_type_oid = AddNewRelationType(relname, relnamespace, diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c index 8360149604..7419b36338 100644 --- a/src/backend/catalog/pg_type.c +++ b/src/backend/catalog/pg_type.c @@ -8,13 +8,14 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/catalog/pg_type.c,v 1.112 2007/05/11 17:57:12 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/catalog/pg_type.c,v 1.113 2007/05/12 00:54:59 tgl Exp $ * *------------------------------------------------------------------------- */ #include "postgres.h" #include "access/heapam.h" +#include "access/xact.h" #include "catalog/dependency.h" #include "catalog/indexing.h" #include "catalog/pg_namespace.h" @@ -26,6 +27,7 @@ #include "utils/acl.h" #include "utils/builtins.h" #include "utils/fmgroids.h" +#include "utils/lsyscache.h" #include "utils/syscache.h" @@ -551,30 +553,35 @@ GenerateTypeDependencies(Oid typeNamespace, /* * TypeRename - * This renames a type + * This renames a type, as well as any associated array type. * - * Note: any associated array type is *not* renamed; caller must make - * another call to handle that case. Currently this is only used for - * renaming types associated with tables, for which there are no arrays. + * Note: this isn't intended to be a user-exposed function; it doesn't check + * permissions etc. (Perhaps TypeRenameInternal would be a better name.) + * Currently this is only used for renaming table rowtypes. */ void -TypeRename(const char *oldTypeName, Oid typeNamespace, - const char *newTypeName) +TypeRename(Oid typeOid, const char *newTypeName, Oid typeNamespace) { Relation pg_type_desc; HeapTuple tuple; + Form_pg_type typ; + Oid arrayOid; pg_type_desc = heap_open(TypeRelationId, RowExclusiveLock); - tuple = SearchSysCacheCopy(TYPENAMENSP, - CStringGetDatum(oldTypeName), - ObjectIdGetDatum(typeNamespace), - 0, 0); + tuple = SearchSysCacheCopy(TYPEOID, + ObjectIdGetDatum(typeOid), + 0, 0, 0); if (!HeapTupleIsValid(tuple)) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("type \"%s\" does not exist", oldTypeName))); + elog(ERROR, "cache lookup failed for type %u", typeOid); + typ = (Form_pg_type) GETSTRUCT(tuple); + + /* We are not supposed to be changing schemas here */ + Assert(typeNamespace == typ->typnamespace); + arrayOid = typ->typarray; + + /* Just to give a more friendly error than unique-index violation */ if (SearchSysCacheExists(TYPENAMENSP, CStringGetDatum(newTypeName), ObjectIdGetDatum(typeNamespace), @@ -583,7 +590,8 @@ TypeRename(const char *oldTypeName, Oid typeNamespace, (errcode(ERRCODE_DUPLICATE_OBJECT), errmsg("type \"%s\" already exists", newTypeName))); - namestrcpy(&(((Form_pg_type) GETSTRUCT(tuple))->typname), newTypeName); + /* OK, do the rename --- tuple is a copy, so OK to scribble on it */ + namestrcpy(&(typ->typname), newTypeName); simple_heap_update(pg_type_desc, &tuple->t_self, tuple); @@ -592,11 +600,20 @@ TypeRename(const char *oldTypeName, Oid typeNamespace, heap_freetuple(tuple); heap_close(pg_type_desc, RowExclusiveLock); + + /* If the type has an array type, recurse to handle that */ + if (OidIsValid(arrayOid)) + { + char *arrname = makeArrayTypeName(newTypeName, typeNamespace); + + TypeRename(arrayOid, arrname, typeNamespace); + pfree(arrname); + } } /* - * makeArrayTypeName(typeName) + * makeArrayTypeName * - given a base type name, make an array type name for it * * the caller is responsible for pfreeing the result @@ -638,3 +655,66 @@ makeArrayTypeName(const char *typeName, Oid typeNamespace) return arr; } + + +/* + * moveArrayTypeName + * - try to reassign an array type name that the user wants to use. + * + * The given type name has been discovered to already exist (with the given + * OID). If it is an autogenerated array type, change the array type's name + * to not conflict. This allows the user to create type "foo" followed by + * type "_foo" without problems. (Of course, there are race conditions if + * two backends try to create similarly-named types concurrently, but the + * worst that can happen is an unnecessary failure --- anything we do here + * will be rolled back if the type creation fails due to conflicting names.) + * + * Note that this must be called *before* calling makeArrayTypeName to + * determine the new type's own array type name; else the latter will + * certainly pick the same name. + * + * Returns TRUE if successfully moved the type, FALSE if not. + * + * We also return TRUE if the given type is a shell type. In this case + * the type has not been renamed out of the way, but nonetheless it can + * be expected that TypeCreate will succeed. This behavior is convenient + * for most callers --- those that need to distinguish the shell-type case + * must do their own typisdefined test. + */ +bool +moveArrayTypeName(Oid typeOid, const char *typeName, Oid typeNamespace) +{ + Oid elemOid; + char *newname; + + /* We need do nothing if it's a shell type. */ + if (!get_typisdefined(typeOid)) + return true; + + /* Can't change it if it's not an autogenerated array type. */ + elemOid = get_element_type(typeOid); + if (!OidIsValid(elemOid) || + get_array_type(elemOid) != typeOid) + return false; + + /* + * OK, use makeArrayTypeName to pick an unused modification of the + * name. Note that since makeArrayTypeName is an iterative process, + * this will produce a name that it might have produced the first time, + * had the conflicting type we are about to create already existed. + */ + newname = makeArrayTypeName(typeName, typeNamespace); + + /* Apply the rename */ + TypeRename(typeOid, newname, typeNamespace); + + /* + * We must bump the command counter so that any subsequent use of + * makeArrayTypeName sees what we just did and doesn't pick the same name. + */ + CommandCounterIncrement(); + + pfree(newname); + + return true; +} diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index eecd4943ae..ee658056ec 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.221 2007/05/11 20:16:36 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.222 2007/05/12 00:54:59 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1626,6 +1626,7 @@ renamerel(Oid myrelid, const char *newrelname) Relation targetrelation; Relation relrelation; /* for RELATION relation */ HeapTuple reltup; + Form_pg_class relform; Oid namespaceId; char *oldrelname; char relkind; @@ -1655,10 +1656,11 @@ renamerel(Oid myrelid, const char *newrelname) relrelation = heap_open(RelationRelationId, RowExclusiveLock); reltup = SearchSysCacheCopy(RELOID, - PointerGetDatum(myrelid), + ObjectIdGetDatum(myrelid), 0, 0, 0); if (!HeapTupleIsValid(reltup)) /* shouldn't happen */ elog(ERROR, "cache lookup failed for relation %u", myrelid); + relform = (Form_pg_class) GETSTRUCT(reltup); if (get_relname_relid(newrelname, namespaceId) != InvalidOid) ereport(ERROR, @@ -1670,7 +1672,7 @@ renamerel(Oid myrelid, const char *newrelname) * Update pg_class tuple with new relname. (Scribbling on reltup is OK * because it's a copy...) */ - namestrcpy(&(((Form_pg_class) GETSTRUCT(reltup))->relname), newrelname); + namestrcpy(&(relform->relname), newrelname); simple_heap_update(relrelation, &reltup->t_self, reltup); @@ -1683,8 +1685,8 @@ renamerel(Oid myrelid, const char *newrelname) /* * Also rename the associated type, if any. */ - if (relkind != RELKIND_INDEX) - TypeRename(oldrelname, namespaceId, newrelname); + if (OidIsValid(targetrelation->rd_rel->reltype)) + TypeRename(targetrelation->rd_rel->reltype, newrelname, namespaceId); /* * Close rel, but keep exclusive lock! diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 915c669c97..5402cde74c 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.103 2007/05/11 20:16:54 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.104 2007/05/12 00:54:59 tgl Exp $ * * DESCRIPTION * The "DefineFoo" routines take the parse tree and pick out the @@ -137,13 +137,27 @@ DefineType(List *names, List *parameters) /* * Look to see if type already exists (presumably as a shell; if not, - * TypeCreate will complain). If it doesn't, create it as a shell, so - * that the OID is known for use in the I/O function definitions. + * TypeCreate will complain). */ typoid = GetSysCacheOid(TYPENAMENSP, CStringGetDatum(typeName), ObjectIdGetDatum(typeNamespace), 0, 0); + + /* + * If it's not a shell, see if it's an autogenerated array type, + * and if so rename it out of the way. + */ + if (OidIsValid(typoid) && get_typisdefined(typoid)) + { + if (moveArrayTypeName(typoid, typeName, typeNamespace)) + typoid = InvalidOid; + } + + /* + * If it doesn't exist, create it as a shell, so that the OID is known + * for use in the I/O function definitions. + */ if (!OidIsValid(typoid)) { typoid = TypeShellMake(typeName, typeNamespace); @@ -602,6 +616,7 @@ DefineDomain(CreateDomainStmt *stmt) ListCell *listptr; Oid basetypeoid; Oid domainoid; + Oid old_type_oid; Form_pg_type baseType; int32 basetypeMod; @@ -617,6 +632,22 @@ DefineDomain(CreateDomainStmt *stmt) get_namespace_name(domainNamespace)); /* + * Check for collision with an existing type name. If there is one and + * it's an autogenerated array, we can rename it out of the way. + */ + old_type_oid = GetSysCacheOid(TYPENAMENSP, + CStringGetDatum(domainName), + ObjectIdGetDatum(domainNamespace), + 0, 0); + if (OidIsValid(old_type_oid)) + { + if (!moveArrayTypeName(old_type_oid, domainName, domainNamespace)) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("type \"%s\" already exists", domainName))); + } + + /* * Look up the base type. */ typeTup = typenameType(NULL, stmt->typename); @@ -948,6 +979,7 @@ DefineEnum(CreateEnumStmt *stmt) Oid enumNamespace; Oid enumTypeOid; AclResult aclresult; + Oid old_type_oid; Oid enumArrayOid; Relation pg_type; @@ -961,6 +993,22 @@ DefineEnum(CreateEnumStmt *stmt) aclcheck_error(aclresult, ACL_KIND_NAMESPACE, get_namespace_name(enumNamespace)); + /* + * Check for collision with an existing type name. If there is one and + * it's an autogenerated array, we can rename it out of the way. + */ + old_type_oid = GetSysCacheOid(TYPENAMENSP, + CStringGetDatum(enumName), + ObjectIdGetDatum(enumNamespace), + 0, 0); + if (OidIsValid(old_type_oid)) + { + if (!moveArrayTypeName(old_type_oid, enumName, enumNamespace)) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("type \"%s\" already exists", enumName))); + } + /* Preassign array type OID so we can insert it in pg_type.typarray */ pg_type = heap_open(TypeRelationId, AccessShareLock); enumArrayOid = GetNewOid(pg_type); diff --git a/src/include/catalog/pg_type.h b/src/include/catalog/pg_type.h index c663ff4ec7..0d9ff0d6ee 100644 --- a/src/include/catalog/pg_type.h +++ b/src/include/catalog/pg_type.h @@ -8,7 +8,7 @@ * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/catalog/pg_type.h,v 1.183 2007/05/11 17:57:13 tgl Exp $ + * $PostgreSQL: pgsql/src/include/catalog/pg_type.h,v 1.184 2007/05/12 00:54:59 tgl Exp $ * * NOTES * the genbki.sh script reads this file and generates .bki @@ -645,9 +645,12 @@ extern void GenerateTypeDependencies(Oid typeNamespace, Node *defaultExpr, bool rebuild); -extern void TypeRename(const char *oldTypeName, Oid typeNamespace, - const char *newTypeName); +extern void TypeRename(Oid typeOid, const char *newTypeName, + Oid typeNamespace); extern char *makeArrayTypeName(const char *typeName, Oid typeNamespace); +extern bool moveArrayTypeName(Oid typeOid, const char *typeName, + Oid typeNamespace); + #endif /* PG_TYPE_H */ diff --git a/src/test/regress/expected/arrays.out b/src/test/regress/expected/arrays.out index 643ae7f39b..936db41c4b 100644 --- a/src/test/regress/expected/arrays.out +++ b/src/test/regress/expected/arrays.out @@ -871,3 +871,26 @@ SELECT max(f1), min(f1), max(f2), min(f2), max(f3), min(f3) FROM arraggtest; {4,2,6,7,8,1} | {} | {{white,yellow},{pink,orange}} | {{black,red},{green,orange}} | {2.1,3.3,1.8,1.7,1.6} | {1.6} (1 row) +-- A few simple tests for arrays of composite types +create type comptype as (f1 int, f2 text); +create table comptable (c1 comptype, c2 comptype[]); +-- XXX would like to not have to specify row() construct types here ... +insert into comptable + values (row(1,'foo'), array[row(2,'bar')::comptype, row(3,'baz')::comptype]); +-- check that implicitly named array type _comptype isn't a problem +create type _comptype as enum('fooey'); +select * from comptable; + c1 | c2 +---------+----------------------- + (1,foo) | {"(2,bar)","(3,baz)"} +(1 row) + +select c2[2].f2 from comptable; + f2 +----- + baz +(1 row) + +drop type _comptype; +drop table comptable; +drop type comptype; diff --git a/src/test/regress/sql/arrays.sql b/src/test/regress/sql/arrays.sql index 1f574a0184..a60bf560fa 100644 --- a/src/test/regress/sql/arrays.sql +++ b/src/test/regress/sql/arrays.sql @@ -318,3 +318,23 @@ SELECT max(f1), min(f1), max(f2), min(f2), max(f3), min(f3) FROM arraggtest; INSERT INTO arraggtest (f1, f2, f3) VALUES ('{}','{{pink,white,blue,red,grey,orange}}','{2.1,1.87,1.4,2.2}'); SELECT max(f1), min(f1), max(f2), min(f2), max(f3), min(f3) FROM arraggtest; + +-- A few simple tests for arrays of composite types + +create type comptype as (f1 int, f2 text); + +create table comptable (c1 comptype, c2 comptype[]); + +-- XXX would like to not have to specify row() construct types here ... +insert into comptable + values (row(1,'foo'), array[row(2,'bar')::comptype, row(3,'baz')::comptype]); + +-- check that implicitly named array type _comptype isn't a problem +create type _comptype as enum('fooey'); + +select * from comptable; +select c2[2].f2 from comptable; + +drop type _comptype; +drop table comptable; +drop type comptype;