Improve makeArrayTypeName's algorithm for choosing array type names.

tglsfdc · tglsfdc · commit 70988b7b0a0b · 2022-07-26T15:38:09.000-04:00
As before, we start by prepending one underscore (truncating the base name if necessary). But if there is a conflict, then instead of prepending more and more underscores, append an underscore and some digits, in much the same way that ChooseRelationName does. While the previous logic could be driven to fail by creating a lot of types with long names differing only near the end, this version seems certain enough to eventually succeed that we can remove the failure code path that was there before. While at it, undo 6df7a96's decision to split this code out of makeArrayTypeName. That wasn't actually accomplishing anything, because no other function was using it --- and it would have been wrong to do so. The convention that a prefix "_" means an array, not something else, is too ancient to mess with. Andrey Lepikhov and Dmitry Koval, reviewed by Masahiko Sawada and myself Discussion: https://postgr.es/m/b84cd82c-cc67-198a-8b1c-60f44e1259ad@postgrespro.ru
diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
@@ -26,6 +26,7 @@
 #include "catalog/pg_namespace.h"
 #include "catalog/pg_proc.h"
 #include "catalog/pg_type.h"
+#include "commands/defrem.h"
 #include "commands/typecmds.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
@@ -37,9 +38,6 @@
 #include "utils/rel.h"
 #include "utils/syscache.h"
 
-static char *makeUniqueTypeName(const char *typeName, Oid typeNamespace,
-								bool tryOriginal);
-
 /* Potentially set by pg_upgrade_support functions */
 Oid			binary_upgrade_next_pg_type_oid = InvalidOid;
 
@@ -815,16 +813,41 @@ RenameTypeInternal(Oid typeOid, const char *newTypeName, Oid typeNamespace)
 char *
 makeArrayTypeName(const char *typeName, Oid typeNamespace)
 {
-	char	   *arr;
+	char	   *arr_name;
+	int			pass = 0;
+	char		suffix[NAMEDATALEN];
 
-	arr = makeUniqueTypeName(typeName, typeNamespace, false);
-	if (arr == NULL)
-		ereport(ERROR,
-				(errcode(ERRCODE_DUPLICATE_OBJECT),
-				 errmsg("could not form array type name for type \"%s\"",
-						typeName)));
+	/*
+	 * Per ancient Postgres tradition, array type names are made by prepending
+	 * an underscore to the base type name.  Much client code knows that
+	 * convention, so don't muck with it.  However, the tradition is less
+	 * clear about what to do in the corner cases where the resulting name is
+	 * too long or conflicts with an existing name.  Our current rules are (1)
+	 * truncate the base name on the right as needed, and (2) if there is a
+	 * conflict, append another underscore and some digits chosen to make it
+	 * unique.  This is similar to what ChooseRelationName() does.
+	 *
+	 * The actual name generation can be farmed out to makeObjectName() by
+	 * giving it an empty first name component.
+	 */
+
+	/* First, try with no numeric suffix */
+	arr_name = makeObjectName("", typeName, NULL);
+
+	for (;;)
+	{
+		if (!SearchSysCacheExists2(TYPENAMENSP,
+								   CStringGetDatum(arr_name),
+								   ObjectIdGetDatum(typeNamespace)))
+			break;
+
+		/* That attempt conflicted.  Prepare a new name with some digits. */
+		pfree(arr_name);
+		snprintf(suffix, sizeof(suffix), "%d", ++pass);
+		arr_name = makeObjectName("", typeName, suffix);
+	}
 
-	return arr;
+	return arr_name;
 }
 
 
@@ -931,48 +954,3 @@ makeMultirangeTypeName(const char *rangeTypeName, Oid typeNamespace)
 
 	return pstrdup(buf);
 }
-
-/*
- * makeUniqueTypeName
- *		Generate a unique name for a prospective new type
- *
- * Given a typeName, return a new palloc'ed name by prepending underscores
- * until a non-conflicting name results.
- *
- * If tryOriginal, first try with zero underscores.
- */
-static char *
-makeUniqueTypeName(const char *typeName, Oid typeNamespace, bool tryOriginal)
-{
-	int			i;
-	int			namelen;
-	char		dest[NAMEDATALEN];
-
-	Assert(strlen(typeName) <= NAMEDATALEN - 1);
-
-	if (tryOriginal &&
-		!SearchSysCacheExists2(TYPENAMENSP,
-							   CStringGetDatum(typeName),
-							   ObjectIdGetDatum(typeNamespace)))
-		return pstrdup(typeName);
-
-	/*
-	 * The idea is to prepend underscores as needed until we make a name that
-	 * doesn't collide with anything ...
-	 */
-	namelen = strlen(typeName);
-	for (i = 1; i < NAMEDATALEN - 1; i++)
-	{
-		dest[i - 1] = '_';
-		strlcpy(dest + i, typeName, NAMEDATALEN - i);
-		if (namelen + i >= NAMEDATALEN)
-			truncate_identifier(dest, NAMEDATALEN, false);
-
-		if (!SearchSysCacheExists2(TYPENAMENSP,
-								   CStringGetDatum(dest),
-								   ObjectIdGetDatum(typeNamespace)))
-			return pstrdup(dest);
-	}
-
-	return NULL;
-}
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
@@ -197,9 +197,9 @@ SELECT typname FROM pg_type WHERE oid = 'attmp_array[]'::regtype;
 (1 row)
 
 SELECT typname FROM pg_type WHERE oid = '_attmp_array[]'::regtype;
-    typname     
-----------------
- ___attmp_array
+     typname     
+-----------------
+ __attmp_array_1
 (1 row)
 
 DROP TABLE _attmp_array;
