On Wed, Jul 21, 2021 at 01:29:49PM -0400, Tom Lane wrote:
> Bryn Llewellyn <b...@yugabyte.com> writes:
> > It was me that started the enormous thread with the title “Have I found an
> > interval arithmetic bug?” on 01-Apr-2021. I presented this testcase:
>
> >> 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
>
> > The consensus was that the outcome that I flagged with “wrong” does indeed
> > have that status.
>
> Yeah, I think it's self-evident that your last four cases should
> produce the same results. Whether '9 mons 12 days' is the best
> possible result is debatable --- in a perfect world, maybe we'd
> produce '9 mons' exactly --- but given that the first two cases
> produce what they do, that does seem self-consistent. I think
> we should be setting out to fix that outlier without causing
> any of the other five results to change.
OK, I decided to reverse some of the changes I was proposing once I
started to think about the inaccuracy of not spilling down from 'weeks'
to seconds when hours also appear. The fundamental issue is that the
months-to-days conversion is almost always an approximation, while the
days to seconds conversion is almost always accurate. This means we are
never going to have consistent spill-down that is useful.
Therefore, I went ahead and accepted that years and larger units spill
only to months, months spill only to days, and weeks and lower spill all
the way down to seconds. I also spelled this out in the docs, and
explained why we have this behavior.
Also, with my patch, the last four queries return the same result
because of the proper rounding also added by the patch, attached.
--
Bruce Momjian <br...@momjian.us> https://momjian.us
EDB https://enterprisedb.com
If only the physical world exists, free will is an illusion.
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index e016f96fb4..e831242bf6 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -2800,15 +2800,18 @@ P <optional> <replaceable>years</replaceable>-<replaceable>months</replaceable>-
</para>
<para>
- In the verbose input format, and in some fields of the more compact
- input formats, field values can have fractional parts; for example
- <literal>'1.5 week'</literal> or <literal>'01:02:03.45'</literal>. Such input is
- converted to the appropriate number of months, days, and seconds
- for storage. When this would result in a fractional number of
- months or days, the fraction is added to the lower-order fields
- using the conversion factors 1 month = 30 days and 1 day = 24 hours.
- For example, <literal>'1.5 month'</literal> becomes 1 month and 15 days.
- Only seconds will ever be shown as fractional on output.
+ Field values can have fractional parts; for example, <literal>'1.5
+ weeks'</literal> or <literal>'01:02:03.45'</literal>. However,
+ because interval internally stores only three integer units (months,
+ days, seconds), fractional units must be spilled to smaller units.
+ For example, because months are approximated to equal 30 days,
+ fractional values of units greater than months is rounded to be the
+ nearest integer number of months. Fractional months are rounded to
+ be the nearest integer number of days, assuming 24 hours per day.
+ Fractional units smaller than months are computed to the nearest
+ second. For example, <literal>'1.5 months'</literal> becomes
+ <literal>1 month 15 days</literal>. Only seconds will ever be shown
+ as fractional on output.
</para>
<para>
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 54ae632de2..5551102447 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -3300,35 +3300,43 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
case DTK_MONTH:
tm->tm_mon += val;
- AdjustFractDays(fval, tm, fsec, DAYS_PER_MONTH);
+ /*
+ * The DAYS_PER_MONTH is an estimate so just round
+ * to days, rather than spilling to seconds.
+ */
+ /* round to a full month? */
+ if (rint(fval * DAYS_PER_MONTH) == DAYS_PER_MONTH)
+ tm->tm_mon++;
+ else
+ tm->tm_mday += rint(fval * DAYS_PER_MONTH);
tmask = DTK_M(MONTH);
break;
case DTK_YEAR:
tm->tm_year += val;
if (fval != 0)
- tm->tm_mon += fval * MONTHS_PER_YEAR;
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR);
tmask = DTK_M(YEAR);
break;
case DTK_DECADE:
tm->tm_year += val * 10;
if (fval != 0)
- tm->tm_mon += fval * MONTHS_PER_YEAR * 10;
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR * 10);
tmask = DTK_M(DECADE);
break;
case DTK_CENTURY:
tm->tm_year += val * 100;
if (fval != 0)
- tm->tm_mon += fval * MONTHS_PER_YEAR * 100;
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR * 100);
tmask = DTK_M(CENTURY);
break;
case DTK_MILLENNIUM:
tm->tm_year += val * 1000;
if (fval != 0)
- tm->tm_mon += fval * MONTHS_PER_YEAR * 1000;
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR * 1000);
tmask = DTK_M(MILLENNIUM);
break;
@@ -3565,11 +3573,15 @@ DecodeISO8601Interval(char *str,
{
case 'Y':
tm->tm_year += val;
- tm->tm_mon += (fval * MONTHS_PER_YEAR);
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR);
break;
case 'M':
tm->tm_mon += val;
- AdjustFractDays(fval, tm, fsec, DAYS_PER_MONTH);
+ /* round to a full month? */
+ if (rint(fval * DAYS_PER_MONTH) == DAYS_PER_MONTH)
+ tm->tm_mon++;
+ else
+ tm->tm_mday += rint(fval * DAYS_PER_MONTH);
break;
case 'W':
tm->tm_mday += val * 7;
@@ -3601,7 +3613,7 @@ DecodeISO8601Interval(char *str,
return DTERR_BAD_FORMAT;
tm->tm_year += val;
- tm->tm_mon += (fval * MONTHS_PER_YEAR);
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR);
if (unit == '\0')
return 0;
if (unit == 'T')
@@ -3615,7 +3627,11 @@ DecodeISO8601Interval(char *str,
if (dterr)
return dterr;
tm->tm_mon += val;
- AdjustFractDays(fval, tm, fsec, DAYS_PER_MONTH);
+ /* round to a full month? */
+ if (rint(fval * DAYS_PER_MONTH) == DAYS_PER_MONTH)
+ tm->tm_mon++;
+ else
+ tm->tm_mday += rint(fval * DAYS_PER_MONTH);
if (*str == '\0')
return 0;
if (*str == 'T')
diff --git a/src/interfaces/ecpg/pgtypeslib/interval.c b/src/interfaces/ecpg/pgtypeslib/interval.c
index 02b3c47223..1a1cb51945 100644
--- a/src/interfaces/ecpg/pgtypeslib/interval.c
+++ b/src/interfaces/ecpg/pgtypeslib/interval.c
@@ -155,11 +155,15 @@ DecodeISO8601Interval(char *str,
{
case 'Y':
tm->tm_year += val;
- tm->tm_mon += (fval * MONTHS_PER_YEAR);
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR);
break;
case 'M':
tm->tm_mon += val;
- AdjustFractDays(fval, tm, fsec, DAYS_PER_MONTH);
+ /* round to a full month? */
+ if (rint(fval * DAYS_PER_MONTH) == DAYS_PER_MONTH)
+ tm->tm_mon++;
+ else
+ tm->tm_mday += rint(fval * DAYS_PER_MONTH);
break;
case 'W':
tm->tm_mday += val * 7;
@@ -191,7 +195,7 @@ DecodeISO8601Interval(char *str,
return DTERR_BAD_FORMAT;
tm->tm_year += val;
- tm->tm_mon += (fval * MONTHS_PER_YEAR);
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR);
if (unit == '\0')
return 0;
if (unit == 'T')
@@ -205,7 +209,11 @@ DecodeISO8601Interval(char *str,
if (dterr)
return dterr;
tm->tm_mon += val;
- AdjustFractDays(fval, tm, fsec, DAYS_PER_MONTH);
+ /* round to a full month? */
+ if (rint(fval * DAYS_PER_MONTH) == DAYS_PER_MONTH)
+ tm->tm_mon++;
+ else
+ tm->tm_mday += rint(fval * DAYS_PER_MONTH);
if (*str == '\0')
return 0;
if (*str == 'T')
@@ -522,35 +530,43 @@ DecodeInterval(char **field, int *ftype, int nf, /* int range, */
case DTK_MONTH:
tm->tm_mon += val;
- AdjustFractDays(fval, tm, fsec, DAYS_PER_MONTH);
+ /*
+ * The DAYS_PER_MONTH is an estimate so just round
+ * to days, rather than spilling to seconds.
+ */
+ /* round to a full month? */
+ if (rint(fval * DAYS_PER_MONTH) == DAYS_PER_MONTH)
+ tm->tm_mon++;
+ else
+ tm->tm_mday += rint(fval * DAYS_PER_MONTH);
tmask = DTK_M(MONTH);
break;
case DTK_YEAR:
tm->tm_year += val;
if (fval != 0)
- tm->tm_mon += fval * MONTHS_PER_YEAR;
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR);
tmask = (fmask & DTK_M(YEAR)) ? 0 : DTK_M(YEAR);
break;
case DTK_DECADE:
tm->tm_year += val * 10;
if (fval != 0)
- tm->tm_mon += fval * MONTHS_PER_YEAR * 10;
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR * 10);
tmask = (fmask & DTK_M(YEAR)) ? 0 : DTK_M(YEAR);
break;
case DTK_CENTURY:
tm->tm_year += val * 100;
if (fval != 0)
- tm->tm_mon += fval * MONTHS_PER_YEAR * 100;
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR * 100);
tmask = (fmask & DTK_M(YEAR)) ? 0 : DTK_M(YEAR);
break;
case DTK_MILLENNIUM:
tm->tm_year += val * 1000;
if (fval != 0)
- tm->tm_mon += fval * MONTHS_PER_YEAR * 1000;
+ tm->tm_mon += rint(fval * MONTHS_PER_YEAR * 1000);
tmask = (fmask & DTK_M(YEAR)) ? 0 : DTK_M(YEAR);
break;
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index e4b1246f45..eae18da946 100644
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -46,6 +46,18 @@ SELECT INTERVAL '1.5 months' AS "One month 15 days";
1 mon 15 days
(1 row)
+SELECT INTERVAL '1.16 months 01:00:00' AS "One mon 5 days one hour";
+ One mon 5 days one hour
+-------------------------
+ 1 mon 5 days 01:00:00
+(1 row)
+
+SELECT INTERVAL '1.99 months' AS "Almost two months";
+ Almost two months
+-------------------
+ 2 mons
+(1 row)
+
SELECT INTERVAL '10 years -11 month -12 days +13:14' AS "9 years...";
9 years...
----------------------------------
diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql
index 6d532398bd..5a6c8924fe 100644
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -13,6 +13,8 @@ SELECT INTERVAL '-1 +02:03' AS "22 hours ago...";
SELECT INTERVAL '-1 days +02:03' AS "22 hours ago...";
SELECT INTERVAL '1.5 weeks' AS "Ten days twelve hours";
SELECT INTERVAL '1.5 months' AS "One month 15 days";
+SELECT INTERVAL '1.16 months 01:00:00' AS "One mon 5 days one hour";
+SELECT INTERVAL '1.99 months' AS "Almost two months";
SELECT INTERVAL '10 years -11 month -12 days +13:14' AS "9 years...";
CREATE TABLE INTERVAL_TBL (f1 interval);
