Thread: unnesting multirange data types

Hi,

I have been exploring multirange data types using PostgreSQL 14 Beta 1.
Thus far I'm really happy with the user experience, and it has allowed
me to simplify some previously onerous queries!

I do have a question about trying to "unnest" a multirange type into its
individual ranges. For example, I have a query where I want to find the
availability over a given week. This query may look something like:

  SELECT datemultirange(daterange(CURRENT_DATE, CURRENT_DATE + 7))
    - datemultirange(daterange(CURRENT_DATE + 2, CURRENT_DATE + 4))
    as availability;

                     availability
  ---------------------------------------------------
   {[2021-06-09,2021-06-11),[2021-06-13,2021-06-16)}
  (1 row)

I would like to decompose the returned multirange into its individual
ranges, similarly to how I would "unnest" an array:

  SELECT * FROM unnest(ARRAY[1,2,3]);
   unnest
  --------
        1
        2
        3
  (3 rows)

So something like:

 SELECT unnest('{[2021-06-09,2021-06-11),
                 [2021-06-13,2021-06-16)}')::datemultirange;

           unnest
  -------------------------
   [2021-06-09,2021-06-11)
   [2021-06-13,2021-06-16)
  (2 rows)

I looked at the various functions + operators available for the
multirange types in the documentation but could not find anything that
could perform this action.

Does this functionality exist?

Thanks,

Jonathan

"Jonathan S. Katz" <jkatz@postgresql.org> writes:
> I would like to decompose the returned multirange into its individual
> ranges, similarly to how I would "unnest" an array:

+1 for adding such a feature, but I suppose it's too late for v14.

AFAICS, "unnest(anymultirange) returns setof anyrange" could coexist
alongside the existing variants of unnest(), so I don't see any
fundamental stumbling block to having it.

            regards, tom lane

On 6/9/21 3:25 PM, Tom Lane wrote:
> "Jonathan S. Katz" <jkatz@postgresql.org> writes:
>> I would like to decompose the returned multirange into its individual
>> ranges, similarly to how I would "unnest" an array:
>
> +1 for adding such a feature, but I suppose it's too late for v14.

Well, the case I would make for v14 is that, as of right now, the onus
is on the driver writers / application developers to be able to unpack
the multiranges.

Maybe it's not terrible as of this moment -- I haven't tried testing it
that far yet -- but it may make it a bit more challenging to work with
these types outside of Postgres. I recall a similar issue when initially
trying to integrate range types into my apps back in the v9.2 days, and
I ended up writing some grotty code to handle it. Yes, I worked around
it, but I preferably wouldn't have had to.

An "unnest" at least lets us bridge the gap a bit, i.e. if you really
need to introspect a multirange type, you have a way of getting it into
a familiar format.

I haven't tried manipulating a multirange in a PL like Python, maybe
some exploration there would unveil more or less pain, or if it could be
iterated over in PL/pgSQL (I'm suspecting no).

That all said, for writing queries within Postgres, the multiranges make
a lot of operations way easier. I do think a missing "unnest" function
does straddle the line of "omission" and "new feature," so I can
understand if it does not make it into v14.

> AFAICS, "unnest(anymultirange) returns setof anyrange" could coexist
> alongside the existing variants of unnest(), so I don't see any
> fundamental stumbling block to having it.

Cool. I was initially throwing out "unnest" as the name as it mirrors
what we currently have with arrays, and seems to be doing something
similar. Open to other names, but this was the one that I was drawn to.
"multirange" is an "ordered array of ranges" after all.

Thanks,

Jonathan

On 6/9/21 3:44 PM, Jonathan S. Katz wrote:
> On 6/9/21 3:25 PM, Tom Lane wrote:
>> "Jonathan S. Katz" <jkatz@postgresql.org> writes:
>>> I would like to decompose the returned multirange into its individual
>>> ranges, similarly to how I would "unnest" an array:
>>
>> +1 for adding such a feature, but I suppose it's too late for v14.
>
> Well, the case I would make for v14 is that, as of right now, the onus
> is on the driver writers / application developers to be able to unpack
> the multiranges.
>
> I haven't tried manipulating a multirange in a PL like Python, maybe
> some exploration there would unveil more or less pain, or if it could be
> iterated over in PL/pgSQL (I'm suspecting no).

I did a couple more tests around this.

As suspected, in PL/pgSQL, there is no way to unpack or iterate over a
multirange type.

In PL/Python, both range types and multirange types are treated as
strings. From there, you can at least ultimately parse and manipulate it
into your preferred Python types, but this goes back to my earlier point
about putting the onus on the developer to do so.

Thanks,

Jonathan

On 2021-Jun-09, Jonathan S. Katz wrote:

> I did a couple more tests around this.
> 
> As suspected, in PL/pgSQL, there is no way to unpack or iterate over a
> multirange type.

Uh.  This is disappointing; the need for some way to unnest or unpack a
multirange was mentioned multiple times in the range_agg thread.  I had
assumed that there was some way to cast the multirange to a range array,
or somehow convert it, but apparently that doesn't work.

If the supporting pieces are mostly there, then I opine we should add
something.

-- 
Álvaro Herrera                            39°49'30"S 73°17'W
"Hay dos momentos en la vida de un hombre en los que no debería
especular: cuando puede permitírselo y cuando no puede" (Mark Twain)

On 6/9/21 4:56 PM, Alvaro Herrera wrote:
> On 2021-Jun-09, Jonathan S. Katz wrote:
>
>> I did a couple more tests around this.
>>
>> As suspected, in PL/pgSQL, there is no way to unpack or iterate over a
>> multirange type.
>
> Uh.  This is disappointing; the need for some way to unnest or unpack a
> multirange was mentioned multiple times in the range_agg thread.  I had
> assumed that there was some way to cast the multirange to a range array,
> or somehow convert it, but apparently that doesn't work.

Just to be pedantic with examples:

  SELECT datemultirange(
    daterange(current_date, current_date + 2),
    daterange(current_date + 5, current_date + 7))::daterange[];

  ERROR:  cannot cast type datemultirange to daterange[]
  LINE 1: ...2), daterange(current_date + 5, current_date + 7))::daterang...

IF there was an array to cast it into an array, we could then use the
array looping construct in PL/pgSQL, but if we could only choose one, I
think it'd be more natural/less verbose to have an "unnest".

> If the supporting pieces are mostly there, then I opine we should add
> something.

Agreed.

Jonathan

Hi, all!

On Thu, Jun 10, 2021 at 2:00 AM Jonathan S. Katz <jkatz@postgresql.org> wrote:
> On 6/9/21 4:56 PM, Alvaro Herrera wrote:
> > On 2021-Jun-09, Jonathan S. Katz wrote:
> >
> >> I did a couple more tests around this.
> >>
> >> As suspected, in PL/pgSQL, there is no way to unpack or iterate over a
> >> multirange type.
> >
> > Uh.  This is disappointing; the need for some way to unnest or unpack a
> > multirange was mentioned multiple times in the range_agg thread.  I had
> > assumed that there was some way to cast the multirange to a range array,
> > or somehow convert it, but apparently that doesn't work.
>
> Just to be pedantic with examples:
>
>   SELECT datemultirange(
>     daterange(current_date, current_date + 2),
>     daterange(current_date + 5, current_date + 7))::daterange[];
>
>   ERROR:  cannot cast type datemultirange to daterange[]
>   LINE 1: ...2), daterange(current_date + 5, current_date + 7))::daterang...
>
> IF there was an array to cast it into an array, we could then use the
> array looping construct in PL/pgSQL, but if we could only choose one, I
> think it'd be more natural/less verbose to have an "unnest".
>
> > If the supporting pieces are mostly there, then I opine we should add
> > something.
>
> Agreed.

