On Thu, Jul 22, 2021 at 03:17:52PM -0700, Zhihong Yu wrote: > On Thu, Jul 22, 2021 at 2:59 PM Zhihong Yu <z...@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 <br...@momjian.us> https://momjian.us EDB https://enterprisedb.com If only the physical world exists, free will is an illusion.
interval.diff.gz
Description: application/gzip