Skip to content

Commit 9aa3c78

Browse files
tglsfdctglsfdc
committed
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.
1 parent d832611 commit 9aa3c78

File tree

8 files changed

+251
-43
lines changed

8 files changed

+251
-43
lines changed

‎doc/src/sgml/ref/create_type.sgml

+22-13
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<!--
2-
$PostgreSQL: pgsql/doc/src/sgml/ref/create_type.sgml,v 1.70 2007/05/11 17:57:11 tgl Exp $
2+
$PostgreSQL: pgsql/doc/src/sgml/ref/create_type.sgml,v 1.71 2007/05/12 00:54:59 tgl Exp $
33
PostgreSQL documentation
44
-->
55

@@ -529,18 +529,6 @@ CREATE TYPE <replaceable class="parameter">name</replaceable>
529529
<refsect1 id="SQL-CREATETYPE-notes">
530530
<title>Notes</title>
531531

532-
<para>
533-
It is best to avoid using type names that begin with the underscore
534-
character (<literal>_</literal>). <productname>PostgreSQL</productname>
535-
forms the name of an array type by prepending one or more underscores
536-
to the element type's name, and these names may collide with user-defined
537-
type names that begin with underscore. While the system will modify
538-
generated array type names to avoid collisions, this does not help if the
539-
conflicting array type already exists when you try to create your type.
540-
Also, various old client software may assume that names beginning with
541-
underscores always represent arrays.
542-
</para>
543-
544532
<para>
545533
Because there are no restrictions on use of a data type once it's been
546534
created, creating a base type is tantamount to granting public execute
@@ -552,6 +540,27 @@ CREATE TYPE <replaceable class="parameter">name</replaceable>
552540
while converting it to or from external form.
553541
</para>
554542

543+
<para>
544+
Before <productname>PostgreSQL</productname> version 8.3, the name of
545+
a generated array type was always exactly the element type's name with one
546+
underscore character (<literal>_</literal>) prepended. (Type names were
547+
therefore restricted in length to one less character than other names.)
548+
While this is still usually the case, the array type name may vary from
549+
this in case of maximum-length names or collisions with user type names
550+
that begin with underscore. Writing code that depends on this convention
551+
is therefore deprecated. Instead, use
552+
<structname>pg_type</>.<structfield>typarray</> to locate the array type
553+
associated with a given type.
554+
</para>
555+
556+
<para>
557+
It may be advisable to avoid using type and table names that begin with
558+
underscore. While the server will change generated array type names to
559+
avoid collisions with user-given names, there is still risk of confusion,
560+
particularly with old client software that may assume that type names
561+
beginning with underscores always represent arrays.
562+
</para>
563+
555564
<para>
556565
Before <productname>PostgreSQL</productname> version 8.2, the syntax
557566
<literal>CREATE TYPE <replaceable>name</></literal> did not exist.

‎src/backend/catalog/heap.c