I agree that unnest(), cast to array and subscription are missing
points.  Proper subscription support requires expanded object
handling.  And that seems too late for v14.  But unnset() and cast to
array seems trivial.  I've drafted unnest support (attached).  I'm
going to add also cast to the array, tests, and docs within a day.
Stay tuned :)

------
Regards,
Alexander Korotkov

On 6/10/21 1:24 PM, Alexander Korotkov wrote:
> Hi, all!
>
> On Thu, Jun 10, 2021 at 2:00 AM Jonathan S. Katz <jkatz@postgresql.org> wrote:
>> On 6/9/21 4:56 PM, Alvaro Herrera wrote:
>>> On 2021-Jun-09, Jonathan S. Katz wrote:
>>>
>>>> I did a couple more tests around this.
>>>>
>>>> As suspected, in PL/pgSQL, there is no way to unpack or iterate over a
>>>> multirange type.
>>>
>>> Uh.  This is disappointing; the need for some way to unnest or unpack a
>>> multirange was mentioned multiple times in the range_agg thread.  I had
>>> assumed that there was some way to cast the multirange to a range array,
>>> or somehow convert it, but apparently that doesn't work.
>>
>> Just to be pedantic with examples:
>>
>>   SELECT datemultirange(
>>     daterange(current_date, current_date + 2),
>>     daterange(current_date + 5, current_date + 7))::daterange[];
>>
>>   ERROR:  cannot cast type datemultirange to daterange[]
>>   LINE 1: ...2), daterange(current_date + 5, current_date + 7))::daterang...
>>
>> IF there was an array to cast it into an array, we could then use the
>> array looping construct in PL/pgSQL, but if we could only choose one, I
>> think it'd be more natural/less verbose to have an "unnest".
>>
>>> If the supporting pieces are mostly there, then I opine we should add
>>> something.
>>
>> Agreed.
>
> I agree that unnest(), cast to array and subscription are missing
> points.  Proper subscription support requires expanded object
> handling.  And that seems too late for v14.

Agreed, the subscripting functionality is too late for v14. (Though
perhaps someone ambitious could bridge that gap temporarily with the
ability to add subscripting to types!).

>  But unnset() and cast to
> array seems trivial.  I've drafted unnest support (attached).  I'm
> going to add also cast to the array, tests, and docs within a day.
> Stay tuned :)

Awesome. I'll defer to others on the implementation. I'll try to test
out the patch in a bit to see how it works.

Are there any objections adding this as a v14 open item?

Thanks,

Jonathan

On Thu, Jun 10, 2021 at 8:57 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:
> On 6/10/21 1:24 PM, Alexander Korotkov wrote:
> > I agree that unnest(), cast to array and subscription are missing
> > points.  Proper subscription support requires expanded object
> > handling.  And that seems too late for v14.
>
> Agreed, the subscripting functionality is too late for v14. (Though
> perhaps someone ambitious could bridge that gap temporarily with the
> ability to add subscripting to types!).
>
> >  But unnset() and cast to
> > array seems trivial.  I've drafted unnest support (attached).  I'm
> > going to add also cast to the array, tests, and docs within a day.
> > Stay tuned :)
>
> Awesome. I'll defer to others on the implementation. I'll try to test
> out the patch in a bit to see how it works.

Good!

> Are there any objections adding this as a v14 open item?

No objections, let's add it.

------
Regards,
Alexander Korotkov

