OSDN Git Service

Fix the problem that creating a user-defined type named _foo, followed by one
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 12 May 2007 00:55:00 +0000 (00:55 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 12 May 2007 00:55:00 +0000 (00:55 +0000)
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.

doc/src/sgml/ref/create_type.sgml
src/backend/catalog/heap.c
src/backend/catalog/pg_type.c
src/backend/commands/tablecmds.c
src/backend/commands/typecmds.c
src/include/catalog/pg_type.h
src/test/regress/expected/arrays.out
src/test/regress/sql/arrays.sql

index 08f4c14..038cf8c 100644 (file)
@@ -1,5 +1,5 @@
 <!--
-$PostgreSQL: pgsql/doc/src/sgml/ref/create_type.sgml,v 1.70 2007/05/11 17:57:11 tgl Exp $
+$PostgreSQL: pgsql/doc/src/sgml/ref/create_type.sgml,v 1.71 2007/05/12 00:54:59 tgl Exp $
 PostgreSQL documentation
 -->
 
@@ -530,18 +530,6 @@ CREATE TYPE <replaceable class="parameter">name</replaceable>
   <title>Notes</title>
 
   <para>
-   It is best to avoid using type names that begin with the underscore
-   character (<literal>_</literal>).  <productname>PostgreSQL</productname>
-   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.
-  </para>
-
-  <para>
    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 <replaceable class="parameter">name</replaceable>
   </para>
 
   <para>
+   Before <productname>PostgreSQL</productname> version 8.3, the name of
+   a generated array type was always exactly the element type's name with one
+   underscore character (<literal>_</literal>) 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
+   <structname>pg_type</>.<structfield>typarray</> to locate the array type
+   associated with a given type.
+  </para>
+
+  <para>
+   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.
+  </para>
+
+  <para>
    Before <productname>PostgreSQL</productname> version 8.2, the syntax
    <literal>CREATE TYPE <replaceable>name</></literal> did not exist.
    The way to create a new base type was to create its input function first.
index 8b34d68..43b4937 100644 (file)
@@ -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,
index 8360149..7419b36 100644 (file)
@@ -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;
+}
index eecd494..ee65805 100644 (file)
@@ -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!
index 915c669..5402cde 100644 (file)
@@ -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);
index c663ff4..0d9ff0d 100644 (file)
@@ -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 */
index 643ae7f..936db41 100644 (file)
@@ -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;
index 1f574a0..a60bf56 100644 (file)
@@ -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;