Re: unnesting multirange data types

On 6/13/21 10:57 AM, Zhihong Yu wrote:
> 
> 
> On Sat, Jun 12, 2021 at 4:58 PM Alexander Korotkov <aekorotkov@gmail.com
> <mailto:aekorotkov@gmail.com>> wrote:
> 
>     On Sun, Jun 13, 2021 at 1:16 AM Jonathan S. Katz
>     <jkatz@postgresql.org <mailto:jkatz@postgresql.org>> wrote:
>     > On 6/12/21 5:57 PM, Alexander Korotkov wrote:
>     > > On Sat, Jun 12, 2021 at 2:44 AM Alexander Korotkov
>     <aekorotkov@gmail.com <mailto:aekorotkov@gmail.com>> wrote:
>     > >> ()On Sat, Jun 12, 2021 at 2:30 AM Justin Pryzby
>     <pryzby@telsasoft.com <mailto:pryzby@telsasoft.com>> wrote:
>     > >>> On Fri, Jun 11, 2021 at 11:37:58PM +0300, Alexander Korotkov
>     wrote:
>     > >>>> On Fri, Jun 11, 2021 at 1:04 AM Justin Pryzby
>     <pryzby@telsasoft.com <mailto:pryzby@telsasoft.com>> wrote:
>     > >>>>>
>     > >>>>> +{ oid => '1293', descr => 'expand mutlirange to set of ranges',
>     > >>>>>
>     > >>>>> typo: mutlirange
>     > >>>>
>     > >>>> Fixed, thanks.
>     > >>>>
>     > >>>> The patch with the implementation of both unnest() and cast
>     to array
>     > >>>> is attached.  It contains both tests and docs.
>     > >>>
>     > >>> |+   The multirange could be explicitly cast to the array of
>     corresponding
>     > >>> should say: "can be cast to an array of corresponding.."
>     > >>>
>     > >>> |+ * Cast multirange to the array of ranges.
>     > >>> I think should be: *an array of ranges
>     > >>
>     > >> Thank you for catching this.
>     > >>
>     > >>> Per sqlsmith, this is causing consistent crashes.
>     > >>> I took one of its less appalling queries and simplified it to
>     this:
>     > >>>
>     > >>> select
>     > >>> pg_catalog.multirange_to_array(
>     > >>>     cast(pg_catalog.int8multirange() as int8multirange)) as c2
>     > >>> from (select 1)x;
>     > >>
>     > >> It seems that multirange_to_array() doesn't handle empty
>     multiranges.
>     > >> I'll post an updated version of the patch tomorrow.
>     > >
>     > > A revised patch is attached.  Now empty multiranges are handled
>     > > properly (and it's covered by tests).  Typos are fixed as well.
>     >
>     > Tested both against my original cases using both SQL + PL/pgSQL. All
>     > worked well. I also tested the empty multirange case as well.
>     >
>     > Overall the documentation seems to make sense, I'd suggest:
>     >
>     > +  <para>
>     > +   The multirange can be cast to an array of corresponding ranges.
>     > +  </para>
>     >
>     > becomes:
>     >
>     > +  <para>
>     > +   A multirange can be cast to an array of ranges of the same type.
>     > +  </para>
> 
>     Thank you. This change is incorporated in the attached revision of
>     the patch.
> 
>     This thread gave me another lesson about English articles.  Hopefully,
>     I would be able to make progress in future patches :)
> 
>     > Again, I'll defer to others on the code, but this seems to solve
>     the use
>     > case I presented. Thanks for the quick turnaround!
> 
>     Thank you for the feedback!
> 
>     ------
>     Regards,
>     Alexander Korotkov
> 
> 
> Hi,
> +   A multirange can be cast to an array of ranges of the same type.
> 
> I think 'same type' is not very accurate. It should be 'of the subtype'.

I think that's more technically correct, but it could be confusing to
the user. There is an example next to it that shows how this function
works, i.e. it returns the type of range that is represented by the
multirange.

> +   ObjectAddress myself,
> 
> nit: myself -> self
> 
> +/* Turn multirange into a set of ranges */
> 
> set of ranges: sequence of ranges

I believe "set of ranges" is accurate here, as the comparable return is
a "SETOF rangetype". Sequences are objects unto themselves.

Jonathan