range_agg
Hi,
w.r.t. patch v27.
+ * 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().
+ 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) ?
Thanks
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 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.
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
Letting user manually name the multirange (after a few automatic attempts)
seems reasonable.
Cheers
On Wed, Dec 16, 2020 at 3:34 PM Alexander Korotkov <aekorotkov@gmail.com>
wrote:
Show quoted text
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 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.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