+{ oid => '1293', descr => 'expand mutlirange to set of ranges',

typo: mutlirange

Thanks Jonathan for excercising this implementation sooner than later.

-- 
Justin

On Fri, Jun 11, 2021 at 1:04 AM Justin Pryzby <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.

------
Regards,
Alexander Korotkov

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> 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

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;

-- 
Justin

()On Sat, Jun 12, 2021 at 2:30 AM Justin Pryzby <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> 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.

------
Regards,
Alexander Korotkov

On Sat, Jun 12, 2021 at 2:44 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> ()On Sat, Jun 12, 2021 at 2:30 AM Justin Pryzby <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> 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.

------
Regards,
Alexander Korotkov

On 6/12/21 5:57 PM, Alexander Korotkov wrote:
> On Sat, Jun 12, 2021 at 2:44 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
>> ()On Sat, Jun 12, 2021 at 2:30 AM Justin Pryzby <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> 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>

Again, I'll defer to others on the code, but this seems to solve the use
case I presented. Thanks for the quick turnaround!

Jonathan

On Sun, Jun 13, 2021 at 1:16 AM Jonathan S. Katz <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> wrote:
> >> ()On Sat, Jun 12, 2021 at 2:30 AM Justin Pryzby <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> 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

On Sun, Jun 13, 2021 at 2:58 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> On Sun, Jun 13, 2021 at 1:16 AM Jonathan S. Katz <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> wrote:
> > >> ()On Sat, Jun 12, 2021 at 2:30 AM Justin Pryzby <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> 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!

I've added the commit message to the patch.  I'm going to push it if
no objections.

------
Regards,
Alexander Korotkov

On 6/13/21 7:43 AM, Alexander Korotkov wrote:
> On Sun, Jun 13, 2021 at 2:58 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
>> On Sun, Jun 13, 2021 at 1:16 AM Jonathan S. Katz <jkatz@postgresql.org> wrote:

>>> 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!
>
> I've added the commit message to the patch.  I'm going to push it if
> no objections.

I went ahead and tried testing a few more cases with the patch, and
everything seems to work as expected.

I did skim through the code -- I'm much less familiar with this part of
the system -- and I did not see anything that I would consider "obvious
to correct" from my perspective.

So I will continue to go with what I said above: no objections on the
use case perspective, but I defer to others on the code.

One question: if I were to make a custom multirange type (e.g. let's say
I use "inet" to make "inetrange" and then a "inetmultirange") will this
method still work? It seems so, but I wanted clarify.

Thanks,

Jonathan

On 6/13/21 8:26 AM, Jonathan S. Katz wrote:

> One question: if I were to make a custom multirange type (e.g. let's say
> I use "inet" to make "inetrange" and then a "inetmultirange") will this
> method still work? It seems so, but I wanted clarify.

I went ahead and answered this myself: "yes":

  CREATE TYPE inetrange AS RANGE (SUBTYPE = inet);

  SELECT unnest(inetmultirange(inetrange('192.168.1.1', '192.168.1.5'),
inetrange('192.168.1.7', '192.168.1.10')));
             unnest
  ----------------------------
   [192.168.1.1,192.168.1.5)
   [192.168.1.7,192.168.1.10)
  (2 rows)

Awesome stuff.

Jonathan

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

On Sun, Jun 13, 2021 at 11:25:05AM -0400, Jonathan S. Katz wrote:
> On 6/13/21 10:57 AM, Zhihong Yu wrote:
> > +/* 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.
> 

I believe the point was that (in mathematics) a "set" is unordered, and a
sequence is ordered.  Also, a "setof" tuples in postgres can contain
duplicates.

The docs say "The ranges are read out in storage order (ascending).", so I
think this is just a confusion between what "set" means in math vs in postgres.

In postgres, "sequence" usually refers to the object that generarates a
sequence:
| CREATE SEQUENCE creates a new sequence number generator.

-- 
Justin

On 6/13/21 11:49 AM, Justin Pryzby wrote:
> On Sun, Jun 13, 2021 at 11:25:05AM -0400, Jonathan S. Katz wrote:
>> On 6/13/21 10:57 AM, Zhihong Yu wrote:
>>> +/* 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.
>>
>
> I believe the point was that (in mathematics) a "set" is unordered, and a
> sequence is ordered.  Also, a "setof" tuples in postgres can contain
> duplicates.

The comment in question is part of the header for the
"multirange_unnest" function in the code and AFAICT it is accurate: it
is returning a "set of" ranges as it's literally calling into the
set-returning function framework.

I would suggest leaving it as is.

> The docs say "The ranges are read out in storage order (ascending).", so I
> think this is just a confusion between what "set" means in math vs in postgres.

This is nearly identical to the language in the array unnest[1]
function, which is what I believed Alexander borrowed from:

"Expands an array into a set of rows. The array's elements are read out
in storage order."

If we tweaked the multirange "unnest" function, we could change it to:

+       <para>
+        Expands a multirange into a set of rows.
+        The ranges are read out in storage order (ascending).
+       </para>

to match what the array "unnest" function docs, or

+       <para>
+        Expands a multirange into a set of rows that each
+        contain an individual range.
+        The ranges are read out in storage order (ascending).
+       </para>

to be a bit more specific. However, I think this is also bordering on
overengineering the text, given there has been a lack of feedback on the
"unnest" array function description being confusing.

Thanks,

Jonathan

[1] https://www.postgresql.org/docs/current/functions-array.html

On Sun, Jun 13, 2021 at 5:53 PM Zhihong Yu <zyu@yugabyte.com> wrote:
> +   ObjectAddress myself,
>
> nit: myself -> self

Probably "self" is a better name than "myself" in this context.
However, you can see that the surrounding code already uses the name
"myself".  Therefore, I prefer to stay with "myself".

------
Regards,
Alexander Korotkov

On Sun, Jun 13, 2021 at 9:46 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:
>
> On 6/13/21 11:49 AM, Justin Pryzby wrote:
> > On Sun, Jun 13, 2021 at 11:25:05AM -0400, Jonathan S. Katz wrote:
> >> On 6/13/21 10:57 AM, Zhihong Yu wrote:
> >>> +/* 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.
> >>
> >
> > I believe the point was that (in mathematics) a "set" is unordered, and a
> > sequence is ordered.  Also, a "setof" tuples in postgres can contain
> > duplicates.
>
> The comment in question is part of the header for the
> "multirange_unnest" function in the code and AFAICT it is accurate: it
> is returning a "set of" ranges as it's literally calling into the
> set-returning function framework.
>
> I would suggest leaving it as is.

+1

> > The docs say "The ranges are read out in storage order (ascending).", so I
> > think this is just a confusion between what "set" means in math vs in postgres.
>
> This is nearly identical to the language in the array unnest[1]
> function, which is what I believed Alexander borrowed from:

Yes, that's it! :)

> "Expands an array into a set of rows. The array's elements are read out
> in storage order."
>
> If we tweaked the multirange "unnest" function, we could change it to:
>
> +       <para>
> +        Expands a multirange into a set of rows.
> +        The ranges are read out in storage order (ascending).
> +       </para>
>
> to match what the array "unnest" function docs, or
>
> +       <para>
> +        Expands a multirange into a set of rows that each
> +        contain an individual range.
> +        The ranges are read out in storage order (ascending).
> +       </para>
>
> to be a bit more specific. However, I think this is also bordering on
> overengineering the text, given there has been a lack of feedback on the
> "unnest" array function description being confusing.

I think it's not necessarily to say about rows here.  Our
documentation already has already a number of examples, where we
describe set of returned values without speaking about rows including:
json_array_elements, json_array_elements_text, json_object_keys,
pg_listening_channels, pg_tablespace_databases...

------
Regards,
Alexander Korotkov

On Sun, Jun 13, 2021 at 06:36:42PM -0700, Zhihong Yu wrote:
> On Sun, Jun 13, 2021 at 2:10 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > On Sun, Jun 13, 2021 at 5:53 PM Zhihong Yu <zyu@yugabyte.com> wrote:
> > > +   ObjectAddress myself,
> > >
> > > nit: myself -> self
> >
> > Probably "self" is a better name than "myself" in this context.
> > However, you can see that the surrounding code already uses the name
> > "myself".  Therefore, I prefer to stay with "myself".
>
> Is it Okay if I submit a patch changing the 'myself's to 'self' ?

I think it's too nit-picky to be useful and and too much like busy-work.

The patch wouldn't be applied to backbranches, and the divergence complicates
future backpatches, and can create the possibility to introduce errors.

I already looked for and reported typos introduced in v14, but I can almost
promise that if someone looks closely at the documentation changes there are
more errors to be found, even without testing that the code behaves as
advertised.

You can look for patches which changed docs in v14 like so:
git log -p --cherry-pick --stat origin/REL_13_STABLE...origin/master -- doc

But I recommend reading the changes to documentation in HTML/PDF, since it's
easy to miss an errors while reading SGML.
https://www.postgresql.org/docs/devel/index.html

-- 
Justin

On 6/13/21 5:18 PM, Alexander Korotkov wrote:

>> "Expands an array into a set of rows. The array's elements are read out
>> in storage order."
>>
>> If we tweaked the multirange "unnest" function, we could change it to:
>>
>> +       <para>
>> +        Expands a multirange into a set of rows.
>> +        The ranges are read out in storage order (ascending).
>> +       </para>
>>
>> to match what the array "unnest" function docs, or
>>
>> +       <para>
>> +        Expands a multirange into a set of rows that each
>> +        contain an individual range.
>> +        The ranges are read out in storage order (ascending).
>> +       </para>
>>
>> to be a bit more specific. However, I think this is also bordering on
>> overengineering the text, given there has been a lack of feedback on the
>> "unnest" array function description being confusing.
>
> I think it's not necessarily to say about rows here.  Our
> documentation already has already a number of examples, where we
> describe set of returned values without speaking about rows including:
> json_array_elements, json_array_elements_text, json_object_keys,
> pg_listening_channels, pg_tablespace_databases...

I do agree -- my main point was that I don't think we need to change
anything. I proposed alternatives just to show some other ways of
looking at it. But as I mentioned, at this point I think it's
overengineering the text.

If folks are good with the method + code, I think this is ready.

Thanks,

Jonathan

On Mon, Jun 14, 2021 at 3:50 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:
> On 6/13/21 5:18 PM, Alexander Korotkov wrote:
>
> >> "Expands an array into a set of rows. The array's elements are read out
> >> in storage order."
> >>
> >> If we tweaked the multirange "unnest" function, we could change it to:
> >>
> >> +       <para>
> >> +        Expands a multirange into a set of rows.
> >> +        The ranges are read out in storage order (ascending).
> >> +       </para>
> >>
> >> to match what the array "unnest" function docs, or
> >>
> >> +       <para>
> >> +        Expands a multirange into a set of rows that each
> >> +        contain an individual range.
> >> +        The ranges are read out in storage order (ascending).
> >> +       </para>
> >>
> >> to be a bit more specific. However, I think this is also bordering on
> >> overengineering the text, given there has been a lack of feedback on the
> >> "unnest" array function description being confusing.
> >
> > I think it's not necessarily to say about rows here.  Our
> > documentation already has already a number of examples, where we
> > describe set of returned values without speaking about rows including:
> > json_array_elements, json_array_elements_text, json_object_keys,
> > pg_listening_channels, pg_tablespace_databases...
>
> I do agree -- my main point was that I don't think we need to change
> anything. I proposed alternatives just to show some other ways of
> looking at it. But as I mentioned, at this point I think it's
> overengineering the text.
>
> If folks are good with the method + code, I think this is ready.

Cool, thank you for the summary.  I'll wait for two days since I've
published the last revision of the patch [1] (comes tomorrow), and
push it if no new issues arise.

Links
1. https://www.postgresql.org/message-id/CAPpHfdvG%3DJR5kqmZx7KvTyVgtQePX0QJ09TO1y3sN73WOfJf1Q%40mail.gmail.com

------
Regards,
Alexander Korotkov

On Mon, Jun 14, 2021 at 4:14 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> On Mon, Jun 14, 2021 at 3:50 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:
> > On 6/13/21 5:18 PM, Alexander Korotkov wrote:
> >
> > >> "Expands an array into a set of rows. The array's elements are read out
> > >> in storage order."
> > >>
> > >> If we tweaked the multirange "unnest" function, we could change it to:
> > >>
> > >> +       <para>
> > >> +        Expands a multirange into a set of rows.
> > >> +        The ranges are read out in storage order (ascending).
> > >> +       </para>
> > >>
> > >> to match what the array "unnest" function docs, or
> > >>
> > >> +       <para>
> > >> +        Expands a multirange into a set of rows that each
> > >> +        contain an individual range.
> > >> +        The ranges are read out in storage order (ascending).
> > >> +       </para>
> > >>
> > >> to be a bit more specific. However, I think this is also bordering on
> > >> overengineering the text, given there has been a lack of feedback on the
> > >> "unnest" array function description being confusing.
> > >
> > > I think it's not necessarily to say about rows here.  Our
> > > documentation already has already a number of examples, where we
> > > describe set of returned values without speaking about rows including:
> > > json_array_elements, json_array_elements_text, json_object_keys,
> > > pg_listening_channels, pg_tablespace_databases...
> >
> > I do agree -- my main point was that I don't think we need to change
> > anything. I proposed alternatives just to show some other ways of
> > looking at it. But as I mentioned, at this point I think it's
> > overengineering the text.
> >
> > If folks are good with the method + code, I think this is ready.
>
> Cool, thank you for the summary.  I'll wait for two days since I've
> published the last revision of the patch [1] (comes tomorrow), and
> push it if no new issues arise.

Pushed!  Thanks to thread participants for raising this topic and
review.  I'll be around to resolve issues if any.

------
Regards,
Alexander Korotkov

Alexander Korotkov <aekorotkov@gmail.com> writes:
> Pushed!  Thanks to thread participants for raising this topic and
> review.  I'll be around to resolve issues if any.

Buildfarm is pretty thoroughly unhappy.  Did you do a "check-world"
before pushing?

            regards, tom lane

On Tue, Jun 15, 2021 at 4:49 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alexander Korotkov <aekorotkov@gmail.com> writes:
> > Pushed!  Thanks to thread participants for raising this topic and
> > review.  I'll be around to resolve issues if any.
>
> Buildfarm is pretty thoroughly unhappy.  Did you do a "check-world"
> before pushing?

Yes, I'm looking at this now.

I did run "check-world", but it passed for me.  Probably the same
reason it passed for some buildfarm animals...

------
Regards,
Alexander Korotkov

Alexander Korotkov <aekorotkov@gmail.com> writes:
> I did run "check-world", but it passed for me.  Probably the same
> reason it passed for some buildfarm animals...

The only buildfarm animals that have passed since this went in
are the ones that don't run the pg_dump or pg_upgrade tests.

It looks to me like the proximate problem is that you should
have taught pg_dump to skip these new auto-generated functions.
However, I fail to see why we need auto-generated functions
for this at all.  Couldn't we have done it with one polymorphic
function?

I think this ought to be reverted and reviewed more carefully.

            regards, tom lane

On 2021-Jun-15, Tom Lane wrote:

> It looks to me like the proximate problem is that you should
> have taught pg_dump to skip these new auto-generated functions.
> However, I fail to see why we need auto-generated functions
> for this at all.  Couldn't we have done it with one polymorphic
> function?

I think such a function would need to take anycompatiblerangearray,
which is not something we currently have.

> I think this ought to be reverted and reviewed more carefully.

It seems to me that removing the cast-to-range[] is a sufficient fix,
and that we can do with only the unnest part for pg14; the casts can be
added in 15 (if at all).  That would mean reverting only half the patch.

-- 
Álvaro Herrera                            39°49'30"S 73°17'W

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> On 2021-Jun-15, Tom Lane wrote:
>> I think this ought to be reverted and reviewed more carefully.

> It seems to me that removing the cast-to-range[] is a sufficient fix,
> and that we can do with only the unnest part for pg14; the casts can be
> added in 15 (if at all).  That would mean reverting only half the patch.

Might be a reasonable solution.  But right now I'm annoyed that the
buildfarm is broken, and I'm also convinced that this didn't get
adequate testing.  I think "revert and reconsider" is the way
forward for today.

            regards, tom lane

On 6/15/21 1:52 PM, Tom Lane wrote:
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
>> On 2021-Jun-15, Tom Lane wrote:
>>> I think this ought to be reverted and reviewed more carefully.
>
>> It seems to me that removing the cast-to-range[] is a sufficient fix,
>> and that we can do with only the unnest part for pg14; the casts can be
>> added in 15 (if at all).  That would mean reverting only half the patch.
>
> Might be a reasonable solution.  But right now I'm annoyed that the
> buildfarm is broken, and I'm also convinced that this didn't get
> adequate testing.

I had focused testing primarily on the "unnest" cases that I had
described in my original note. I did a couple of casts and had no issue;
I did not test with pg_dump / pg_upgrade, but noting to do so in the
future in cases like this.

>  I think "revert and reconsider" is the way
> forward for today.

I don't want the buildfarm broken so I'm fine if this is the best way
forward. If we can keep the "unnest" functionality I would strongly
suggest it as that was the premise of the original note to complete the
utility of multiranges for v14. The casting, while convenient, is not
needed.

Jonathan

On 6/15/21 12:06 PM, Alexander Korotkov wrote:
> On Tue, Jun 15, 2021 at 4:49 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Alexander Korotkov <aekorotkov@gmail.com> writes:
>>> Pushed!  Thanks to thread participants for raising this topic and
>>> review.  I'll be around to resolve issues if any.
>> Buildfarm is pretty thoroughly unhappy.  Did you do a "check-world"
>> before pushing?
> Yes, I'm looking at this now.
>
> I did run "check-world", but it passed for me.  Probably the same
> reason it passed for some buildfarm animals...
>

Did you configure with --enable-tap-tests? If not, then `make
check-world` won't run the tests that are failing here.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On Tue, Jun 15, 2021 at 8:18 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alexander Korotkov <aekorotkov@gmail.com> writes:
> > I did run "check-world", but it passed for me.  Probably the same
> > reason it passed for some buildfarm animals...
>
> The only buildfarm animals that have passed since this went in
> are the ones that don't run the pg_dump or pg_upgrade tests.
>
> It looks to me like the proximate problem is that you should
> have taught pg_dump to skip these new auto-generated functions.
> However, I fail to see why we need auto-generated functions
> for this at all.  Couldn't we have done it with one polymorphic
> function?
>
> I think this ought to be reverted and reviewed more carefully.

Thank you for your feedback.  I've reverted the patch.

I'm going to have closer look at this tomorrow.

------
Regards,
Alexander Korotkov

On 6/15/21 1:52 PM, Tom Lane wrote:
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
>> On 2021-Jun-15, Tom Lane wrote:
>>> I think this ought to be reverted and reviewed more carefully.
>> It seems to me that removing the cast-to-range[] is a sufficient fix,
>> and that we can do with only the unnest part for pg14; the casts can be
>> added in 15 (if at all).  That would mean reverting only half the patch.
> Might be a reasonable solution.  But right now I'm annoyed that the
> buildfarm is broken, and I'm also convinced that this didn't get
> adequate testing.  I think "revert and reconsider" is the way
> forward for today.
>
>     



(RMT hat on) That would be my inclination at this stage. The commit
message states that it's trivial, but it seems not to be, and I suspect
it should not have been done at this stage of the development cycle.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On Tue, Jun 15, 2021 at 8:28 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> On 2021-Jun-15, Tom Lane wrote:
>
> > It looks to me like the proximate problem is that you should
> > have taught pg_dump to skip these new auto-generated functions.
> > However, I fail to see why we need auto-generated functions
> > for this at all.  Couldn't we have done it with one polymorphic
> > function?
>
> I think such a function would need to take anycompatiblerangearray,
> which is not something we currently have.

Yes, I've started with polymorphic function
multirange_to_array(anymultirange) returning anyarray.  But then I got
that for int4multirange return type Is integer[] instead of
int4range[] :)

# select pg_typeof(multirange_to_array('{[1,2),[5,6)}'::int4multirange));
 pg_typeof
-----------
 integer[]
(1 row)

So, a new anyrangearray polymorphic type is required for this
function.  Not sure if it worth it to introduce a new polymorphic
function for this use case.

------
Regards,
Alexander Korotkov

On Tue, Jun 15, 2021 at 9:43 PM Andrew Dunstan <andrew@dunslane.net> wrote:
> On 6/15/21 12:06 PM, Alexander Korotkov wrote:
> > On Tue, Jun 15, 2021 at 4:49 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Alexander Korotkov <aekorotkov@gmail.com> writes:
> >>> Pushed!  Thanks to thread participants for raising this topic and
> >>> review.  I'll be around to resolve issues if any.
> >> Buildfarm is pretty thoroughly unhappy.  Did you do a "check-world"
> >> before pushing?
> > Yes, I'm looking at this now.
> >
> > I did run "check-world", but it passed for me.  Probably the same
> > reason it passed for some buildfarm animals...
> >
>
> Did you configure with --enable-tap-tests? If not, then `make
> check-world` won't run the tests that are failing here.

I've rechecked that check-world actually fails on my machine on that
commit.  I definitely configured with --enable-tap-tests.  So, it
appears that I just did something wrong (run make check-world on a
different branch or something like that).  Sorry for that.  I'll
double-check in the future.

------
Regards,
Alexander Korotkov

On Tue, Jun 15, 2021 at 8:18 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alexander Korotkov <aekorotkov@gmail.com> writes:
> > I did run "check-world", but it passed for me.  Probably the same
> > reason it passed for some buildfarm animals...
>
> It looks to me like the proximate problem is that you should
> have taught pg_dump to skip these new auto-generated functions.

Yes, it appears that pg_dump skips auto-generated functions, but
doesn't skip auto-generated casts.  It appears to be enough to tune
query getCasts() to resolve the issue.  The revised patch is attached.

------
Regards,
Alexander Korotkov

On Wed, Jun 16, 2021 at 3:44 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> On Tue, Jun 15, 2021 at 8:18 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Alexander Korotkov <aekorotkov@gmail.com> writes:
> > > I did run "check-world", but it passed for me.  Probably the same
> > > reason it passed for some buildfarm animals...
> >
> > It looks to me like the proximate problem is that you should
> > have taught pg_dump to skip these new auto-generated functions.
>
> Yes, it appears that pg_dump skips auto-generated functions, but
> doesn't skip auto-generated casts.  It appears to be enough to tune
> query getCasts() to resolve the issue.  The revised patch is attached.

Here is the next revision of the patch: I've adjusted some comments.

In my point of view this patch is not actually complex.  The reason
why it colored buildfarm in red is purely my fault: I messed up with
"make check-world" :(

I've registered it on the commitfest application to make it go through
commitfest.cputube.org.  My proposal is to re-push it once it goes
through commitfest.cputube.org.

------
Regards,
Alexander Korotkov

On Thu, Jun 17, 2021 at 7:54 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> On Wed, Jun 16, 2021 at 3:44 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > On Tue, Jun 15, 2021 at 8:18 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > > Alexander Korotkov <aekorotkov@gmail.com> writes:
> > > > I did run "check-world", but it passed for me.  Probably the same
> > > > reason it passed for some buildfarm animals...
> > >
> > > It looks to me like the proximate problem is that you should
> > > have taught pg_dump to skip these new auto-generated functions.
> >
> > Yes, it appears that pg_dump skips auto-generated functions, but
> > doesn't skip auto-generated casts.  It appears to be enough to tune
> > query getCasts() to resolve the issue.  The revised patch is attached.
>
> Here is the next revision of the patch: I've adjusted some comments.
>
> In my point of view this patch is not actually complex.  The reason
> why it colored buildfarm in red is purely my fault: I messed up with
> "make check-world" :(
>
> I've registered it on the commitfest application to make it go through
> commitfest.cputube.org.  My proposal is to re-push it once it goes
> through commitfest.cputube.org.

Patch successfully passed commitfest.cputube.org.  I'm going to
re-push it if there are no objections.

------
Regards,
Alexander Korotkov

Alexander Korotkov <aekorotkov@gmail.com> writes:
> Patch successfully passed commitfest.cputube.org.  I'm going to
> re-push it if there are no objections.

I'm still not happy about the way you've done the multirange-to-array
part.  I think we'd be better off improving the polymorphism rules so
that that can be handled by one polymorphic function.  Obviously that'd
be a task for v15, but we've already concluded that just having the
unnest ability would be minimally sufficient for v14.

So I think you should trim it down to just the unnest part.

In any case, beta2 wraps on Monday, and there is very little time
left for a full round of buildfarm testing.  I almost feel that
it's too late to consider pushing this today.  Tomorrow absolutely
is too late for beta2.

            regards, tom lane

On Sat, Jun 19, 2021 at 7:35 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alexander Korotkov <aekorotkov@gmail.com> writes:
> > Patch successfully passed commitfest.cputube.org.  I'm going to
> > re-push it if there are no objections.
>
> I'm still not happy about the way you've done the multirange-to-array
> part.  I think we'd be better off improving the polymorphism rules so
> that that can be handled by one polymorphic function.  Obviously that'd
> be a task for v15, but we've already concluded that just having the
> unnest ability would be minimally sufficient for v14.
>
> So I think you should trim it down to just the unnest part.

I'm not entirely sure it's worth introducing anyrangearray.  There
might be not many use-cases of anyrangearray besides this cast
(normally one should use multirange instead of an array of ranges).
But I agree that this subject should be carefully considered for v15.

> In any case, beta2 wraps on Monday, and there is very little time
> left for a full round of buildfarm testing.  I almost feel that
> it's too late to consider pushing this today.  Tomorrow absolutely
> is too late for beta2.

+1
I also don't feel comfortable hurrying with unnest part to beta2.
According to the open items wiki page, there should be beta3.  Does
unnest part have a chance for beta3?

------
Regards,
Alexander Korotkov

Alexander Korotkov <aekorotkov@gmail.com> writes:
> I also don't feel comfortable hurrying with unnest part to beta2.
> According to the open items wiki page, there should be beta3.  Does
> unnest part have a chance for beta3?

Hm.  I'd prefer to avoid another forced initdb after beta2.  On the
other hand, it's entirely likely that there will be some other thing
that forces that; in which case there'd be no reason not to push in
the unnest feature as well.

I'd say let's sit on the unnest code for a little bit and see what
happens.

            regards, tom lane

On Sat, Jun 19, 2021 at 10:05:09PM -0400, Tom Lane wrote:
> Alexander Korotkov <aekorotkov@gmail.com> writes:
> > I also don't feel comfortable hurrying with unnest part to beta2.
> > According to the open items wiki page, there should be beta3.  Does
> > unnest part have a chance for beta3?
> 
> Hm.  I'd prefer to avoid another forced initdb after beta2.  On the
> other hand, it's entirely likely that there will be some other thing
> that forces that; in which case there'd be no reason not to push in
> the unnest feature as well.
> 
> I'd say let's sit on the unnest code for a little bit and see what
> happens.

I think $SUBJECT can't simultaneously offer too little to justify its own
catversion bump and also offer enough to bypass feature freeze.  If multirange
is good without $SUBJECT, then $SUBJECT should wait for v15.  Otherwise, the
matter of the catversion bump should not delay commit of $SUBJECT.

On Sun, Jun 20, 2021 at 11:09 AM Noah Misch <noah@leadboat.com> wrote:
> On Sat, Jun 19, 2021 at 10:05:09PM -0400, Tom Lane wrote:
> > Alexander Korotkov <aekorotkov@gmail.com> writes:
> > > I also don't feel comfortable hurrying with unnest part to beta2.
> > > According to the open items wiki page, there should be beta3.  Does
> > > unnest part have a chance for beta3?
> >
> > Hm.  I'd prefer to avoid another forced initdb after beta2.  On the
> > other hand, it's entirely likely that there will be some other thing
> > that forces that; in which case there'd be no reason not to push in
> > the unnest feature as well.
> >
> > I'd say let's sit on the unnest code for a little bit and see what
> > happens.
>
> I think $SUBJECT can't simultaneously offer too little to justify its own
> catversion bump and also offer enough to bypass feature freeze.  If multirange
> is good without $SUBJECT, then $SUBJECT should wait for v15.  Otherwise, the
> matter of the catversion bump should not delay commit of $SUBJECT.

FWIW, there is a patch implementing just unnest() function.

------
Regards,
Alexander Korotkov

On Mon, Jun 21, 2021 at 1:24 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> On Sun, Jun 20, 2021 at 11:09 AM Noah Misch <noah@leadboat.com> wrote:
> > On Sat, Jun 19, 2021 at 10:05:09PM -0400, Tom Lane wrote:
> > > Alexander Korotkov <aekorotkov@gmail.com> writes:
> > > > I also don't feel comfortable hurrying with unnest part to beta2.
> > > > According to the open items wiki page, there should be beta3.  Does
> > > > unnest part have a chance for beta3?
> > >
> > > Hm.  I'd prefer to avoid another forced initdb after beta2.  On the
> > > other hand, it's entirely likely that there will be some other thing
> > > that forces that; in which case there'd be no reason not to push in
> > > the unnest feature as well.
> > >
> > > I'd say let's sit on the unnest code for a little bit and see what
> > > happens.
> >
> > I think $SUBJECT can't simultaneously offer too little to justify its own
> > catversion bump and also offer enough to bypass feature freeze.  If multirange
> > is good without $SUBJECT, then $SUBJECT should wait for v15.  Otherwise, the
> > matter of the catversion bump should not delay commit of $SUBJECT.
>
> FWIW, there is a patch implementing just unnest() function.

BTW, I found some small inconsistencies in the declaration of
multirange operators in the system catalog.  Nothing critical, but if
we decide to bump catversion in beta3, this patch is also nice to
push.

------
Regards,
Alexander Korotkov

On 2021-Jun-27, Alexander Korotkov wrote:

> BTW, I found some small inconsistencies in the declaration of
> multirange operators in the system catalog.  Nothing critical, but if
> we decide to bump catversion in beta3, this patch is also nice to
> push.

Hmm, I think you should push this and not bump catversion.  That way,
nobody is forced to initdb if we end up not having a catversion bump for
some other reason; but also anybody who initdb's with beta3 or later
will get the correct descriptions.

If you don't push it, everybody will have the wrong descriptions.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

On Sat, Jul 10, 2021 at 7:34 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> On 2021-Jun-27, Alexander Korotkov wrote:
>
> > BTW, I found some small inconsistencies in the declaration of
> > multirange operators in the system catalog.  Nothing critical, but if
> > we decide to bump catversion in beta3, this patch is also nice to
> > push.
>
> Hmm, I think you should push this and not bump catversion.  That way,
> nobody is forced to initdb if we end up not having a catversion bump for
> some other reason; but also anybody who initdb's with beta3 or later
> will get the correct descriptions.
>
> If you don't push it, everybody will have the wrong descriptions.

True, but I'm a bit uncomfortable about user instances with different
catalogs but the same catversions.  On the other hand, initdb's with
beta3 or later will be the vast majority among pg14 instances.

Did we have similar precedents in the past?

------
Regards,
Alexander Korotkov

On Sun, Jul 11, 2021 at 01:00:27AM +0300, Alexander Korotkov wrote:
> On Sat, Jul 10, 2021 at 7:34 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > On 2021-Jun-27, Alexander Korotkov wrote:
> >
> > > BTW, I found some small inconsistencies in the declaration of
> > > multirange operators in the system catalog.  Nothing critical, but if
> > > we decide to bump catversion in beta3, this patch is also nice to
> > > push.
> >
> > Hmm, I think you should push this and not bump catversion.  That way,
> > nobody is forced to initdb if we end up not having a catversion bump for
> > some other reason; but also anybody who initdb's with beta3 or later
> > will get the correct descriptions.
> >
> > If you don't push it, everybody will have the wrong descriptions.
> 
> True, but I'm a bit uncomfortable about user instances with different
> catalogs but the same catversions.  On the other hand, initdb's with
> beta3 or later will be the vast majority among pg14 instances.
> 
> Did we have similar precedents in the past?

It seems so.

Note in particular 74ab96a45, which adds a new function with no bump.
Although that one may not be a good precedent to follow, or one that's been
followed recently.

commit 0aac73e6a2602696d23aa7a9686204965f9093dc
Author: Noah Misch <noah@leadboat.com>
Date:   Mon Jun 14 17:29:37 2021 -0700

    Copy-edit text for the pg_terminate_backend() "timeout" parameter.

commit b09a64d602a19c9a3cc69e4bb0f8986a6f5facf4
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date:   Thu Sep 20 16:06:18 2018 -0400

    Add missing pg_description strings for pg_type entries.

commit a4627e8fd479ff74fffdd49ad07636b79751be45
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date:   Tue Feb 2 11:39:50 2016 -0500

    Fix pg_description entries for jsonb_to_record() and jsonb_to_recordset().

commit b852dc4cbd09156e2c74786d5b265f03d45bc404
Author: Bruce Momjian <bruce@momjian.us>
Date:   Wed Oct 7 09:06:49 2015 -0400

    docs:  clarify JSONB operator descriptions

commit a80889a7359e720107b881bcd3e8fd47f3874e36
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date:   Wed Oct 10 12:19:25 2012 -0400

    Set procost to 10 for each of the pg_foo_is_visible() functions.

commit c246eb5aafe66d5537b468d6da2116c462775faf
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date:   Sat Aug 18 16:14:57 2012 -0400

    Make use of LATERAL in information_schema.sequences view.

commit 74ab96a45ef6259aa6a86a781580edea8488511a
Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date:   Wed Jan 25 13:15:29 2012 -0300

    Add pg_trigger_depth() function

commit ddd6ff289f2512f881493b7793853a96955459ff
Author: Bruce Momjian <bruce@momjian.us>
Date:   Tue Mar 15 11:26:20 2011 -0400

    Add database comments to template0 and postgres databases, and improve

Justin Pryzby <pryzby@telsasoft.com> writes:
> On Sun, Jul 11, 2021 at 01:00:27AM +0300, Alexander Korotkov wrote:
>> True, but I'm a bit uncomfortable about user instances with different
>> catalogs but the same catversions.  On the other hand, initdb's with
>> beta3 or later will be the vast majority among pg14 instances.
>> 
>> Did we have similar precedents in the past?

> It seems so.

If it's *only* the description strings you want to change, then yeah,
we've done that before.

            regards, tom lane

 On Sun, Jul 11, 2021 at 1:28 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Justin Pryzby <pryzby@telsasoft.com> writes:
> > On Sun, Jul 11, 2021 at 01:00:27AM +0300, Alexander Korotkov wrote:
> >> True, but I'm a bit uncomfortable about user instances with different
> >> catalogs but the same catversions.  On the other hand, initdb's with
> >> beta3 or later will be the vast majority among pg14 instances.
> >>
> >> Did we have similar precedents in the past?
>
> > It seems so.
>
> If it's *only* the description strings you want to change, then yeah,
> we've done that before.

My patch also changes 'oprjoin' from 'scalargtjoinsel' to
'scalarltjoinsel'.  Implementation is the same, but 'scalarltjoinsel'
looks more logical here.

------
Regards,
Alexander Korotkov

On Sun, Jul 11, 2021 at 1:20 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> On Sun, Jul 11, 2021 at 01:00:27AM +0300, Alexander Korotkov wrote:
> > On Sat, Jul 10, 2021 at 7:34 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > > On 2021-Jun-27, Alexander Korotkov wrote:
> > >
> > > > BTW, I found some small inconsistencies in the declaration of
> > > > multirange operators in the system catalog.  Nothing critical, but if
> > > > we decide to bump catversion in beta3, this patch is also nice to
> > > > push.
> > >
> > > Hmm, I think you should push this and not bump catversion.  That way,
> > > nobody is forced to initdb if we end up not having a catversion bump for
> > > some other reason; but also anybody who initdb's with beta3 or later
> > > will get the correct descriptions.
> > >
> > > If you don't push it, everybody will have the wrong descriptions.
> >
> > True, but I'm a bit uncomfortable about user instances with different
> > catalogs but the same catversions.  On the other hand, initdb's with
> > beta3 or later will be the vast majority among pg14 instances.
> >
> > Did we have similar precedents in the past?
>
> It seems so.
>
> Note in particular 74ab96a45, which adds a new function with no bump.
> Although that one may not be a good precedent to follow, or one that's been
> followed recently.

Justin, thank you very much for the summary.

Given we have similar precedents in the past, I'm going to push the
patch [1] to master and pg14 if no objections.

Links
1. https://www.postgresql.org/message-id/CAPpHfdv9OZEuZDqOQoUKpXhq%3Dmc-qa4gKCPmcgG5Vvesu7%3Ds1w%40mail.gmail.com

------
Regards,
Alexander Korotkov

On Tue, Jul 13, 2021 at 03:11:16PM +0300, Alexander Korotkov wrote:
> On Sun, Jul 11, 2021 at 1:20 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > On Sun, Jul 11, 2021 at 01:00:27AM +0300, Alexander Korotkov wrote:
> > > On Sat, Jul 10, 2021 at 7:34 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > > > On 2021-Jun-27, Alexander Korotkov wrote:
> > > >
> > > > > BTW, I found some small inconsistencies in the declaration of
> > > > > multirange operators in the system catalog.  Nothing critical, but if
> > > > > we decide to bump catversion in beta3, this patch is also nice to
> > > > > push.
> > > >
> > > > Hmm, I think you should push this and not bump catversion.  That way,
> > > > nobody is forced to initdb if we end up not having a catversion bump for
> > > > some other reason; but also anybody who initdb's with beta3 or later
> > > > will get the correct descriptions.
> > > >
> > > > If you don't push it, everybody will have the wrong descriptions.
> > >
> > > True, but I'm a bit uncomfortable about user instances with different
> > > catalogs but the same catversions.  On the other hand, initdb's with
> > > beta3 or later will be the vast majority among pg14 instances.
> > >
> > > Did we have similar precedents in the past?
> >
> > It seems so.
> >
> > Note in particular 74ab96a45, which adds a new function with no bump.
> > Although that one may not be a good precedent to follow, or one that's been
> > followed recently.
> 
> Justin, thank you very much for the summary.
> 
> Given we have similar precedents in the past, I'm going to push the
> patch [1] to master and pg14 if no objections.

To be clear, do you mean with or without this hunk ?

-  oprrest => 'multirangesel', oprjoin => 'scalargtjoinsel' },
+  oprrest => 'multirangesel', oprjoin => 'scalarltjoinsel' },

-- 
Justin

On Tue, Jul 13, 2021 at 5:07 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> On Tue, Jul 13, 2021 at 03:11:16PM +0300, Alexander Korotkov wrote:
> > On Sun, Jul 11, 2021 at 1:20 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > > On Sun, Jul 11, 2021 at 01:00:27AM +0300, Alexander Korotkov wrote:
> > > > On Sat, Jul 10, 2021 at 7:34 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > > > > On 2021-Jun-27, Alexander Korotkov wrote:
> > > > >
> > > > > > BTW, I found some small inconsistencies in the declaration of
> > > > > > multirange operators in the system catalog.  Nothing critical, but if
> > > > > > we decide to bump catversion in beta3, this patch is also nice to
> > > > > > push.
> > > > >
> > > > > Hmm, I think you should push this and not bump catversion.  That way,
> > > > > nobody is forced to initdb if we end up not having a catversion bump for
> > > > > some other reason; but also anybody who initdb's with beta3 or later
> > > > > will get the correct descriptions.
> > > > >
> > > > > If you don't push it, everybody will have the wrong descriptions.
> > > >
> > > > True, but I'm a bit uncomfortable about user instances with different
> > > > catalogs but the same catversions.  On the other hand, initdb's with
> > > > beta3 or later will be the vast majority among pg14 instances.
> > > >
> > > > Did we have similar precedents in the past?
> > >
> > > It seems so.
> > >
> > > Note in particular 74ab96a45, which adds a new function with no bump.
> > > Although that one may not be a good precedent to follow, or one that's been
> > > followed recently.
> >
> > Justin, thank you very much for the summary.
> >
> > Given we have similar precedents in the past, I'm going to push the
> > patch [1] to master and pg14 if no objections.
>
> To be clear, do you mean with or without this hunk ?
>
> -  oprrest => 'multirangesel', oprjoin => 'scalargtjoinsel' },
> +  oprrest => 'multirangesel', oprjoin => 'scalarltjoinsel' },

I mean with this hunk unless I hear objection to it.

The implementations of scalarltjoinsel and scalargtjoinsel are the
same.  And I don't think they are going to be changed on pg14.

------
Regards,
Alexander Korotkov

On 2021-Jul-13, Alexander Korotkov wrote:

> > To be clear, do you mean with or without this hunk ?
> >
> > -  oprrest => 'multirangesel', oprjoin => 'scalargtjoinsel' },
> > +  oprrest => 'multirangesel', oprjoin => 'scalarltjoinsel' },
> 
> I mean with this hunk unless I hear objection to it.

+1 for pushing with that hunk, no catversion bump.

-- 
Álvaro Herrera           39°49'30"S 73°17'W  —  https://www.EnterpriseDB.com/
"The Gord often wonders why people threaten never to come back after they've
been told never to return" (www.actsofgord.com)

On Tue, Jul 13, 2021 at 5:29 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2021-Jul-13, Alexander Korotkov wrote:
>
> > > To be clear, do you mean with or without this hunk ?
> > >
> > > -  oprrest => 'multirangesel', oprjoin => 'scalargtjoinsel' },
> > > +  oprrest => 'multirangesel', oprjoin => 'scalarltjoinsel' },
> >
> > I mean with this hunk unless I hear objection to it.
>
> +1 for pushing with that hunk, no catversion bump.

Thank you for the feedback.  Pushed!

------
Regards,
Alexander Korotkov

On 2021-Jun-19, Tom Lane wrote:

> Alexander Korotkov <aekorotkov@gmail.com> writes:
> > I also don't feel comfortable hurrying with unnest part to beta2.
> > According to the open items wiki page, there should be beta3.  Does
> > unnest part have a chance for beta3?
> 
> Hm.  I'd prefer to avoid another forced initdb after beta2.  On the
> other hand, it's entirely likely that there will be some other thing
> that forces that; in which case there'd be no reason not to push in
> the unnest feature as well.
> 
> I'd say let's sit on the unnest code for a little bit and see what
> happens.

... So, almost a month has gone by, and we still don't have multirange
unnest().  Looking at the open items list, it doesn't look like we have
anything that would require a catversion bump.  Does that mean that
we're going to ship pg14 without multirange unnest?

That seems pretty sad, as the usability of the feature is greatly
reduced.  Just look at what's being suggested:
  https://postgr.es/m/20210715121508.GA30348@depesz.com
To me this screams of an incomplete datatype.  I far prefer a beta3
initdb than shipping 14GA without multirange unnest.

I haven't seen any announcements about beta3, but it's probably not far
off; I think if we're going to have it, it would be much better to give
it buildfarm cycles several days in advance and not just the last
weekend.

What do others think?

-- 
Álvaro Herrera           39°49'30"S 73°17'W  —  https://www.EnterpriseDB.com/
"Ninguna manada de bestias tiene una voz tan horrible como la humana" (Orual)

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> On 2021-Jun-19, Tom Lane wrote:
>> I'd say let's sit on the unnest code for a little bit and see what
>> happens.

> ... So, almost a month has gone by, and we still don't have multirange
> unnest().  Looking at the open items list, it doesn't look like we have
> anything that would require a catversion bump.  Does that mean that
> we're going to ship pg14 without multirange unnest?

> That seems pretty sad, as the usability of the feature is greatly
> reduced.  Just look at what's being suggested:
>   https://postgr.es/m/20210715121508.GA30348@depesz.com
> To me this screams of an incomplete datatype.  I far prefer a beta3
> initdb than shipping 14GA without multirange unnest.

Yeah, that seems pretty horrid.  I still don't like the way the
array casts were done, but I'd be okay with pushing the unnest
addition.

            regards, tom lane

On Thu, Jul 15, 2021 at 6:47 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> > On 2021-Jun-19, Tom Lane wrote:
> >> I'd say let's sit on the unnest code for a little bit and see what
> >> happens.
>
> > ... So, almost a month has gone by, and we still don't have multirange
> > unnest().  Looking at the open items list, it doesn't look like we have
> > anything that would require a catversion bump.  Does that mean that
> > we're going to ship pg14 without multirange unnest?
>
> > That seems pretty sad, as the usability of the feature is greatly
> > reduced.  Just look at what's being suggested:
> >   https://postgr.es/m/20210715121508.GA30348@depesz.com
> > To me this screams of an incomplete datatype.  I far prefer a beta3
> > initdb than shipping 14GA without multirange unnest.
>
> Yeah, that seems pretty horrid.  I still don't like the way the
> array casts were done, but I'd be okay with pushing the unnest
> addition.

I agree that array casts require better polymorphism and should be
considered for pg15.

+1 for just unnest().

------
Regards,
Alexander Korotkov

On 7/15/21 12:26 PM, Alexander Korotkov wrote:
> On Thu, Jul 15, 2021 at 6:47 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yeah, that seems pretty horrid.  I still don't like the way the
>> array casts were done, but I'd be okay with pushing the unnest
>> addition.
>
> +1 for just unnest().

...which was my original ask at the beginning of the thread :) So +1.

Thanks,

Jonathan

On Thu, Jul 15, 2021 at 10:27 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:
> On 7/15/21 12:26 PM, Alexander Korotkov wrote:
> > On Thu, Jul 15, 2021 at 6:47 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Yeah, that seems pretty horrid.  I still don't like the way the
> >> array casts were done, but I'd be okay with pushing the unnest
> >> addition.
> >
> > +1 for just unnest().
>
> ...which was my original ask at the beginning of the thread :) So +1.

Thanks for the feedback.  I've pushed the unnest() patch to master and
pg14.  I've initially forgotten to change catversion.h for master, so
I made it with an additional commit.

I've double-checked that "make check-world" passes on my machine for
both master and pg14.  And I'm keeping my fingers crossed looking at
buildfarm.

------
Regards,
Alexander Korotkov

On Sun, Jul 18, 2021 at 8:20 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
>
> On Thu, Jul 15, 2021 at 10:27 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:
> > On 7/15/21 12:26 PM, Alexander Korotkov wrote:
> > > On Thu, Jul 15, 2021 at 6:47 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > >> Yeah, that seems pretty horrid.  I still don't like the way the
> > >> array casts were done, but I'd be okay with pushing the unnest
> > >> addition.
> > >
> > > +1 for just unnest().
> >
> > ...which was my original ask at the beginning of the thread :) So +1.
>
> Thanks for the feedback.  I've pushed the unnest() patch to master and
> pg14.  I've initially forgotten to change catversion.h for master, so
> I made it with an additional commit.
>
> I've double-checked that "make check-world" passes on my machine for
> both master and pg14.  And I'm keeping my fingers crossed looking at
> buildfarm.

This patch was closed with "moved to next commitfest" in the July
commitfest, and is currently sitting as "Needs review" in the
September one.

If it's committed, it should probably have been closed with that? And
if there are things still needed, they should perhaps have their own
CF entry instead since we clearly do have unnest() for multiranges?

-- 
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/