Re: updated WIP: arrays of composites - Mailing list pgsql-patches

Andrew Dunstan <andrew@dunslane.net> writes:
> Tom Lane wrote:
>> Auto-rename.  I'm working on a patch now, and it doesn't look like it'll
>> be too awful.  Will post it for comments when it's working.

> Ok, cool. I look forward to it.

Here's a bare-bones patch (no doc or regression tests).  Seems to work.
Anyone think this is too ugly a way to proceed?

            regards, tom lane

Index: src/backend/catalog/heap.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/catalog/heap.c,v
retrieving revision 1.319
diff -c -r1.319 heap.c
*** src/backend/catalog/heap.c    11 May 2007 17:57:11 -0000    1.319
--- src/backend/catalog/heap.c    11 May 2007 23:18:51 -0000
***************
*** 797,802 ****
--- 797,803 ----
  {
      Relation    pg_class_desc;
      Relation    new_rel_desc;
+     Oid            old_type_oid;
      Oid            new_type_oid;
      Oid         new_array_oid = InvalidOid;

***************
*** 815,820 ****
--- 816,842 ----
                   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,868 ****
       * 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.
       */
      new_type_oid = AddNewRelationType(relname,
                                        relnamespace,
--- 883,891 ----
       * 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 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: src/backend/catalog/pg_type.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/catalog/pg_type.c,v
retrieving revision 1.112
diff -c -r1.112 pg_type.c
*** src/backend/catalog/pg_type.c    11 May 2007 17:57:12 -0000    1.112
--- src/backend/catalog/pg_type.c    11 May 2007 23:18:51 -0000
***************
*** 15,20 ****
--- 15,21 ----
  #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,31 ****
--- 27,33 ----
  #include "utils/acl.h"
  #include "utils/builtins.h"
  #include "utils/fmgroids.h"
+ #include "utils/lsyscache.h"
  #include "utils/syscache.h"


***************
*** 551,580 ****

  /*
   * TypeRename
!  *        This renames a 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.
   */
  void
! TypeRename(const char *oldTypeName, Oid typeNamespace,
!            const char *newTypeName)
  {
      Relation    pg_type_desc;
      HeapTuple    tuple;

      pg_type_desc = heap_open(TypeRelationId, RowExclusiveLock);

!     tuple = SearchSysCacheCopy(TYPENAMENSP,
!                                CStringGetDatum(oldTypeName),
!                                ObjectIdGetDatum(typeNamespace),
!                                0, 0);
      if (!HeapTupleIsValid(tuple))
!         ereport(ERROR,
!                 (errcode(ERRCODE_UNDEFINED_OBJECT),
!                  errmsg("type \"%s\" does not exist", oldTypeName)));

      if (SearchSysCacheExists(TYPENAMENSP,
                               CStringGetDatum(newTypeName),
                               ObjectIdGetDatum(typeNamespace),
--- 553,587 ----

  /*
   * TypeRename
!  *        This renames a type, as well as any associated array type.
   *
!  * 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(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(TYPEOID,
!                                ObjectIdGetDatum(typeOid),
!                                0, 0, 0);
      if (!HeapTupleIsValid(tuple))
!         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,589 ****
                  (errcode(ERRCODE_DUPLICATE_OBJECT),
                   errmsg("type \"%s\" already exists", newTypeName)));

!     namestrcpy(&(((Form_pg_type) GETSTRUCT(tuple))->typname), newTypeName);

      simple_heap_update(pg_type_desc, &tuple->t_self, tuple);

--- 590,597 ----
                  (errcode(ERRCODE_DUPLICATE_OBJECT),
                   errmsg("type \"%s\" already exists", 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,602 ****

      heap_freetuple(tuple);
      heap_close(pg_type_desc, RowExclusiveLock);
  }


  /*
!  * makeArrayTypeName(typeName)
   *      - given a base type name, make an array type name for it
   *
   * the caller is responsible for pfreeing the result
--- 600,619 ----

      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
   *      - given a base type name, make an array type name for it
   *
   * the caller is responsible for pfreeing the result
***************
*** 638,640 ****
--- 655,712 ----

      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.
+  */
+ bool
+ moveArrayTypeName(Oid typeOid, const char *typeName, Oid typeNamespace)
+ {
+     Oid            elemOid;
+     char       *newname;
+
+     /*
+      * 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: src/backend/commands/tablecmds.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.221
diff -c -r1.221 tablecmds.c
*** src/backend/commands/tablecmds.c    11 May 2007 20:16:36 -0000    1.221
--- src/backend/commands/tablecmds.c    11 May 2007 23:18:52 -0000
***************
*** 1626,1631 ****
--- 1626,1632 ----
      Relation    targetrelation;
      Relation    relrelation;    /* for RELATION relation */
      HeapTuple    reltup;
+     Form_pg_class relform;
      Oid            namespaceId;
      char       *oldrelname;
      char        relkind;
***************
*** 1655,1664 ****
      relrelation = heap_open(RelationRelationId, RowExclusiveLock);

      reltup = SearchSysCacheCopy(RELOID,
!                                 PointerGetDatum(myrelid),
                                  0, 0, 0);
      if (!HeapTupleIsValid(reltup))        /* shouldn't happen */
          elog(ERROR, "cache lookup failed for relation %u", myrelid);

      if (get_relname_relid(newrelname, namespaceId) != InvalidOid)
          ereport(ERROR,
--- 1656,1666 ----
      relrelation = heap_open(RelationRelationId, RowExclusiveLock);

      reltup = SearchSysCacheCopy(RELOID,
!                                 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,1676 ****
       * 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);

      simple_heap_update(relrelation, &reltup->t_self, reltup);

--- 1672,1678 ----
       * Update pg_class tuple with new relname.    (Scribbling on reltup is OK
       * because it's a copy...)
       */
!     namestrcpy(&(relform->relname), newrelname);

      simple_heap_update(relrelation, &reltup->t_self, reltup);

***************
*** 1683,1690 ****
      /*
       * Also rename the associated type, if any.
       */
!     if (relkind != RELKIND_INDEX)
!         TypeRename(oldrelname, namespaceId, newrelname);

      /*
       * Close rel, but keep exclusive lock!
--- 1685,1692 ----
      /*
       * Also rename the associated type, if any.
       */
!     if (OidIsValid(targetrelation->rd_rel->reltype))
!         TypeRename(targetrelation->rd_rel->reltype, newrelname, namespaceId);

      /*
       * Close rel, but keep exclusive lock!
Index: src/backend/commands/typecmds.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/typecmds.c,v
retrieving revision 1.103
diff -c -r1.103 typecmds.c
*** src/backend/commands/typecmds.c    11 May 2007 20:16:54 -0000    1.103
--- src/backend/commands/typecmds.c    11 May 2007 23:18:52 -0000
***************
*** 137,149 ****

      /*
       * 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.
       */
      typoid = GetSysCacheOid(TYPENAMENSP,
                              CStringGetDatum(typeName),
                              ObjectIdGetDatum(typeNamespace),
                              0, 0);
      if (!OidIsValid(typoid))
      {
          typoid = TypeShellMake(typeName, typeNamespace);
--- 137,163 ----

      /*
       * Look to see if type already exists (presumably as a shell; if not,
!      * 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,607 ****
--- 616,622 ----
      ListCell   *listptr;
      Oid            basetypeoid;
      Oid            domainoid;
+     Oid            old_type_oid;
      Form_pg_type baseType;
      int32        basetypeMod;

***************
*** 617,622 ****
--- 632,653 ----
                         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,953 ****
--- 979,985 ----
      Oid        enumNamespace;
      Oid        enumTypeOid;
      AclResult    aclresult;
+     Oid        old_type_oid;
      Oid     enumArrayOid;
      Relation pg_type;

***************
*** 961,966 ****
--- 993,1014 ----
          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: src/include/catalog/pg_type.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/catalog/pg_type.h,v
retrieving revision 1.183
diff -c -r1.183 pg_type.h
*** src/include/catalog/pg_type.h    11 May 2007 17:57:13 -0000    1.183
--- src/include/catalog/pg_type.h    11 May 2007 23:18:52 -0000
***************
*** 645,653 ****
                           Node *defaultExpr,
                           bool rebuild);

! extern void TypeRename(const char *oldTypeName, Oid typeNamespace,
!            const char *newTypeName);

  extern char *makeArrayTypeName(const char *typeName, Oid typeNamespace);

  #endif   /* PG_TYPE_H */
--- 645,656 ----
                           Node *defaultExpr,
                           bool rebuild);

! 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 */