On 6/27/22 06:38, Masahiko Sawada wrote:
On Fri, Jun 24, 2022 at 2:12 PM Andrey Lepikhov
<a.lepik...@postgrespro.ru> wrote:
On 6/23/22 07:03, Masahiko Sawada wrote:
> On Sat, Jun 4, 2022 at 4:03 AM Andrey Lepikhov
> <a.lepik...@postgrespro.ru> wrote:
>> It is very corner case, of course. But solution is easy and short. So,
>> why not to fix? - See the patch in attachment.
>
> While this seems to be a good improvement, I think it's not a bug.
> Probably we cannot backpatch it as it will end up having type names
> defined by different naming rules. I'd suggest discussing it on
> -hackers.
Done.
Thank for updating the patch. Please register this item to the next CF
if not yet.
Done [1].
> Regarding the patch, I think we can merge makeUniqueTypeName() to
> makeArrayTypeName() as there is no caller of makeUniqueTypeName() who
> pass tryOriginal = true.
I partially agree with you. But I have one reason to leave
makeUniqueTypeName() separated:
It may be used in other codes with auto generated types. For example, I
think, the DefineRelation routine should choose composite type instead
of using the same name as the table.
Okay.
I have one comment on v2 patch:
+ for(;;)
{
- dest[i - 1] = '_';
- strlcpy(dest + i, typeName, NAMEDATALEN - i);
- if (namelen + i >= NAMEDATALEN)
- truncate_identifier(dest, NAMEDATALEN, false);
-
if (!SearchSysCacheExists2(TYPENAMENSP,
- CStringGetDatum(dest),
+ CStringGetDatum(type_name),
ObjectIdGetDatum(typeNamespace)))
- return pstrdup(dest);
+ return type_name;
+
+ /* Previous attempt was failed. Prepare a new one. */
+ pfree(type_name);
+ snprintf(suffix, sizeof(suffix), "%d", ++pass);
+ type_name = makeObjectName("", typeName, suffix);
}
return NULL;
I think it's better to break from the loop instead of returning from
there. That way, we won't need "return NULL".
Agree. Done.
[1] https://commitfest.postgresql.org/38/3712/
--
Regards
Andrey Lepikhov
Postgres Professional
From 73b80676fff98e9edadab2576cf22778c6b5168b Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" <a.lepik...@postgrespro.ru>
Date: Fri, 3 Jun 2022 21:40:01 +0300
Subject: [PATCH] Allow postgresql to generate more relations with the same 63
symbols long prefix. Rewrite the makeUniqueTypeName routine - generator of
unique name is based on the same idea as the ChooseRelationName() routine.
Authors: Dmitry Koval, Andrey Lepikhov
---
src/backend/catalog/pg_type.c | 37 ++++++++++++-----------
src/test/regress/expected/alter_table.out | 6 ++--
2 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
index 03788cb975..405553013e 100644
--- 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"
@@ -936,17 +937,17 @@ makeMultirangeTypeName(const char *rangeTypeName, Oid typeNamespace)
* 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.
+ * Given a typeName, return a new palloc'ed name by prepending underscore
+ * and (if needed) adding a suffix to the end of the type name.
*
* If tryOriginal, first try with zero underscores.
*/
static char *
makeUniqueTypeName(const char *typeName, Oid typeNamespace, bool tryOriginal)
{
- int i;
- int namelen;
- char dest[NAMEDATALEN];
+ int pass = 0;
+ char suffix[NAMEDATALEN];
+ char *type_name;
Assert(strlen(typeName) <= NAMEDATALEN - 1);
@@ -956,23 +957,25 @@ makeUniqueTypeName(const char *typeName, Oid typeNamespace, bool tryOriginal)
ObjectIdGetDatum(typeNamespace)))
return pstrdup(typeName);
+ /* Prepare initial object name. Just for compatibility. */
+ type_name = makeObjectName("", typeName, NULL);
+
/*
- * The idea is to prepend underscores as needed until we make a name that
- * doesn't collide with anything ...
+ * The idea of unique name generation is similar to ChooseRelationName()
+ * implementation logic.
*/
- namelen = strlen(typeName);
- for (i = 1; i < NAMEDATALEN - 1; i++)
+ for(;;)
{
- dest[i - 1] = '_';
- strlcpy(dest + i, typeName, NAMEDATALEN - i);
- if (namelen + i >= NAMEDATALEN)
- truncate_identifier(dest, NAMEDATALEN, false);
-
if (!SearchSysCacheExists2(TYPENAMENSP,
- CStringGetDatum(dest),
+ CStringGetDatum(type_name),
ObjectIdGetDatum(typeNamespace)))
- return pstrdup(dest);
+ break;
+
+ /* Previous attempt was failed. Prepare a new one. */
+ pfree(type_name);
+ snprintf(suffix, sizeof(suffix), "%d", ++pass);
+ type_name = makeObjectName("", typeName, suffix);
}
- return NULL;
+ return type_name;
}
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 5ede56d9b5..e634dbd86d 100644
--- 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;
--
2.34.1
