On Fri, Jun 24, 2022 at 2:12 PM Andrey Lepikhov
<a.lepikhov@postgrespro.ru> wrote:
>
> Moved from the pgsql-bugs mailing list [1].
>
> On 6/23/22 07:03, Masahiko Sawada wrote:
> > Hi,
> >
> > On Sat, Jun 4, 2022 at 4:03 AM Andrey Lepikhov
> > <a.lepikhov@postgrespro.ru> wrote:
> >>
> >> According to subj you can try to create many tables (induced by the case
> >> of partitioned table) with long prefix - see 6727v.sql for reproduction.
> >> But now it's impossible because of logic of the makeUniqueTypeName()
> >> routine.
> >> You get the error:
> >> ERROR: could not form array type name for type ...
> >>
> >> 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.
>
> > 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".
Regards,
--
Masahiko Sawada
EDB: https://www.enterprisedb.com/