On Thu, Dec 17, 2020 at 12:54 AM Zhihong Yu <zyu@yugabyte.com> wrote:
> + * The idea is to prepend underscores as needed until we make a name that
> + * doesn't collide with anything ...
>
> I wonder if other characters (e.g. [a-z0-9]) can be used so that name without collision can be found without calling
truncate_identifier().
Probably. But multiranges just shares naming logic already existing
in arrays. If we're going to change this, I think we should change
this for arrays too. And this change shouldn't be part of multirange
patch.
> + else if (strcmp(defel->defname, "multirange_type_name") == 0)
> + {
> + if (multirangeTypeName != NULL)
> + ereport(ERROR,
> + (errcode(ERRCODE_SYNTAX_ERROR),
> + errmsg("conflicting or redundant options")));
>
> Maybe make the error message a bit different from occurrences of similar error message (such as including
multirangeTypeName)?
This is again isn't an invention of multirange. We use this message
many times in DefineRange() and other places. From the first glance,
I've nothing against changing this to a more informative message, but
that should be done globally. And this change isn't directly related
to multirage. Feel free to propose a patch improving this.
------
Regards,
Alexander Korotkov