On Tue, 10 Oct 2023 at 12:36, Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
>
> > > 2. The various in_range() functions needed adjusting to handle
> > > infinite interval offsets.
> > >
> I think this code is reasonable. But users may find it inconsistent
> with the other ways to achieve the same outcome. For example, We don't
> allow non-finite intervals to be added or subtracted from time or
> timetz. So if someone tries to compare the rows using val >/<= base
> +/- offset, those queries will fail whereas similar implied conditions
> in window specification will not throw an error. If you have
> considered this already, I am fine with the code as is.
>
It's consistent with the documented contract of the in_range support
functions. See https://www.postgresql.org/docs/current/btree-support-funcs.html
In particular, this part:
An additional expectation is that in_range functions should, if
practical, avoid throwing an error if base + offset or base - offset
would overflow. The correct comparison result can be determined even
if that value would be out of the data type's range. Note that if the
data type includes concepts such as "infinity" or "NaN", extra
care may be needed to ensure that in_range's results agree with the
normal sort order of the operator family.
> Added a separate patch (0009) to fix
> brin_minmax_multi_distance_interval().
>
I think we can drop this from this thread now, given the discussion
over on the other thread
(https://www.postgresql.org/message-id/eef0ea8c-4aaa-8d0d-027f-58b1f35dd170%40enterprisedb.com)
> I have added a separate patch (0008) to test negative interval values,
> including -infinity, in preceding and following specification.
>
> Patches from 0001 to 0007 are same as what you attached but rebased on
> the latest HEAD.
>
I'm attaching another update, with a minor change to the aggregate
deserialization function, in line with the recent change to how these
now work elsewhere (see 0c882a298881056176a27ccc44c5c3bb7c8f308c).
0008 seems reasonable. I have added some comments to indicate that
those tests are expected to fail, and why.
> I think we should squash 0002 to 0007.
>
Yes, let's do that with the next update. In fact, we may as well
squash 0002 to 0008.
Regards,
Dean