+26-3
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.319 2007/05/11 17:57:11 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.320 2007/05/12 00:54:59 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -797,6 +797,7 @@ heap_create_with_catalog(const char *relname,
797797
{
798798
Relation pg_class_desc;
799799
Relation new_rel_desc;
800+
Oid old_type_oid;
800801
Oid new_type_oid;
801802
Oid new_array_oid = InvalidOid;
802803

@@ -814,6 +815,27 @@ heap_create_with_catalog(const char *relname,
814815
(errcode(ERRCODE_DUPLICATE_TABLE),
815816
errmsg("relation \"%s\" already exists", relname)));
816817

818+
/*
819+
* Since we are going to create a rowtype as well, also check for
820+
* collision with an existing type name. If there is one and it's
821+
* an autogenerated array, we can rename it out of the way; otherwise
822+
* we can at least give a good error message.
823+
*/
824+
old_type_oid = GetSysCacheOid(TYPENAMENSP,
825+
CStringGetDatum(relname),
826+
ObjectIdGetDatum(relnamespace),
827+
0, 0);
828+
if (OidIsValid(old_type_oid))
829+
{
830+
if (!moveArrayTypeName(old_type_oid, relname, relnamespace))
831+
ereport(ERROR,
832+
(errcode(ERRCODE_DUPLICATE_OBJECT),
833+
errmsg("type \"%s\" already exists", relname),
834+
errhint("A relation has an associated type of the same name, "
835+
"so you must use a name that doesn't conflict "
836+
"with any existing type.")));
837+
}
838+
817839
/*
818840
* Allocate an OID for the relation, unless we were told what to use.
819841
*
@@ -861,8 +883,9 @@ heap_create_with_catalog(const char *relname,
861883
* Since defining a relation also defines a complex type, we add a new
862884
* system type corresponding to the new relation.
863885
*
864-
* NOTE: we could get a unique-index failure here, in case the same name
865-
* has already been used for a type.
886+
* NOTE: we could get a unique-index failure here, in case someone else is
887+
* creating the same type name in parallel but hadn't committed yet
888+
* when we checked for a duplicate name above.
866889
*/
867890
new_type_oid = AddNewRelationType(relname,
868891
relnamespace,

‎src/backend/catalog/pg_type.c

+96-16
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,14 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/catalog/pg_type.c,v 1.112 2007/05/11 17:57:12 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/catalog/pg_type.c,v 1.113 2007/05/12 00:54:59 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
1515
#include "postgres.h"
1616

1717
#include "access/heapam.h"
18+
#include "access/xact.h"
1819
#include "catalog/dependency.h"
1920
#include "catalog/indexing.h"
2021
#include "catalog/pg_namespace.h"
@@ -26,6 +27,7 @@
2627
#include "utils/acl.h"
2728
#include "utils/builtins.h"
2829
#include "utils/fmgroids.h"
30+
#include "utils/lsyscache.h"
2931
#include "utils/syscache.h"
3032

3133

@@ -551,30 +553,35 @@ GenerateTypeDependencies(Oid typeNamespace,
551553

552554
/*
553555
* TypeRename
554-
* This renames a type
556+
* This renames a type, as well as any associated array type.
555557
*
556-
* Note: any associated array type is *not* renamed; caller must make
557-
* another call to handle that case. Currently this is only used for
558-
* renaming types associated with tables, for which there are no arrays.
558+
* Note: this isn't intended to be a user-exposed function; it doesn't check
559+
* permissions etc. (Perhaps TypeRenameInternal would be a better name.)
560+
* Currently this is only used for renaming table rowtypes.
559561
*/
560562
void
561-
TypeRename(const char *oldTypeName, Oid typeNamespace,
562-
const char *newTypeName)
563+
TypeRename(Oid typeOid, const char *newTypeName, Oid typeNamespace)
563564
{
564565
Relation pg_type_desc;
565566
HeapTuple tuple;
567+
Form_pg_type typ;
568+
Oid arrayOid;
566569

567570
pg_type_desc = heap_open(TypeRelationId, RowExclusiveLock);
568571

569-
tuple = SearchSysCacheCopy(TYPENAMENSP,
570-
CStringGetDatum(oldTypeName),
571-
ObjectIdGetDatum(typeNamespace),
572-
0, 0);
572+
tuple = SearchSysCacheCopy(TYPEOID,
573+
ObjectIdGetDatum(typeOid),
574+
0, 0, 0);
573575
if (!HeapTupleIsValid(tuple))
574-
ereport(ERROR,
575-
(errcode(ERRCODE_UNDEFINED_OBJECT),
576-
errmsg("type \"%s\" does not exist", oldTypeName)));
576+
elog(ERROR, "cache lookup failed for type %u", typeOid);
577+
typ = (Form_pg_type) GETSTRUCT(tuple);
578+
579+
/* We are not supposed to be changing schemas here */
580+
Assert(typeNamespace == typ->typnamespace);
577581

582+
arrayOid = typ->typarray;
583+
584+
/* Just to give a more friendly error than unique-index violation */
578585
if (SearchSysCacheExists(TYPENAMENSP,
579586
CStringGetDatum(newTypeName),
580587
ObjectIdGetDatum(typeNamespace),
@@ -583,7 +590,8 @@ TypeRename(const char *oldTypeName, Oid typeNamespace,
583590
(errcode(ERRCODE_DUPLICATE_OBJECT),
584591
errmsg("type \"%s\" already exists", newTypeName)));
585592

586-
namestrcpy(&(((Form_pg_type) GETSTRUCT(tuple))->typname), newTypeName);
593+
/* OK, do the rename --- tuple is a copy, so OK to scribble on it */
594+
namestrcpy(&(typ->typname), newTypeName);
587595

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

@@ -592,11 +600,20 @@ TypeRename(const char *oldTypeName, Oid typeNamespace,
592600

593601
heap_freetuple(tuple);
594602
heap_close(pg_type_desc, RowExclusiveLock);
603+
604+
/* If the type has an array type, recurse to handle that */
605+
if (OidIsValid(arrayOid))
606+
{
607+
char *arrname = makeArrayTypeName(newTypeName, typeNamespace);
608+
609+
TypeRename(arrayOid, arrname, typeNamespace);
610+
pfree(arrname);
611+
}
595612
}
596613

597614

598615
/*
599-
* makeArrayTypeName(typeName)
616+
* makeArrayTypeName
600617
* - given a base type name, make an array type name for it
601618
*
602619
* the caller is responsible for pfreeing the result
@@ -638,3 +655,66 @@ makeArrayTypeName(const char *typeName, Oid typeNamespace)
638655

639656
return arr;
640657
}
658+
659+
660+
/*
661+
* moveArrayTypeName
662+
* - try to reassign an array type name that the user wants to use.
663+
*
664+
* The given type name has been discovered to already exist (with the given
665+
* OID). If it is an autogenerated array type, change the array type's name
666+
* to not conflict. This allows the user to create type "foo" followed by
667+
* type "_foo" without problems. (Of course, there are race conditions if
668+
* two backends try to create similarly-named types concurrently, but the
669+
* worst that can happen is an unnecessary failure --- anything we do here
670+
* will be rolled back if the type creation fails due to conflicting names.)
671+
*
672+
* Note that this must be called *before* calling makeArrayTypeName to
673+
* determine the new type's own array type name; else the latter will
674+
* certainly pick the same name.
675+
*
676+
* Returns TRUE if successfully moved the type, FALSE if not.
677+
*
678+
* We also return TRUE if the given type is a shell type. In this case
679+
* the type has not been renamed out of the way, but nonetheless it can
680+
* be expected that TypeCreate will succeed. This behavior is convenient
681+
* for most callers --- those that need to distinguish the shell-type case
682+
* must do their own typisdefined test.
683+
*/
684+
bool
685+
moveArrayTypeName(Oid typeOid, const char *typeName, Oid typeNamespace)
686+
{
687+
Oid elemOid;
688+
char *newname;
689+
690+
/* We need do nothing if it's a shell type. */
691+
if (!get_typisdefined(typeOid))
692+
return true;
693+
694+
/* Can't change it if it's not an autogenerated array type. */
695+
elemOid = get_element_type(typeOid);
696+
if (!OidIsValid(elemOid) ||
697+
get_array_type(elemOid) != typeOid)
698+
return false;
699+
700+
/*
701+
* OK, use makeArrayTypeName to pick an unused modification of the
702+
* name. Note that since makeArrayTypeName is an iterative process,
703+
* this will produce a name that it might have produced the first time,
704+
* had the conflicting type we are about to create already existed.
705+
*/
706+
newname = makeArrayTypeName(typeName, typeNamespace);
707+
708+
/* Apply the rename */
709+
TypeRename(typeOid, newname, typeNamespace);
710+
711+
/*
712+
* We must bump the command counter so that any subsequent use of
713+
* makeArrayTypeName sees what we just did and doesn't pick the same name.
714+
*/
715+
CommandCounterIncrement();
716+
717+
pfree(newname);
718+
719+
return true;
720+
}

‎src/backend/commands/tablecmds.c

+7-5
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.221 2007/05/11 20:16:36 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.222 2007/05/12 00:54:59 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -1626,6 +1626,7 @@ renamerel(Oid myrelid, const char *newrelname)
16261626
Relation targetrelation;
16271627
Relation relrelation; /* for RELATION relation */
16281628
HeapTuple reltup;
1629+
Form_pg_class relform;
16291630
Oid namespaceId;
16301631
char *oldrelname;
16311632
char relkind;
@@ -1655,10 +1656,11 @@ renamerel(Oid myrelid, const char *newrelname)
16551656
relrelation = heap_open(RelationRelationId, RowExclusiveLock);
16561657

16571658
reltup = SearchSysCacheCopy(RELOID,
1658-
PointerGetDatum(myrelid),
1659+
ObjectIdGetDatum(myrelid),
16591660
0, 0, 0);
16601661
if (!HeapTupleIsValid(reltup)) /* shouldn't happen */
16611662
elog(ERROR, "cache lookup failed for relation %u", myrelid);
1663+
relform = (Form_pg_class) GETSTRUCT(reltup);
16621664

16631665
if (get_relname_relid(newrelname, namespaceId) != InvalidOid)
16641666
ereport(ERROR,
@@ -1670,7 +1672,7 @@ renamerel(Oid myrelid, const char *newrelname)
16701672
* Update pg_class tuple with new relname. (Scribbling on reltup is OK
16711673
* because it's a copy...)
16721674
*/
1673-
namestrcpy(&(((Form_pg_class) GETSTRUCT(reltup))->relname), newrelname);
1675+
namestrcpy(&(relform->relname), newrelname);
16741676

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

@@ -1683,8 +1685,8 @@ renamerel(Oid myrelid, const char *newrelname)
16831685
/*
16841686
* Also rename the associated type, if any.
16851687
*/
1686-
if (relkind != RELKIND_INDEX)
1687-
TypeRename(oldrelname, namespaceId, newrelname);
1688+
if (OidIsValid(targetrelation->rd_rel->reltype))
1689+
TypeRename(targetrelation->rd_rel->reltype, newrelname, namespaceId);
16881690

16891691
/*
16901692
* Close rel, but keep exclusive lock!

0 commit comments

Comments
 (0)