Thread: range_agg

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

On Thu, Dec 17, 2020 at 1:03 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> 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
callingtruncate_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.

I gave this another thought.  Now we have facility to name multirange
types manually.  I think we should give up with underscore naming
completely.  If both replacing "range" with "mutlirange" in the
typename and appending "_multirange" to the type name failed (very
unlikely), then let user manually name the multirange.  Any thoughts?

------
Regards,
Alexander Korotkov

On Thu, Dec 17, 2020 at 2:37 AM Zhihong Yu <zyu@yugabyte.com> wrote:
> Letting user manually name the multirange (after a few automatic attempts) seems reasonable.

Accepted.  Thank you for your feedback.

------
Regards,
Alexander Korotkov