Re: Have I found an interval arithmetic bug?

Bruce:
Thanks for tackling this issue.

The patch looks good to me.
When you have time, can you include the places which were not covered by
the current diff ?

Cheers

On Fri, Apr 2, 2021 at 11:06 AM Bruce Momjian <bruce(at)momjian(dot)us> wrote:

> On Thu, Apr 1, 2021 at 09:46:58PM -0700, Bryn Llewellyn wrote:
> > Or am I misunderstanding something?
> >
> > Try this. The result of each “select” is shown as the trailing comment
> on the
> > same line. I added whitespace by hand to line up the fields.
> >
> > select interval '-1.7 years'; -- -1 years -8
> mons
> >
> > select interval '29.4 months'; -- 2 years 5
> mons 12
> > days
> >
> > select interval '-1.7 years 29.4 months'; -- 8
> mons 12
> > days << wrong
> > select interval '29.4 months -1.7 years'; -- 9
> mons 12
> > days
> >
> > select interval '-1.7 years' + interval '29.4 months'; -- 9
> mons 12
> > days
> > select interval '29.4 months' + interval '-1.7 years'; -- 9
> mons 12
> > days
> >
> > As I reason it, the last four “select” statements are all semantically
> the
> > same. They’re just different syntaxes to add the two intervals the the
> first
> > two “select” statements use separately. There’s one odd man out. And I
> reason
> > this one to be wrong. Is there a flaw in my reasoning?
>
> [Thread moved to hackers.]
>
> Looking at your report, I thought I could easily find the cause, but it
> wasn't obvious. What is happening is that when you cast a float to an
> integer in C, it rounds toward zero, meaning that 8.4 rounds to 8 and
> -8.4 rounds to -8. The big problem here is that -8.4 is rounding in a
> positive direction, while 8.4 rounds in a negative direction. See this:
>
> int(int(-8.4) + 29)
> 21
> int(int(29) + -8.4)
> 20
>
> When you do '-1.7 years' first, it become -8, and then adding 29 yields
> 21. In the other order, it is 29 - 8.4, which yields 20.6, which
> becomes 20. I honestly had never studied this interaction, though you
> would think I would have seen it before. One interesting issue is that
> it only happens when the truncations happen to values with different
> signs --- if they are both positive or negative, it is fine.
>
> The best fix I think is to use rint()/round to round to the closest
> integer, not toward zero. The attached patch does this in a few places,
> but the code needs more research if we are happy with this approach,
> since there are probably other cases. Using rint() does help to produce
> more accurate results, thought the regression tests show no change from
> this patch.
>
> > Further… there’s a notable asymmetry. The fractional part of “1.7 years”
> is 8.4
> > months. But the fractional part of the months value doesn’t spread
> further down
> > into days. However, the fractional part of “29.4 months” (12 days) _does_
> > spread further down into days. What’s the rationale for this asymmetry?
>
> Yes, looking at the code, it seems we only spill down to one unit, not
> more. I think we need to have a discussion if we want to change that.
> I think the idea was that if you specify a non-whole number, you
> probably want to spill down one level, but don't want it spilling all
> the way to milliseconds, which is certainly possible.
>
> > I can’t see that my observations here can be explained by the difference
> > between calendar time and clock time. Here I’m just working with
> non-metric
> > units like feet and inches. One year is just defined as 12 months. And
> one
> > month is just defined as 30 days. All that stuff about adding a month to
> > 3-Feb-2020 taking you to 3-Mar-2020 (same for leap years an non-leap
> years) ,
> > and that other stuff about adding one day to 23:00 on the day before the
> > “spring forward” moment taking you to 23:00 on the next day (i.w. when
> > intervals are added to timestamps) is downstream of simply adding two
> > intervals.
>
> Ah, seems you have done some research. ;-)
>
> --
> Bruce Momjian <bruce(at)momjian(dot)us> https://momjian.us
> EDB https://enterprisedb.com
>
> If only the physical world exists, free will is an illusion.
>
>