Re: Have I found an interval arithmetic bug?

> On 23-Jul-2021, at 08:05, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
> On Thu, Jul 22, 2021 at 03:17:52PM -0700, Zhihong Yu wrote:
>> On Thu, Jul 22, 2021 at 2:59 PM Zhihong Yu <zyu(at)yugabyte(dot)com> wrote:
>> Hi,
>>
>> - tm->tm_mon += (fval * MONTHS_PER_YEAR);
>> + tm->tm_mon += rint(fval * MONTHS_PER_YEAR);
>>
>> Should the handling for year use the same check as that for month ?
>>
>> - AdjustFractDays(fval, tm, fsec, DAYS_PER_MONTH);
>> + /* round to a full month? */
>> + if (rint(fval * DAYS_PER_MONTH) == DAYS_PER_MONTH)
>>
>> Cheers
>>
>> Hi,
>> I guess the reason for current patch was that year to months conversion is
>> accurate.
>
> Our internal units are hours/days/seconds, so the spill _up_ from months
> to years happens automatically:
>
> SELECT INTERVAL '23.99 months';
> interval
> ----------
> 2 years
>
>> On the new test:
>>
>> +SELECT INTERVAL '1.16 months 01:00:00' AS "One mon 5 days one hour";
>>
>> 0.16 * 31 = 4.96 < 5
>>
>> I wonder why 5 days were chosen in the test output.
>
> We use 30 days/month, not 31. However, I think you are missing the
> changes in the patch and I am just understanding them fully now. There
> are two big changes:
>
> 1. The amount of spill from months only to days
> 2. The _rounding_ of the result once we stop spilling at months or days
>
> #2 is the part I think you missed.
>
> One thing missing from my previous patch was the handling of negative
> units, which is now handled properly in the attached patch:
>
> SELECT INTERVAL '-1.99 years';
> interval
> ----------
> -2 years
>
> SELECT INTERVAL '-1.99 months';
> interval
> ----------
> -2 mons
>
> I ended up creating a function to handle this, which allowed me to
> simplify some of the surrounding code.
>
> --
> Bruce Momjian <bruce(at)momjian(dot)us> https://www.google.com/url?q=https://momjian.us&source=gmail-imap&ust=1627657554000000&usg=AOvVaw2pMx7QBd3qSjHK1L9oUnl0
> EDB https://www.google.com/url?q=https://enterprisedb.com&source=gmail-imap&ust=1627657554000000&usg=AOvVaw2Q92apfhXmqqFYz7aN16YL
>
> If only the physical world exists, free will is an illusion.
>
> <interval.diff.gz>
>

Will the same new spilldown rules hold in the same way for interval multiplication and division as they will for the interpretation of an interval literal?

The semantics here are (at least as far as my limited search skills have shown me) simply undocumented. But my tests in 13.3 have to date not disproved this hypothesis:

* considering "new_i ◄— i * f"

* # notice that the internal representation is _months_, days, and seconds at odds with "Our internal units are hours/days/seconds,"
* let i’s internal representation be [mm, dd, ss]

* new_i’s “intermediate” internal representation is [mm*f, dd*f, ss*f]

* input these values to the same spilldown algorithm that is applied when these same intermediate values are used in an interval literal

* so the result is [new_mm, new_dd, new_ss]

Here’s an example:

select
'1.2345 months 1.2345 days 1.2345 seconds'::interval =
'1 month 1 day 1 second'::interval*1.2345;

In 13.3, the result is TRUE. (I know that this doesn’t guarantee that the internal representations of the two compared interval values are the same. But it’s a necessary condition for the outcome that I’m referring to and serves to indecate the pont I’m making. A more careful test can be made.