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

From Bryn Llewellyn
Subject Re: Have I found an interval arithmetic bug?
Date
Msg-id A73D4699-C3A4-463A-8F33-6E9C40130C66@yugabyte.com
Whole thread Raw
In response to Re: Have I found an interval arithmetic bug?  (Bruce Momjian <bruce@momjian.us>)
Responses Re: Have I found an interval arithmetic bug?
List pgsql-hackers
On 23-Jul-2021, at 08:05, Bruce Momjian <bruce@momjian.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@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://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.

pgsql-hackers by date:

Previous
From: "Bossart, Nathan"
Date:
Subject: Re: Avoiding data loss with synchronous replication
Next
From: Egor Rogov
Date:
Subject: Re: pg_stats and range statistics