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

From Bruce Momjian
Subject Re: Have I found an interval arithmetic bug?
Date
Msg-id 20210723150551.GA8025@momjian.us
Whole thread Raw
In response to Re: Have I found an interval arithmetic bug?  (Zhihong Yu <zyu@yugabyte.com>)
Responses Re: Have I found an interval arithmetic bug?  (Bryn Llewellyn <bryn@yugabyte.com>)
List pgsql-hackers
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@yugabyte.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@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

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


Attachment

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Fix memory leak when output postgres_fdw's "Relations"
Next
From: Ronan Dunklau
Date:
Subject: Re: Showing applied extended statistics in explain