Re: Have I found an interval arithmetic bug? - Mailing list pgsql-hackers

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@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.