Re: range_agg

On Thu, Dec 17, 2020 at 12:54 AM Zhihong Yu <zyu(at)yugabyte(dot)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