Thread: Re: [HACKERS] Interval aggregate regression failure (expected

I used your ideas to make a patch to fix your example:

    test=> select '41 months'::interval  / 10;
       ?column?
    ---------------
     4 mons 3 days
    (1 row)

and

    test=> select '41 months'::interval  * 0.3;
       ?column?
    ---------------
     1 year 9 days
    (1 row)

The trick was not to play with the division, but to check if the number
of seconds cascaded into full days and/or months.

---------------------------------------------------------------------------

Tom Lane wrote:
> Michael Glaesemann <grzm@seespotcode.net> writes:
> > ... I think this just confirms that there is some kind of rounding (or
> > lack of) in interval_div. Kind of frustrating that it's not visible
> > in the result.
>
> I think the fundamental problem is that the float8 results of division
> are inaccurate, and yet we're assuming that we can (for instance) coerce
> them to integer and get exactly the right answer.  For instance, in the
> '41 months'/10 example, I get month_remainder_days being computed as
>
> (gdb) p month_remainder
> $19 = 0.099999999999999645
> (gdb) s
> 2575            result->day += (int32) month_remainder_days;
> (gdb) p month_remainder_days
> $20 = 2.9999999999999893
>
> The only way we can really fix this is to be willing to round off
> the numbers, and I think the only principled way to do that is to
> settle on a specific target accuracy, probably 1 microsecond.
> Then the thing to do would be to scale up all the intermediate
> float results to microseconds and apply rint().  Something like
> (untested)
>
>     month_remainder = rint(span->month * USECS_PER_MONTH / factor);
>     day_remainder = rint(span->day * USECS_PER_DAY / factor);
>     result->month = (int32) (month_remainder / USECS_PER_MONTH);
>     result->day = (int32) (day_remainder / USECS_PER_DAY);
>     month_remainder -= result->month * USECS_PER_MONTH;
>     day_remainder -= result->day * USECS_PER_DAY;
>
>     /*
>      * Handle any fractional parts the same way as in interval_mul.
>      */
>
>     /* fractional months full days into days */
>     month_remainder_days = month_remainder * DAYS_PER_MONTH;
>     extra_days = (int32) (month_remainder_days / USECS_PER_DAY);
>     result->day += extra_days;
>     /* fractional months partial days into time */
>     day_remainder += month_remainder_days - extra_days * USECS_PER_DAY;
>
> #ifdef HAVE_INT64_TIMESTAMP
>     result->time = rint(span->time / factor + day_remainder);
> #else
>     result->time = rint(span->time * 1.0e6 / factor + day_remainder) / 1.0e6;
> #endif
>
> This might need a few more rint() calls --- I'm assuming that float ops
> with exact integral inputs will be OK, which is an assumption used
> pretty widely in the datetime code, but ...
>
> Per the comment, if we do this here we probably want to make
> interval_mul work similarly.
>
>             regards, tom lane

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/backend/utils/adt/timestamp.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
retrieving revision 1.165
diff -c -c -r1.165 timestamp.c
*** src/backend/utils/adt/timestamp.c    13 Jul 2006 16:49:16 -0000    1.165
--- src/backend/utils/adt/timestamp.c    26 Aug 2006 02:36:28 -0000
***************
*** 2526,2531 ****
--- 2526,2546 ----
      result->time = rint(span->time * factor + day_remainder * USECS_PER_DAY);
  #else
      result->time = span->time * factor + day_remainder * SECS_PER_DAY;
+     /*
+      *    The imprecision of float8 causes unusual rounding of even integer
+      *    division, so round up if we have full units.  Check seconds only
+      *    as far as microseconds.
+      */
+     if (TSROUND(result->time) == SECS_PER_DAY)
+     {
+         result->day++;
+         result->time = 0;
+     }
+     if (result->day == DAYS_PER_MONTH)
+     {
+         result->month++;
+         result->day = 0;
+     }
  #endif

      PG_RETURN_INTERVAL_P(result);
***************
*** 2579,2584 ****
--- 2594,2614 ----
      result->time = rint(span->time / factor + day_remainder * USECS_PER_DAY);
  #else
      result->time = span->time / factor + day_remainder * SECS_PER_DAY;
+     /*
+      *    The imprecision of float8 causes unusual rounding of even integer
+      *    division, so round up if we have full units.  Check seconds only
+      *    as far as microseconds.
+      */
+     if (TSROUND(result->time) == SECS_PER_DAY)
+     {
+         result->day++;
+         result->time = 0;
+     }
+     if (result->day == DAYS_PER_MONTH)
+     {
+         result->month++;
+         result->day = 0;
+     }
  #endif

      PG_RETURN_INTERVAL_P(result);

On Aug 26, 2006, at 11:40 , Bruce Momjian wrote:

>
> I used your ideas to make a patch to fix your example:
>
>     test=> select '41 months'::interval  / 10;
>        ?column?
>     ---------------
>      4 mons 3 days
>     (1 row)
>
> and
>
>     test=> select '41 months'::interval  * 0.3;
>        ?column?
>     ---------------
>      1 year 9 days
>     (1 row)
>
> The trick was not to play with the division, but to check if the
> number
> of seconds cascaded into full days and/or months.

While this does provide a fix for the example, I don't believe it's a
complete solution. For example, with your patch, you also get the
following results:

select '41 mon 360:00'::interval / 10 as "pos"
     , '-41 mon -360:00'::interval / 10 as "neg";
           pos           |             neg
------------------------+------------------------------
4 mons 2 days 60:00:00 | -4 mons -2 days -59:59:60.00
(1 row)

If I've done the math right, this should be:
4 mons 3 days 36:00:00 | -4 mons -3 days -36:00:00

select '41 mon -360:00'::interval / 10 as "pos"
     , '-41 mon 360:00'::interval / 10 as "neg";
            pos           |            neg
-------------------------+---------------------------
4 mons 2 days -12:00:00 | -4 mons -2 days +12:00:00
(1 row)

Should be:
4 mons 3 days -36:00:00 | -4 mons -3 days +36:00:00

What we want to do is check just the month contribution to the day
component to see if it is greater than 24 hours. Perhaps the simplest
way to accomplish this is something like (psuedo code):

if (abs(tsround(month_remainder * SECS_PER_DAY)) == SECS_PER_DAY)
{
    if (month_remainder > 0)
    {
       result->month++;
    }
    else
    {
       result->month--;
    }
}

I'm going to try something along these lines this evening.

FWIW, I've included the patch of for what I'm working on. It's pretty
heavily commented right now with expected results as I think through
what the code's doing. (It also includes the DecodeInterval patch I
sent to -patches earlier today.) I'm still getting overflow warnings
in the lines including USECS_PER_DAY and USECS_PER_MONTH, and my
inexperience with C and gdb is getting the best of me right now
(though I'm still plugging away: ).

Michael Glaesemann
grzm seespotcode net

----8<-------------------
? CONFIGURE_ARGS
? datetime.patch
? timestamp.patch
? src/backend/.DS_Store
? src/include/.DS_Store
? src/test/.DS_Store
Index: src/backend/utils/adt/datetime.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/datetime.c,v
retrieving revision 1.169
diff -c -r1.169 datetime.c
*** src/backend/utils/adt/datetime.c    25 Jul 2006 03:51:21 -0000    1.169
--- src/backend/utils/adt/datetime.c    28 Aug 2006 07:08:46 -0000
***************
*** 2920,2935 ****
                           tm->tm_mday += val * 7;
                           if (fval != 0)
                           {
!                             int            sec;
!
!                             fval *= 7 * SECS_PER_DAY;
!                             sec = fval;
!                             tm->tm_sec += sec;
   #ifdef HAVE_INT64_TIMESTAMP
!                             *fsec += (fval - sec) * 1000000;
   #else
!                             *fsec += fval - sec;
   #endif
                           }
                           tmask = (fmask & DTK_M(DAY)) ? 0 : DTK_M(DAY);
                           break;
--- 2920,2942 ----
                           tm->tm_mday += val * 7;
                           if (fval != 0)
                           {
!                             int extra_days;
!                             fval *= 7;
!                             extra_days = (int32) fval;
!                             tm->tm_mday += extra_days;
!                             fval -= extra_days;
!                             if (fval != 0)
!                             {
!                                 int            sec;
!                                 fval *= SECS_PER_DAY;
!                                 sec = fval;
!                                 tm->tm_sec += sec;
   #ifdef HAVE_INT64_TIMESTAMP
!                                 *fsec += (fval - sec) * 1000000;
   #else
!                                 *fsec += fval - sec;
   #endif
+                             }
                           }
                           tmask = (fmask & DTK_M(DAY)) ? 0 : DTK_M(DAY);
                           break;
***************
*** 2938,2953 ****
                           tm->tm_mon += val;
                           if (fval != 0)
                           {
!                             int            sec;
!
!                             fval *= DAYS_PER_MONTH * SECS_PER_DAY;
!                             sec = fval;
!                             tm->tm_sec += sec;
   #ifdef HAVE_INT64_TIMESTAMP
!                             *fsec += (fval - sec) * 1000000;
   #else
!                             *fsec += fval - sec;
   #endif
                           }
                           tmask = DTK_M(MONTH);
                           break;
--- 2945,2967 ----
                           tm->tm_mon += val;
                           if (fval != 0)
                           {
!                             int         day;
!                             fval *= DAYS_PER_MONTH;
!                             day = fval;
!                             tm->tm_mday += day;
!                             fval -= day;
!                             if (fval != 0)
!                             {
!                                 int            sec;
!                                 fval *= SECS_PER_DAY;
!                                 sec = fval;
!                                 tm->tm_sec += sec;
   #ifdef HAVE_INT64_TIMESTAMP
!                                 *fsec += (fval - sec) * 1000000;
   #else
!                                 *fsec += fval - sec;
   #endif
+                             }
                           }
                           tmask = DTK_M(MONTH);
                           break;
Index: src/backend/utils/adt/timestamp.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
retrieving revision 1.165
diff -c -r1.165 timestamp.c
*** src/backend/utils/adt/timestamp.c    13 Jul 2006 16:49:16 -0000    1.165
--- src/backend/utils/adt/timestamp.c    28 Aug 2006 07:08:48 -0000
***************
*** 2547,2556 ****
       Interval   *span = PG_GETARG_INTERVAL_P(0);
       float8        factor = PG_GETARG_FLOAT8(1);
       double        month_remainder,
                   day_remainder,
!                 month_remainder_days;
       Interval   *result;

       result = (Interval *) palloc(sizeof(Interval));

       if (factor == 0.0)
--- 2547,2572 ----
       Interval   *span = PG_GETARG_INTERVAL_P(0);
       float8        factor = PG_GETARG_FLOAT8(1);
       double        month_remainder,
+                 month_remainder_usecs = 0.,
                   day_remainder,
!                 day_remainder_usecs = 0.;
!     int32        extra_days;
       Interval   *result;

+ /*
+    example a:
+    select '1.5 mon'::interval / 10;
+    span = { time = 0, day = 15, month = 1 }
+    factor = 10.0
+    result: 4 days 12:00 { time = 43200.0 (int64 43_200_000_000),
day = 4,  month = 0 }
+
+    example b:
+    select '41 mon'::interval / 10;
+    span = { time = 0, day = 0, month = 41 }
+    factor = 10.0
+    result: 4 mon 3 days { time = 0, day = 3, month = 4 }
+  */
+
       result = (Interval *) palloc(sizeof(Interval));

       if (factor == 0.0)
***************
*** 2559,2584 ****
                    errmsg("division by zero")));

       month_remainder = span->month / factor;
!     day_remainder = span->day / factor;
       result->month = (int32) month_remainder;
!     result->day = (int32) day_remainder;
!     month_remainder -= result->month;
!     day_remainder -= result->day;

       /*
!      * Handle any fractional parts the same way as in interval_mul.
        */

       /* fractional months full days into days */
!     month_remainder_days = month_remainder * DAYS_PER_MONTH;
!     result->day += (int32) month_remainder_days;
!     /* fractional months partial days into time */
!     day_remainder += month_remainder_days - (int32) month_remainder_days;

   #ifdef HAVE_INT64_TIMESTAMP
!     result->time = rint(span->time / factor + day_remainder *
USECS_PER_DAY);
   #else
!     result->time = span->time / factor + day_remainder * SECS_PER_DAY;
   #endif

       PG_RETURN_INTERVAL_P(result);
--- 2575,2682 ----
                    errmsg("division by zero")));

       month_remainder = span->month / factor;
!     /*
!        a: month_remainder = 1 / 10.0 = 0.1
!        b: month_remainder = 41 / 10.0 = 4.1
!      */
       result->month = (int32) month_remainder;
!     /*
!        a: result->month = 0
!        b: result->month = 4
!      */
!      /* FIXME integer overflow */
!     month_remainder_usecs = rint( (double) (month_remainder - result-
 >month)
!                                         * USECS_PER_MONTH);
!     /*
!         a: month_remainder_usecs = rint((0.1 - 0) * ((double)
DAYS_PER_MONTH * (double) USECS_PER_DAY))
!                                  = rint(0.1 * (30.0 * (SECS_PER_DAY * 1000000)))
!                                  = rint(0.1 * 30.0 * 86400 * 1000000)
!                                  = 259_200_000_000
!         b: month_remainder_usecs = rint((4.1 - 4) * ((double)
DAYS_PER_MONTH * (double) USECS_PER_DAY))
!                                  = rint( 0.1 * 30.0 * 86400 * 1000000)
!                                  = 259_200_000_000
!      */
!
!     /*
!      * Due to the inherent inaccuracy of floating point division,
round to
!      * microsecond accuracy. Scale up intermediate results to
microseconds
!      * and scale back down for the final result.
!      */

+     day_remainder = span->day / factor;
       /*
!         a: day_remainder = 15 / 10.0 = 1.5
!         b: day_remainder = 0 / 10.0 = 0.0
!      */
!     result->day = (int32) day_remainder;
!     /*
!         a: result->day = (int32) 1.5 = 1
!         b: result->day = (int32) 0.0 = 0
!      */
!      /* FIXME integer overflow */
!     day_remainder_usecs = rint( (double) (day_remainder - result->day)
* (double) USECS_PER_DAY);
!     /*
!         a: day_remainder_usecs = rint((1.5 - 1) * (SECS_PER_DAY * 1000000))
!                                = rint( 0.5 * 86400 * 1_000_000 )
!                                = 43_200_000_000
!         b: day_remainder_usecs = rint((0.0 - 0 ) * 86400 * 1_000_000)
!                                = 0
!      */
!
!     /*
!      * Handle any fractional parts the same way as in interval_mul.
!      * Fractional months and fractional days are added to the result
separately
!      * to prevent their time fractional components from contributing
to the day
!      * component.
        */

       /* fractional months full days into days */
!     /* FIXME integer overflow */
!     extra_days = (int32) (month_remainder_usecs / (double)
USECS_PER_DAY);
!     /*
!         a: extra_days = (int32) (259_200_000_000 / (SECS_PER_DAY * 1000000))
!                       = (int32) (259_200_000_000 / (86400 * 1000000))
!                       = (int32) 3.0
!                       = 3
!         b: extra_days = (int32) (259_200_000_000 / (86400 * 1000000) )
!                       = (int32) 3.0
!                       = 3
!      */
!     result->day += extra_days;
!     /*
!         a: result->day = 1 + 3 = 4
!         b: result->day = 0 + 3 = 3
!      */
!      /* FIXME integer overflow */
!     month_remainder_usecs -= extra_days * (double) USECS_PER_DAY;
!     /*
!         a: month_remainder_usecs = 259_200_000_000 - (3 * (86400 * 1000000))
!                                  = 259_200_000_000 - (3 * (86400 * 1000000))
!                                  = 259_200_000_000 - 259_200_000_000
!                                  = 0
!         b: month_remainder_usecs = 259_200_000_000 - (3 * (86400 * 1000000))
!                                  = 259_200_000_000 - 259_200_000_000
!                                  = 0
!      */

   #ifdef HAVE_INT64_TIMESTAMP
!     result->time = rint(span->time / factor +
!                             day_remainder_usecs + month_remainder_usecs);
!     /*
!         a: result->time = rint(0.0 / 10.0 + 43_200_000_000 + 0.0)
!                         = 43_200_000_000
!         b: result->time = rint(0.0 / 10.0 + 0 + 0.0)
!                         = 0
!      */
   #else
!     result->time = rint(span->time / factor * 1.0e6 * +
!                             day_remainder_usecs + month_remainder_usecs) / 1.0e6;
!     /*
!         a: result->time = rint(0.0 / 10.0 * 1.0e6 + 43_200_000_000 +
0.0) / 1.0e6
!                         = 43200.0
!         b: result->time = rint(0.0 / 10.0 * 1.0e6 + 0.0 + 0.0) / 1.0e6
!                         = 0.0
!      */
   #endif

       PG_RETURN_INTERVAL_P(result);
Index: src/include/utils/timestamp.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/timestamp.h,v
retrieving revision 1.62
diff -c -r1.62 timestamp.h
*** src/include/utils/timestamp.h    13 Jul 2006 16:49:20 -0000    1.62
--- src/include/utils/timestamp.h    28 Aug 2006 07:08:51 -0000
***************
*** 79,92 ****
   #define SECS_PER_YEAR    (36525 * 864)    /* avoid floating-point
computation */
   #define SECS_PER_DAY    86400
   #define SECS_PER_HOUR    3600
! #define SECS_PER_MINUTE 60
   #define MINS_PER_HOUR    60

   #ifdef HAVE_INT64_TIMESTAMP
   #define USECS_PER_DAY    INT64CONST(86400000000)
   #define USECS_PER_HOUR    INT64CONST(3600000000)
! #define USECS_PER_MINUTE INT64CONST(60000000)
   #define USECS_PER_SEC    INT64CONST(1000000)
   #endif

   /*
--- 79,96 ----
   #define SECS_PER_YEAR    (36525 * 864)    /* avoid floating-point
computation */
   #define SECS_PER_DAY    86400
   #define SECS_PER_HOUR    3600
! #define SECS_PER_MINUTE    60
   #define MINS_PER_HOUR    60

   #ifdef HAVE_INT64_TIMESTAMP
+ #define USECS_PER_MONTH    INT64CONST(2592000000000)
   #define USECS_PER_DAY    INT64CONST(86400000000)
   #define USECS_PER_HOUR    INT64CONST(3600000000)
! #define USECS_PER_MINUTE    INT64CONST(60000000)
   #define USECS_PER_SEC    INT64CONST(1000000)
+ #else
+ #define USECS_PER_DAY    (SECS_PER_DAY * 1000000)
+ #define USECS_PER_MONTH ((double) DAYS_PER_MONTH * (double)
USECS_PER_DAY)
   #endif

   /*

I think I've got it. I plan to update the regression tests this
evening, but I wanted to post what I believe is a solution.

select '41 mon'::interval / 10;
    ?column?
---------------
4 mons 3 days
(1 row)

select '41 mon 360:00'::interval / 10 as "pos"
     , '-41 mon -360:00'::interval / 10 as "neg";
           pos           |            neg
------------------------+---------------------------
4 mons 3 days 36:00:00 | -4 mons -3 days -36:00:00
(1 row)

select '41 mon -360:00'::interval / 10 as "pos"
     , '-41 mon 360:00'::interval / 10 as "neg";
            pos           |            neg
-------------------------+---------------------------
4 mons 3 days -36:00:00 | -4 mons -3 days +36:00:00
(1 row)

If anyone sees anything untoward, please let me know and I'll do my
best to fix it. Also, should the duplicate code in interval_mul and
interval_div be refactored into its own function?

Thanks!

Michael Glaesemann
grzm seespotcode net

---8<-----
Index: src/backend/utils/adt/timestamp.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
retrieving revision 1.165
diff -c -r1.165 timestamp.c
*** src/backend/utils/adt/timestamp.c    13 Jul 2006 16:49:16 -0000    1.165
--- src/backend/utils/adt/timestamp.c    29 Aug 2006 06:20:03 -0000
***************
*** 2494,2500 ****
       float8        factor = PG_GETARG_FLOAT8(1);
       double        month_remainder,
                   day_remainder,
!                 month_remainder_days;
       Interval   *result;

       result = (Interval *) palloc(sizeof(Interval));
--- 2494,2502 ----
       float8        factor = PG_GETARG_FLOAT8(1);
       double        month_remainder,
                   day_remainder,
!                 month_remainder_days,
!                 month_remainder_day_frac,
!                 month_remainder_time;
       Interval   *result;

       result = (Interval *) palloc(sizeof(Interval));
***************
*** 2519,2526 ****
       /* fractional months full days into days */
       month_remainder_days = month_remainder * DAYS_PER_MONTH;
       result->day += (int32) month_remainder_days;
!     /* fractional months partial days into time */
!     day_remainder += month_remainder_days - (int32) month_remainder_days;

   #ifdef HAVE_INT64_TIMESTAMP
       result->time = rint(span->time * factor + day_remainder *
USECS_PER_DAY);
--- 2521,2556 ----
       /* fractional months full days into days */
       month_remainder_days = month_remainder * DAYS_PER_MONTH;
       result->day += (int32) month_remainder_days;
!
!     month_remainder_day_frac = month_remainder_days - (int32)
month_remainder_days;
!
! #ifdef HAVE_INT64_TIMESTAMP
!     month_remainder_day_frac = month_remainder_days - (int32)
month_remainder_days;
!     month_remainder_time = month_remainder_day_frac * USECS_PER_DAY;
!     if (rint(month_remainder_time) == USECS_PER_DAY)
!     {
!        result->day++;
!     }
!     else if ((rint(month_remainder_time)) == -USECS_PER_DAY)
!     {
!        result->day--;
!     }
! #else
!     month_remainder_time = month_remainder_day_frac * SECS_PER_DAY;
!     if ((TSROUND(month_remainder_time) == SECS_PER_DAY))
!     {
!        result->day++;
!     }
!     else if ((TSROUND(month_remainder_time) == -SECS_PER_DAY))
!     {
!        result->day--;
!     }
! #endif
!     else
!     {
!         /* fractional months partial days into time */
!         day_remainder += month_remainder_day_frac;
!     }

   #ifdef HAVE_INT64_TIMESTAMP
       result->time = rint(span->time * factor + day_remainder *
USECS_PER_DAY);
***************
*** 2548,2558 ****
       float8        factor = PG_GETARG_FLOAT8(1);
       double        month_remainder,
                   day_remainder,
!                 month_remainder_days;
       Interval   *result;

       result = (Interval *) palloc(sizeof(Interval));

       if (factor == 0.0)
           ereport(ERROR,
                   (errcode(ERRCODE_DIVISION_BY_ZERO),
--- 2578,2596 ----
       float8        factor = PG_GETARG_FLOAT8(1);
       double        month_remainder,
                   day_remainder,
!                 month_remainder_days,
!                 month_remainder_day_frac,
!                 month_remainder_time;
       Interval   *result;

       result = (Interval *) palloc(sizeof(Interval));

+     /*
+         a: (fl) select '41 mon'::interval / 10;
+         *span = { time = 0., day = 0, month = 41 }
+         factor = 10.
+      */
+
       if (factor == 0.0)
           ereport(ERROR,
                   (errcode(ERRCODE_DIVISION_BY_ZERO),
***************
*** 2560,2579 ****

       month_remainder = span->month / factor;
       day_remainder = span->day / factor;
       result->month = (int32) month_remainder;
-     result->day = (int32) day_remainder;
       month_remainder -= result->month;
-     day_remainder -= result->day;

!     /*
!      * Handle any fractional parts the same way as in interval_mul.
!      */

       /* fractional months full days into days */
       month_remainder_days = month_remainder * DAYS_PER_MONTH;
       result->day += (int32) month_remainder_days;
!     /* fractional months partial days into time */
!     day_remainder += month_remainder_days - (int32) month_remainder_days;

   #ifdef HAVE_INT64_TIMESTAMP
       result->time = rint(span->time / factor + day_remainder *
USECS_PER_DAY);
--- 2598,2648 ----

       month_remainder = span->month / factor;
       day_remainder = span->day / factor;
+
       result->month = (int32) month_remainder;
       month_remainder -= result->month;

!     result->day = (int32) day_remainder;
!     day_remainder -= result->day;

       /* fractional months full days into days */
       month_remainder_days = month_remainder * DAYS_PER_MONTH;
       result->day += (int32) month_remainder_days;
!
!     month_remainder_day_frac = month_remainder_days - (int32)
month_remainder_days;
!
!     /*
!      * Add to result->day any fractional days from the month
component that
!      * round to 24 hour periods after being converted to time.
!      * Handle any fractional parts the same way as in interval_mul.
!      */
!
! #ifdef HAVE_INT64_TIMESTAMP
!     month_remainder_time = month_remainder_day_frac * USECS_PER_DAY;
!     if (rint(month_remainder_time) == USECS_PER_DAY)
!     {
!        result->day++;
!     }
!     else if ((rint(month_remainder_time)) == -USECS_PER_DAY)
!     {
!        result->day--;
!     }
! #else
!     month_remainder_time = month_remainder_day_frac * SECS_PER_DAY;
!     if ((TSROUND(month_remainder_time) == SECS_PER_DAY))
!     {
!        result->day++;
!     }
!     else if ((TSROUND(month_remainder_time) == -SECS_PER_DAY))
!     {
!        result->day--;
!     }
! #endif
!     else
!     {
!         /* fractional months partial days into time */
!         day_remainder += month_remainder_day_frac;
!     }

   #ifdef HAVE_INT64_TIMESTAMP
       result->time = rint(span->time / factor + day_remainder *
USECS_PER_DAY);

On Aug 29, 2006, at 15:38 , Michael Glaesemann wrote:

> I think I've got it. I plan to update the regression tests this
> evening, but I wanted to post what I believe is a solution.

I've cleaned up the patch a bit in terms of whitespace, comments, and
parens. I've also updated the interval and horology regression tests.
The horology tests needed updating because I added 5 rows to
INTERVAL_TBL. I didn't check the math for every row of time(tz |
stamp | stamptz)/interval arithmetic in the horology tests as I think
problems in this area would have shown up before. Does that make
sense or it just rationalization on my part?

Both with and without --enable-integer-datetimes pass the regression
tests.

Thanks!

Michael Glaesemann
grzm seespotcode net

Michael Glaesemann wrote:
>
> On Aug 29, 2006, at 15:38 , Michael Glaesemann wrote:
>
> > I think I've got it. I plan to update the regression tests this
> > evening, but I wanted to post what I believe is a solution.
>
> I've cleaned up the patch a bit in terms of whitespace, comments, and
> parens. I've also updated the interval and horology regression tests.
> The horology tests needed updating because I added 5 rows to
> INTERVAL_TBL. I didn't check the math for every row of time(tz |
> stamp | stamptz)/interval arithmetic in the horology tests as I think
> problems in this area would have shown up before. Does that make
> sense or it just rationalization on my part?
>
> Both with and without --enable-integer-datetimes pass the regression
> tests.

Uh, I came up with a cleaner one, I think.  I didn't test
--enable-integer-datetimes yet.

I tested a few of your examples:

    test=> select '41 mon 10:00:00'::interval / 10 as "pos";
              pos
    ------------------------
     4 mons 3 days 01:00:00
    (1 row)

It basically rounds the remainders to full values if they are close to
full (+/- 0.000001).

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/backend/utils/adt/timestamp.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
retrieving revision 1.165
diff -c -c -r1.165 timestamp.c
*** src/backend/utils/adt/timestamp.c    13 Jul 2006 16:49:16 -0000    1.165
--- src/backend/utils/adt/timestamp.c    29 Aug 2006 16:06:49 -0000
***************
*** 2505,2510 ****
--- 2505,2513 ----
      result->day = (int32) day_remainder;
      month_remainder -= result->month;
      day_remainder -= result->day;
+     if (day_remainder != (int32)day_remainder &&
+         TSROUND(day_remainder) == rint(day_remainder))
+         day_remainder = rint(day_remainder);

      /*
       * The above correctly handles the whole-number part of the month and day
***************
*** 2518,2523 ****
--- 2521,2529 ----

      /* fractional months full days into days */
      month_remainder_days = month_remainder * DAYS_PER_MONTH;
+     if (month_remainder_days != (int32)month_remainder_days &&
+         TSROUND(month_remainder_days) == rint(month_remainder_days))
+         month_remainder_days = rint(month_remainder_days);
      result->day += (int32) month_remainder_days;
      /* fractional months partial days into time */
      day_remainder += month_remainder_days - (int32) month_remainder_days;
***************
*** 2564,2569 ****
--- 2570,2578 ----
      result->day = (int32) day_remainder;
      month_remainder -= result->month;
      day_remainder -= result->day;
+     if (day_remainder != (int32)day_remainder &&
+         TSROUND(day_remainder) == rint(day_remainder))
+         day_remainder = rint(day_remainder);

      /*
       * Handle any fractional parts the same way as in interval_mul.
***************
*** 2571,2576 ****
--- 2580,2588 ----

      /* fractional months full days into days */
      month_remainder_days = month_remainder * DAYS_PER_MONTH;
+     if (month_remainder_days != (int32)month_remainder_days &&
+         TSROUND(month_remainder_days) == rint(month_remainder_days))
+         month_remainder_days = rint(month_remainder_days);
      result->day += (int32) month_remainder_days;
      /* fractional months partial days into time */
      day_remainder += month_remainder_days - (int32) month_remainder_days;

On Aug 30, 2006, at 1:13 , Bruce Momjian wrote:

> Uh, I came up with a cleaner one, I think.  I didn't test
> --enable-integer-datetimes yet.

Cool. It's indeed much cleaner. Thanks, Bruce. I'm about to head to
bed, but I'll look at it more closely tomorrow.

I also noticed that my regression tests didn't exercise the code I
thought it did. If you have a chance before I get to it, you might
want to try these as well:

select interval '41 mon 12 days 360:00' / 10 as quotient_a
     , interval '41 mon -12 days -360:00' / 10 as quotient_b
     , interval '-41 mon 12 days 360:00' / 10 as quotient_c
     , interval '-41 mon -12 days -360:00' / 10 as quotient_d;
        quotient_a       |       quotient_b        |
quotient_c         |        quotient_d
------------------------+-------------------------
+---------------------------+---------------------------
4 mons 4 days 40:48:00 | 4 mons 2 days -40:48:00 | -4 mons -2 days
+40:48:00 | -4 mons -4 days -40:48:00
(1 row)

select interval '41 mon 12 days 360:00' * 0.3 as product_a
     , interval '41 mon -12 days -360:00' * 0.3 as product_b
     , interval '-41 mon 12 days 360:00' * 0.3 as product_c
     , interval '-41 mon -12 days -360:00' * 0.3 as product_d;
         product_a         |          product_b          |
product_c          |            product_d
--------------------------+-----------------------------
+-----------------------------+---------------------------------
1 year 12 days 122:24:00 | 1 year 6 days -122:23:60.00 | -1 years -6
days +122:24:00 | -1 years -12 days -122:23:60.00
(1 row)

The quotients look fine, but I'm wondering if another set of rounding
is needed to bump those -122:23:60.00 to -122:24:00 in product_b and
product_d.

Michael Glaesemann
grzm seespotcode net

Michael Glaesemann wrote:
>
> On Aug 30, 2006, at 1:13 , Bruce Momjian wrote:
>
> > Uh, I came up with a cleaner one, I think.  I didn't test
> > --enable-integer-datetimes yet.
>
> Cool. It's indeed much cleaner. Thanks, Bruce. I'm about to head to
> bed, but I'll look at it more closely tomorrow.
>
> I also noticed that my regression tests didn't exercise the code I
> thought it did. If you have a chance before I get to it, you might
> want to try these as well:
>
> select interval '41 mon 12 days 360:00' / 10 as quotient_a
>      , interval '41 mon -12 days -360:00' / 10 as quotient_b
>      , interval '-41 mon 12 days 360:00' / 10 as quotient_c
>      , interval '-41 mon -12 days -360:00' / 10 as quotient_d;
>         quotient_a       |       quotient_b        |
> quotient_c         |        quotient_d
> ------------------------+-------------------------
> +---------------------------+---------------------------
> 4 mons 4 days 40:48:00 | 4 mons 2 days -40:48:00 | -4 mons -2 days
> +40:48:00 | -4 mons -4 days -40:48:00
> (1 row)
>
> select interval '41 mon 12 days 360:00' * 0.3 as product_a
>      , interval '41 mon -12 days -360:00' * 0.3 as product_b
>      , interval '-41 mon 12 days 360:00' * 0.3 as product_c
>      , interval '-41 mon -12 days -360:00' * 0.3 as product_d;
>          product_a         |          product_b          |
> product_c          |            product_d
> --------------------------+-----------------------------
> +-----------------------------+---------------------------------
> 1 year 12 days 122:24:00 | 1 year 6 days -122:23:60.00 | -1 years -6
> days +122:24:00 | -1 years -12 days -122:23:60.00
> (1 row)
>
> The quotients look fine, but I'm wondering if another set of rounding
> is needed to bump those -122:23:60.00 to -122:24:00 in product_b and
> product_d.

Here are the results using my newest patch:

    test=> select interval '41 mon 12 days 360:00' / 10 as quotient_a
         , interval '41 mon -12 days -360:00' / 10 as quotient_b
         , interval '-41 mon 12 days 360:00' / 10 as quotient_c
         , interval '-41 mon -12 days -360:00' / 10 as quotient_d;
           quotient_a       |       quotient_b        |        quotient_c         |        quotient_d
    ------------------------+-------------------------+---------------------------+---------------------------
     4 mons 4 days 40:48:00 | 4 mons 2 days -40:48:00 | -4 mons -2 days +40:48:00 | -4 mons -4 days -40:48:00
    (1 row)

    test=> select interval '41 mon 12 days 360:00' * 0.3 as product_a
         , interval '41 mon -12 days -360:00' * 0.3 as product_b
         , interval '-41 mon 12 days 360:00' * 0.3 as product_c
         , interval '-41 mon -12 days -360:00' * 0.3 as product_d;
            product_a         |        product_b         |          product_c          |          product_d
    --------------------------+--------------------------+-----------------------------+------------------------------
     1 year 12 days 122:24:00 | 1 year 6 days -122:24:00 | -1 years -6 days +122:24:00 | -1 years -12 days -122:24:00
    (1 row)

I see no "23:60" entries.

I realize the problem with my first patch.  I was rounding at the
'seconds' level, but that is too late in the process.  The rounding has
to happen right after the division.  In fact the only rounding problem I
can find is with month_remainder_days, because of a division by factor,
and a multiplication to convert it to days.  The combination of steps
is where the rounding problem is happening.  The patch is even smaller
now.

The code assume if it is within 0.000001 of a whole number, it should be
rounded to a whole number. Patch attached with comments added.

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/backend/utils/adt/timestamp.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
retrieving revision 1.165
diff -c -c -r1.165 timestamp.c
*** src/backend/utils/adt/timestamp.c    13 Jul 2006 16:49:16 -0000    1.165
--- src/backend/utils/adt/timestamp.c    29 Aug 2006 22:04:41 -0000
***************
*** 2518,2523 ****
--- 2518,2530 ----

      /* fractional months full days into days */
      month_remainder_days = month_remainder * DAYS_PER_MONTH;
+     /*
+      *    The remainders suffer from float rounding, so if they are
+      *    within 0.000001 of an integer, we round them to integers.
+      */
+     if (month_remainder_days != (int32)month_remainder_days &&
+         TSROUND(month_remainder_days) == rint(month_remainder_days))
+         month_remainder_days = rint(month_remainder_days);
      result->day += (int32) month_remainder_days;
      /* fractional months partial days into time */
      day_remainder += month_remainder_days - (int32) month_remainder_days;
***************
*** 2571,2576 ****
--- 2578,2590 ----

      /* fractional months full days into days */
      month_remainder_days = month_remainder * DAYS_PER_MONTH;
+     /*
+      *    The remainders suffer from float rounding, so if they are
+      *    within 0.000001 of an integer, we round them to integers.
+      */
+     if (month_remainder_days != (int32)month_remainder_days &&
+         TSROUND(month_remainder_days) == rint(month_remainder_days))
+         month_remainder_days = rint(month_remainder_days);
      result->day += (int32) month_remainder_days;
      /* fractional months partial days into time */
      day_remainder += month_remainder_days - (int32) month_remainder_days;

On Aug 30, 2006, at 7:12 , Bruce Momjian wrote:

> Here are the results using my newest patch:
>
>     test=> select interval '41 mon 12 days 360:00' / 10 as quotient_a
>          , interval '41 mon -12 days -360:00' / 10 as quotient_b
>          , interval '-41 mon 12 days 360:00' / 10 as quotient_c
>          , interval '-41 mon -12 days -360:00' / 10 as quotient_d;
>            quotient_a       |       quotient_b        |
> quotient_c         |        quotient_d
>     ------------------------+-------------------------
> +---------------------------+---------------------------
>      4 mons 4 days 40:48:00 | 4 mons 2 days -40:48:00 | -4 mons -2
> days +40:48:00 | -4 mons -4 days -40:48:00
>     (1 row)
>
>     test=> select interval '41 mon 12 days 360:00' * 0.3 as product_a
>          , interval '41 mon -12 days -360:00' * 0.3 as product_b
>          , interval '-41 mon 12 days 360:00' * 0.3 as product_c
>          , interval '-41 mon -12 days -360:00' * 0.3 as product_d;
>             product_a         |        product_b         |
> product_c          |          product_d
>     --------------------------+--------------------------
> +-----------------------------+------------------------------
>      1 year 12 days 122:24:00 | 1 year 6 days -122:24:00 | -1 years -6
> days +122:24:00 | -1 years -12 days -122:24:00
>     (1 row)
>
> I see no "23:60" entries.

Using Bruce's newest patch, I still get the "23:60" entries on my
machine (no integer-datetimes)

select version();

version
------------------------------------------------------------------------
------------------------------------------------------------------------
-
PostgreSQL 8.2devel on powerpc-apple-darwin8.7.0, compiled by GCC
powerpc-apple-darwin8-gcc-4.0.1 (GCC) 4.0.1 (Apple Computer, Inc.
build 5341)
(1 row)

select interval '41 mon 12 days 360:00' / 10 as quotient_a
     , interval '41 mon -12 days -360:00' / 10 as quotient_b
     , interval '-41 mon 12 days 360:00' / 10 as quotient_c
     , interval '-41 mon -12 days -360:00' / 10 as quotient_d;
        quotient_a       |       quotient_b        |
quotient_c         |        quotient_d
------------------------+-------------------------
+---------------------------+---------------------------
4 mons 4 days 40:48:00 | 4 mons 2 days -40:48:00 | -4 mons -2 days
+40:48:00 | -4 mons -4 days -40:48:00
(1 row)

select interval '41 mon 12 days 360:00' * 0.3 as product_a
     , interval '41 mon -12 days -360:00' * 0.3 as product_b
     , interval '-41 mon 12 days 360:00' * 0.3 as product_c
     , interval '-41 mon -12 days -360:00' * 0.3 as product_d;
         product_a         |          product_b          |
product_c          |            product_d
--------------------------+-----------------------------
+-----------------------------+---------------------------------
1 year 12 days 122:24:00 | 1 year 6 days -122:23:60.00 | -1 years -6
days +122:24:00 | -1 years -12 days -122:23:60.00
(1 row)


> The code assume if it is within 0.000001 of a whole number, it
> should be
> rounded to a whole number. Patch attached with comments added.

>       /* fractional months full days into days */
>       month_remainder_days = month_remainder * DAYS_PER_MONTH;
> +     /*
> +      *    The remainders suffer from float rounding, so if they are
> +      *    within 0.000001 of an integer, we round them to integers.
> +      */
> +     if (month_remainder_days != (int32)month_remainder_days &&
> +         TSROUND(month_remainder_days) == rint(month_remainder_days))
> +         month_remainder_days = rint(month_remainder_days);
>       result->day += (int32) month_remainder_days;
>

Don't we want to be checking for rounding at the usec level rather
than 0.000001 of a day? I think this should be

    if (month_remainder_days != (int32)month_remainder_days &&
        TSROUND(month_remainder_days * SECS_PER_DAY) ==
                                    rint(month_remainder_days * SECS_PER_DAY))
        month_remainder_days = rint(month_remainder_days);

Another question I have concerns the month_remainder_days != (int32)
month_remainder_days comparison. If I understand it correctly, if the
TSROUND == rint portion is true, the first part is true. Or is this
just a quick, fast check to see if it's necessary to do a more
computationally intensive check?

TSROUND isn't defined for HAVE_INT64_TIMESTAMP. My first attempt at
performing a corresponding comparison doesn't work:

+     if (month_remainder_days != (int32) month_remainder_days &&
+ #ifdef HAVE_INT64_TIMESTAMP
+         rint(month_remainder_days * USECS_PER_DAY) ==
+            (month_remainder_days * USECS_PER_DAY))
+ #else
+         TSROUND(month_remainder_days * SECS_PER_DAY) ==
+            rint(month_remainder_days * SECS_PER_DAY))
+ #endif
+         month_remainder_days = rint(month_remainder_days);

Anyway, I'll pound on this some more tonight.

Michael Glaesemann
grzm seespotcode net

----------------------------------------

Index: src/backend/utils/adt/timestamp.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
retrieving revision 1.165
diff -c -r1.165 timestamp.c
*** src/backend/utils/adt/timestamp.c    13 Jul 2006 16:49:16 -0000    1.165
--- src/backend/utils/adt/timestamp.c    30 Aug 2006 00:48:37 -0000
***************
*** 2518,2523 ****
--- 2518,2536 ----

       /* fractional months full days into days */
       month_remainder_days = month_remainder * DAYS_PER_MONTH;
+     /*
+      *    The remainders suffer from float rounding, so if they are
+      *    within 0.000001 of an integer, we round them to integers.
+      */
+     if (month_remainder_days != (int32) month_remainder_days &&
+ #ifdef HAVE_INT64_TIMESTAMP
+         rint(month_remainder_days * USECS_PER_DAY) ==
+            (month_remainder_days * USECS_PER_DAY))
+ #else
+         TSROUND(month_remainder_days * SECS_PER_DAY) ==
+            rint(month_remainder_days * SECS_PER_DAY))
+ #endif
+         month_remainder_days = rint(month_remainder_days);
       result->day += (int32) month_remainder_days;
       /* fractional months partial days into time */
       day_remainder += month_remainder_days - (int32)
month_remainder_days;
***************
*** 2571,2576 ****
--- 2584,2602 ----

       /* fractional months full days into days */
       month_remainder_days = month_remainder * DAYS_PER_MONTH;
+     /*
+      *    The remainders suffer from float rounding, so if they are
+      *    within 0.000001 of an integer, we round them to integers.
+      */
+     if (month_remainder_days != (int32) month_remainder_days &&
+ #ifdef HAVE_INT64_TIMESTAMP
+         rint(month_remainder_days * USECS_PER_DAY) ==
+            (month_remainder_days * USECS_PER_DAY))
+ #else
+         TSROUND(month_remainder_days * SECS_PER_DAY) ==
+            rint(month_remainder_days * SECS_PER_DAY))
+ #endif
+         month_remainder_days = rint(month_remainder_days);
       result->day += (int32) month_remainder_days;
       /* fractional months partial days into time */
       day_remainder += month_remainder_days - (int32)
month_remainder_days;

Michael Glaesemann wrote:
> On Aug 30, 2006, at 7:12 , Bruce Momjian wrote:
>
> > Here are the results using my newest patch:
> >
> >     test=> select interval '41 mon 12 days 360:00' / 10 as quotient_a
> >          , interval '41 mon -12 days -360:00' / 10 as quotient_b
> >          , interval '-41 mon 12 days 360:00' / 10 as quotient_c
> >          , interval '-41 mon -12 days -360:00' / 10 as quotient_d;
> >            quotient_a       |       quotient_b        |
> > quotient_c         |        quotient_d
> >     ------------------------+-------------------------
> > +---------------------------+---------------------------
> >      4 mons 4 days 40:48:00 | 4 mons 2 days -40:48:00 | -4 mons -2
> > days +40:48:00 | -4 mons -4 days -40:48:00
> >     (1 row)
> >
> >     test=> select interval '41 mon 12 days 360:00' * 0.3 as product_a
> >          , interval '41 mon -12 days -360:00' * 0.3 as product_b
> >          , interval '-41 mon 12 days 360:00' * 0.3 as product_c
> >          , interval '-41 mon -12 days -360:00' * 0.3 as product_d;
> >             product_a         |        product_b         |
> > product_c          |          product_d
> >     --------------------------+--------------------------
> > +-----------------------------+------------------------------
> >      1 year 12 days 122:24:00 | 1 year 6 days -122:24:00 | -1 years -6
> > days +122:24:00 | -1 years -12 days -122:24:00
> >     (1 row)
> >
> > I see no "23:60" entries.
>
> Using Bruce's newest patch, I still get the "23:60" entries on my
> machine (no integer-datetimes)


Strange, I do not see that here.  Is there something wrong with our
hour/minute display?  Someone posted a patch a few days ago for that.

Here is a test program.  What does it show for you?

    #include <stdio.h>


    int
    main(int argc, char *argv[])
    {
        double x;

        x = 41;
        x = x / 10.0;
        printf("%f\n", x);
        x = x - (int)x;
        x = x * 30;
        printf("%15.15f\n", x);
        x = 0.1 * 30;
        printf("%15.15f\n", x);
        return 0;
    }

The output for me is:

    4.100000000000000
    2.999999999999989
    3.000000000000000


>
> select version();
>
> version
> ------------------------------------------------------------------------
> ------------------------------------------------------------------------
> -
> PostgreSQL 8.2devel on powerpc-apple-darwin8.7.0, compiled by GCC
> powerpc-apple-darwin8-gcc-4.0.1 (GCC) 4.0.1 (Apple Computer, Inc.
> build 5341)
> (1 row)

Powerpc.  Hmmm.  I am on Intel.

> select interval '41 mon 12 days 360:00' / 10 as quotient_a
>      , interval '41 mon -12 days -360:00' / 10 as quotient_b
>      , interval '-41 mon 12 days 360:00' / 10 as quotient_c
>      , interval '-41 mon -12 days -360:00' / 10 as quotient_d;
>         quotient_a       |       quotient_b        |
> quotient_c         |        quotient_d
> ------------------------+-------------------------
> +---------------------------+---------------------------
> 4 mons 4 days 40:48:00 | 4 mons 2 days -40:48:00 | -4 mons -2 days
> +40:48:00 | -4 mons -4 days -40:48:00
> (1 row)
>
> select interval '41 mon 12 days 360:00' * 0.3 as product_a
>      , interval '41 mon -12 days -360:00' * 0.3 as product_b
>      , interval '-41 mon 12 days 360:00' * 0.3 as product_c
>      , interval '-41 mon -12 days -360:00' * 0.3 as product_d;
>          product_a         |          product_b          |
> product_c          |            product_d
> --------------------------+-----------------------------
> +-----------------------------+---------------------------------
> 1 year 12 days 122:24:00 | 1 year 6 days -122:23:60.00 | -1 years -6
> days +122:24:00 | -1 years -12 days -122:23:60.00
> (1 row)
>

Yea, I see that -122:23:60.00.

> > The code assume if it is within 0.000001 of a whole number, it
> > should be
> > rounded to a whole number. Patch attached with comments added.
>
> >       /* fractional months full days into days */
> >       month_remainder_days = month_remainder * DAYS_PER_MONTH;
> > +     /*
> > +      *    The remainders suffer from float rounding, so if they are
> > +      *    within 0.000001 of an integer, we round them to integers.
> > +      */
> > +     if (month_remainder_days != (int32)month_remainder_days &&
> > +         TSROUND(month_remainder_days) == rint(month_remainder_days))
> > +         month_remainder_days = rint(month_remainder_days);
> >       result->day += (int32) month_remainder_days;
> >
>
> Don't we want to be checking for rounding at the usec level rather
> than 0.000001 of a day? I think this should be
>
>     if (month_remainder_days != (int32)month_remainder_days &&
>         TSROUND(month_remainder_days * SECS_PER_DAY) ==
>         rint(month_remainder_days * SECS_PER_DAY))
>         month_remainder_days = rint(month_remainder_days);
>

Good idea.  I updated the patch to do that.

> Another question I have concerns the month_remainder_days != (int32)
> month_remainder_days comparison. If I understand it correctly, if the
> TSROUND == rint portion is true, the first part is true. Or is this
> just a quick, fast check to see if it's necessary to do a more
> computationally intensive check?

Yea, just an optimization, but I was worried that the computations might
throw problems for certain numbers, so I figured I would only trigger it
when necessary.

> TSROUND isn't defined for HAVE_INT64_TIMESTAMP. My first attempt at
> performing a corresponding comparison doesn't work:
>
> +     if (month_remainder_days != (int32) month_remainder_days &&
> + #ifdef HAVE_INT64_TIMESTAMP
> +         rint(month_remainder_days * USECS_PER_DAY) ==
> +            (month_remainder_days * USECS_PER_DAY))
> + #else
> +         TSROUND(month_remainder_days * SECS_PER_DAY) ==
> +            rint(month_remainder_days * SECS_PER_DAY))
> + #endif
> +         month_remainder_days = rint(month_remainder_days);
>
> Anyway, I'll pound on this some more tonight.

You don't want to do any conditional tests.  SECS_PER_DAY is all you
want.  Those two macros are useful only in representing the internal
storage format, not for float compuations of rounding.

Patch attached.  It also fixes a regression test output too.

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/backend/utils/adt/timestamp.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
retrieving revision 1.165
diff -c -c -r1.165 timestamp.c
*** src/backend/utils/adt/timestamp.c    13 Jul 2006 16:49:16 -0000    1.165
--- src/backend/utils/adt/timestamp.c    30 Aug 2006 03:45:41 -0000
***************
*** 2518,2523 ****
--- 2518,2532 ----

      /* fractional months full days into days */
      month_remainder_days = month_remainder * DAYS_PER_MONTH;
+     /*
+      *    The remainders suffer from float rounding, so if they are
+      *    within 1us of an integer, we round them to integers.
+      *    It seems doing two multiplies is causing the imprecision.
+      */
+     if (month_remainder_days != (int32)month_remainder_days &&
+         TSROUND(month_remainder_days * SECS_PER_DAY) ==
+         rint(month_remainder_days * SECS_PER_DAY))
+         month_remainder_days = rint(month_remainder_days);
      result->day += (int32) month_remainder_days;
      /* fractional months partial days into time */
      day_remainder += month_remainder_days - (int32) month_remainder_days;
***************
*** 2571,2576 ****
--- 2580,2595 ----

      /* fractional months full days into days */
      month_remainder_days = month_remainder * DAYS_PER_MONTH;
+     /*
+      *    The remainders suffer from float rounding, so if they are
+      *    within 1us of an integer, we round them to integers.
+      *    It seems doing a division and multiplies is causing the
+      *    imprecision.
+      */
+     if (month_remainder_days != (int32)month_remainder_days &&
+         TSROUND(month_remainder_days * SECS_PER_DAY) ==
+         rint(month_remainder_days * SECS_PER_DAY))
+         month_remainder_days = rint(month_remainder_days);
      result->day += (int32) month_remainder_days;
      /* fractional months partial days into time */
      day_remainder += month_remainder_days - (int32) month_remainder_days;
Index: src/include/utils/timestamp.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/utils/timestamp.h,v
retrieving revision 1.62
diff -c -c -r1.62 timestamp.h
*** src/include/utils/timestamp.h    13 Jul 2006 16:49:20 -0000    1.62
--- src/include/utils/timestamp.h    30 Aug 2006 03:45:42 -0000
***************
*** 161,171 ****

  typedef double fsec_t;

! /* round off to MAX_TIMESTAMP_PRECISION decimal places */
! /* note: this is also used for rounding off intervals */
  #define TS_PREC_INV 1000000.0
  #define TSROUND(j) (rint(((double) (j)) * TS_PREC_INV) / TS_PREC_INV)
- #endif

  #define TIMESTAMP_MASK(b) (1 << (b))
  #define INTERVAL_MASK(b) (1 << (b))
--- 161,175 ----

  typedef double fsec_t;

! #endif
!
! /*
!  *    Round off to MAX_TIMESTAMP_PRECISION decimal places.
!  *    This is also used for rounding off intervals, and
!  *    for rounding interval multiplication/division calculations.
!  */
  #define TS_PREC_INV 1000000.0
  #define TSROUND(j) (rint(((double) (j)) * TS_PREC_INV) / TS_PREC_INV)

  #define TIMESTAMP_MASK(b) (1 << (b))
  #define INTERVAL_MASK(b) (1 << (b))
Index: src/test/regress/expected/interval.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/interval.out,v
retrieving revision 1.15
diff -c -c -r1.15 interval.out
*** src/test/regress/expected/interval.out    6 Mar 2006 22:49:17 -0000    1.15
--- src/test/regress/expected/interval.out    30 Aug 2006 03:45:43 -0000
***************
*** 218,224 ****
  select avg(f1) from interval_tbl;
                         avg
  -------------------------------------------------
!  @ 4 years 1 mon 9 days 28 hours 18 mins 23 secs
  (1 row)

  -- test long interval input
--- 218,224 ----
  select avg(f1) from interval_tbl;
                         avg
  -------------------------------------------------
!  @ 4 years 1 mon 10 days 4 hours 18 mins 23 secs
  (1 row)

  -- test long interval input

On Aug 30, 2006, at 12:50 , Bruce Momjian wrote:

> Here is a test program.  What does it show for you?

> The output for me is:
>
>     4.100000000000000
>     2.999999999999989
>     3.000000000000000

Here's what I get. Just to make sure I'm doing this right, I'm
including how I compiled it.

$ cat div_test.c
#include <stdio.h>


int
main(int argc, char *argv[])
{
     double x;

     x = 41;
     x = x / 10.0;
     printf("%f\n", x);
     x = x - (int)x;
     x = x * 30;
     printf("%15.15f\n", x);
     x = 0.1 * 30;
     printf("%15.15f\n", x);
     return 0;
}
$ gcc div_test.c -o div_test
$ ./div_test
4.100000
2.999999999999989
3.000000000000000
$

> Yea, just an optimization, but I was worried that the computations
> might
> throw problems for certain numbers, so I figured I would only
> trigger it
> when necessary.

Thanks for the explanation. Helps me know I might actually be
learning this.

> Patch attached.  It also fixes a regression test output too.

Thanks for the patch. I'll look at it more closely tonight.

Michael Glaesemann
grzm seespotcode net

On Aug 30, 2006, at 12:50 , Bruce Momjian wrote:

> Yea, I see that -122:23:60.00.

After applying your patch, I believe that on my machine it's the
contribution from the day component that is producing the 23:60.00.
For example,

select interval '-12 days' * 0.3;
        ?column?
----------------------
-3 days -14:23:60.00
(1 row)

I think some kind of rounding needs to be done to the day
contribution as well. I'm continuing to work on it, but wanted to get
this out there.

Michael Glaesemann
grzm seespotcode net

Michael Glaesemann wrote:
> > Yea, just an optimization, but I was worried that the computations
> > might
> > throw problems for certain numbers, so I figured I would only
> > trigger it
> > when necessary.
>
> Thanks for the explanation. Helps me know I might actually be
> learning this.
>
> > Patch attached.  It also fixes a regression test output too.
>
> Thanks for the patch. I'll look at it more closely tonight.

OK, here is a much nicer patch.  The fix is to do no rounding, but to
find the number of days before applying the factor adjustment.  It
passes all your tests here:

    test=> select '41 mon 10:00:00'::interval / 10 as "pos";
              pos
    ------------------------
     4 mons 3 days 01:00:00
    (1 row)

    test=> select interval '41 mon 12 days 360:00' * 0.3 as product_a
         , interval '41 mon -12 days -360:00' * 0.3 as product_b
         , interval '-41 mon 12 days 360:00' * 0.3 as product_c
         , interval '-41 mon -12 days -360:00' * 0.3 as product_d;
            product_a         |        product_b         |
    product_c          |          product_d
    --------------------------+--------------------------+-----------------------------+------------------------------

     1 year 12 days 122:24:00 | 1 year 6 days -122:24:00 | -1 years -6 days
    +122:24:00 | -1 years -12 days -122:24:00
    (1 row)

    test=> select interval '41 mon 12 days 360:00' / 10 as quotient_a
         , interval '41 mon -12 days -360:00' / 10 as quotient_b
         , interval '-41 mon 12 days 360:00' / 10 as quotient_c
         , interval '-41 mon -12 days -360:00' / 10 as quotient_d;
           quotient_a       |       quotient_b        |        quotient_c
         |        quotient_d
    ------------------------+-------------------------+---------------------------+---------------------------

     4 mons 4 days 40:48:00 | 4 mons 2 days -40:48:00 | -4 mons -2 days
    +40:48:00 | -4 mons -4 days -40:48:00
    (1 row)

What do you see on your platform?

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/backend/utils/adt/timestamp.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
retrieving revision 1.165
diff -c -c -r1.165 timestamp.c
*** src/backend/utils/adt/timestamp.c    13 Jul 2006 16:49:16 -0000    1.165
--- src/backend/utils/adt/timestamp.c    30 Aug 2006 17:08:12 -0000
***************
*** 2496,2501 ****
--- 2496,2502 ----
                  day_remainder,
                  month_remainder_days;
      Interval   *result;
+     int32        orig_month = span->month;

      result = (Interval *) palloc(sizeof(Interval));

***************
*** 2516,2523 ****
       * using justify_hours and/or justify_days.
       */

!     /* fractional months full days into days */
!     month_remainder_days = month_remainder * DAYS_PER_MONTH;
      result->day += (int32) month_remainder_days;
      /* fractional months partial days into time */
      day_remainder += month_remainder_days - (int32) month_remainder_days;
--- 2517,2533 ----
       * using justify_hours and/or justify_days.
       */

!     /*
!      *    Fractional months full days into days.
!      *
!      *    The remainders suffer from float rounding, so instead of
!      *    doing the computation using just the remainder, we calculate
!      *    the total number of days and subtract.  Specifically, we are
!      *    multipling by DAYS_PER_MONTH before dividing by factor.
!      *    This greatly reduces rounding errors.
!      */
!     month_remainder_days = (orig_month * DAYS_PER_MONTH) * factor -
!             result->month * DAYS_PER_MONTH;
      result->day += (int32) month_remainder_days;
      /* fractional months partial days into time */
      day_remainder += month_remainder_days - (int32) month_remainder_days;
***************
*** 2550,2556 ****
                  day_remainder,
                  month_remainder_days;
      Interval   *result;
!
      result = (Interval *) palloc(sizeof(Interval));

      if (factor == 0.0)
--- 2560,2567 ----
                  day_remainder,
                  month_remainder_days;
      Interval   *result;
!     int32        orig_month = span->month;
!
      result = (Interval *) palloc(sizeof(Interval));

      if (factor == 0.0)
***************
*** 2566,2576 ****
      day_remainder -= result->day;

      /*
!      * Handle any fractional parts the same way as in interval_mul.
       */
!
!     /* fractional months full days into days */
!     month_remainder_days = month_remainder * DAYS_PER_MONTH;
      result->day += (int32) month_remainder_days;
      /* fractional months partial days into time */
      day_remainder += month_remainder_days - (int32) month_remainder_days;
--- 2577,2587 ----
      day_remainder -= result->day;

      /*
!      *    Fractional months full days into days.  See comment in
!      *    interval_mul().
       */
!     month_remainder_days = (orig_month * DAYS_PER_MONTH) / factor -
!             result->month * DAYS_PER_MONTH;
      result->day += (int32) month_remainder_days;
      /* fractional months partial days into time */
      day_remainder += month_remainder_days - (int32) month_remainder_days;
Index: src/test/regress/expected/interval.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/interval.out,v
retrieving revision 1.15
diff -c -c -r1.15 interval.out
*** src/test/regress/expected/interval.out    6 Mar 2006 22:49:17 -0000    1.15
--- src/test/regress/expected/interval.out    30 Aug 2006 17:08:16 -0000
***************
*** 218,224 ****
  select avg(f1) from interval_tbl;
                         avg
  -------------------------------------------------
!  @ 4 years 1 mon 9 days 28 hours 18 mins 23 secs
  (1 row)

  -- test long interval input
--- 218,224 ----
  select avg(f1) from interval_tbl;
                         avg
  -------------------------------------------------
!  @ 4 years 1 mon 10 days 4 hours 18 mins 23 secs
  (1 row)

  -- test long interval input

Michael Glaesemann wrote:
>
> On Aug 30, 2006, at 12:50 , Bruce Momjian wrote:
>
> > Yea, I see that -122:23:60.00.
>
> After applying your patch, I believe that on my machine it's the
> contribution from the day component that is producing the 23:60.00.
> For example,
>
> select interval '-12 days' * 0.3;
>         ?column?
> ----------------------
> -3 days -14:23:60.00
> (1 row)
>
> I think some kind of rounding needs to be done to the day
> contribution as well. I'm continuing to work on it, but wanted to get
> this out there.

Please test with my new patch and let me know how it looks.  Thanks.

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce@momjian.us> writes:
> OK, here is a much nicer patch.  The fix is to do no rounding, but to
> find the number of days before applying the factor adjustment.

You have forgotten the problem of the factor not being exactly
representable (eg, things like '10 days' * 0.1 not giving the expected
result).  Also, as coded this is subject to integer-overflow risks
that weren't there before.  That could be fixed, but it's still only
addressing a subset of the problems.  I don't think you can fix them
all without rounding somewhere.

            regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > OK, here is a much nicer patch.  The fix is to do no rounding, but to
> > find the number of days before applying the factor adjustment.
>
> You have forgotten the problem of the factor not being exactly
> representable (eg, things like '10 days' * 0.1 not giving the expected
> result).  Also, as coded this is subject to integer-overflow risks
> that weren't there before.  That could be fixed, but it's still only
> addressing a subset of the problems.  I don't think you can fix them
> all without rounding somewhere.

Well, the patch only multiplies by 30, so the interval would have to
span +5 million years to overflow.  I don't see any reason to add
rounding until we get an actual query that needs it (and because
rounding is arbitrary).  I think the proposed fix is the best we can do
at this time.

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce@momjian.us> writes:
> Well, the patch only multiplies by 30, so the interval would have to
> span +5 million years to overflow.  I don't see any reason to add
> rounding until we get an actual query that needs it

Have you tried your patch against the various cases that have been
discussed in the past?  In particular there were several distinct
examples of this behavior posted at the beginning of the thread, and
I'd not assume that a fix for one handles them all.

BTW, while trolling for examples I came across this:
http://archives.postgresql.org/pgsql-bugs/2005-10/msg00307.php
pointing out some issues that still haven't been addressed.

            regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Well, the patch only multiplies by 30, so the interval would have to
> > span +5 million years to overflow.  I don't see any reason to add
> > rounding until we get an actual query that needs it
>
> Have you tried your patch against the various cases that have been
> discussed in the past?  In particular there were several distinct
> examples of this behavior posted at the beginning of the thread, and
> I'd not assume that a fix for one handles them all.

Yes, it fixes all posted examples, except one that displays 23:60.  I
cannot reproduce that failure from Powerpc so am waiting for Michael to
test it.

> BTW, while trolling for examples I came across this:
> http://archives.postgresql.org/pgsql-bugs/2005-10/msg00307.php
> pointing out some issues that still haven't been addressed.

Yea, that is a bunch of issues.  They are already on the TODO list.

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

On Sep 1, 2006, at 5:05 , Bruce Momjian wrote:

> Tom Lane wrote:
>> Bruce Momjian <bruce@momjian.us> writes:
>>> Well, the patch only multiplies by 30, so the interval would have to
>>> span +5 million years to overflow.  I don't see any reason to add
>>> rounding until we get an actual query that needs it
>>
>> Have you tried your patch against the various cases that have been
>> discussed in the past?  In particular there were several distinct
>> examples of this behavior posted at the beginning of the thread, and
>> I'd not assume that a fix for one handles them all.
>
> Yes, it fixes all posted examples, except one that displays 23:60.  I
> cannot reproduce that failure from Powerpc so am waiting for
> Michael to
> test it.

Here's your patch tested on my machine, both with and without --
enable-integer-datetimes. I've tweaked the ad hoc test suite to
include a case where the days and time differ in sign and added a
couple of queries to the ad hoc test suite to include the problems
Tom referred to--not that this patch will fix them, but to keep the
known problems together. I hope to add more to this to test more edge
cases.

Unfortunately the problem still occur (see product_d), and --enable-
integer-datetimes is pretty broken with this patch.

Michael Glaesemann
grzm seespotcode net


-- test queries
select interval '41 mon 12 days 360:00' * 0.3 as product_a
     , interval '-41 mon -12 days +360:00' * 0.3 as product_b
     , interval '-41 mon 12 days 360:00' * 0.3 as product_c
     , interval '-41 mon -12 days -360:00' * 0.3 as product_d;

select interval '41 mon 12 days 360:00' / 10 as quotient_a
     , interval '-41 mon -12 days +360:00' / 10 as quotient_b
     , interval '-41 mon 12 days 360:00' / 10 as quotient_c
     , interval '-41 mon -12 days -360:00' / 10 as quotient_d;

select interval '-12 days' * 0.3;

select 10000 * '1000000 hours'::interval as "ten billion";

set time zone 'EST5EDT';
select '2005-10-29 13:22:00-04'::timestamptz + '1 day'::interval as
"2005-01-30 13:22:00-05";
select '2005-10-30 13:22:00-05'::timestamptz - '2005-10-29
13:22:00-04'::timestamptz as "a day";
set time zone local;

-- end test queries


-- without --enable-integer-datetimes

select interval '41 mon 12 days 360:00' * 0.3 as product_a
     , interval '-41 mon -12 days +360:00' * 0.3 as product_b
     , interval '-41 mon 12 days 360:00' * 0.3 as product_c
     , interval '-41 mon -12 days -360:00' * 0.3 as product_d;
         product_a         |          product_b          |
product_c          |            product_d
--------------------------+-----------------------------
+----------------------------+---------------------------------
1 year 11 days 146:24:00 | -1 years -11 days +69:36:00 | -1 years -5
days +98:24:00 | -1 years -11 days -146:23:60.00
(1 row)


select interval '41 mon 12 days 360:00' / 10 as quotient_a
     , interval '-41 mon -12 days +360:00' / 10 as quotient_b
     , interval '-41 mon 12 days 360:00' / 10 as quotient_c
     , interval '-41 mon -12 days -360:00' / 10 as quotient_d;
        quotient_a       |        quotient_b         |
quotient_c         |        quotient_d
------------------------+---------------------------
+---------------------------+---------------------------
4 mons 4 days 40:48:00 | -4 mons -4 days +31:12:00 | -4 mons -2 days
+40:48:00 | -4 mons -4 days -40:48:00
(1 row)


select interval '-12 days' * 0.3;
        ?column?
----------------------
-3 days -14:23:60.00
(1 row)


select 10000 * '1000000 hours'::interval as "ten billion";
    ten billion
------------------
2147483647:00:00
(1 row)


set time zone 'EST5EDT';
SET
select '2005-10-29 13:22:00-04'::timestamptz + '1 day'::interval as
"2005-01-30 13:22:00-05";
2005-01-30 13:22:00-05
------------------------
2005-10-30 13:22:00-05
(1 row)

select '2005-10-30 13:22:00-05'::timestamptz - '2005-10-29
13:22:00-04'::timestamptz as "a day";
      a day
----------------
1 day 01:00:00
(1 row)

set time zone local;
SET

-- with --enable-integer-datetimes

select interval '41 mon 12 days 360:00' * 0.3 as product_a
     , interval '-41 mon -12 days +360:00' * 0.3 as product_b
     , interval '-41 mon 12 days 360:00' * 0.3 as product_c
     , interval '-41 mon -12 days -360:00' * 0.3 as product_d;
         product_a         |          product_b          |
product_c          |          product_d
--------------------------+-----------------------------
+----------------------------+------------------------------
1 year 11 days 146:24:00 | -1 years -11 days +69:36:00 | -1 years -5
days +98:24:00 | -1 years -11 days -146:24:00
(1 row)


select interval '41 mon 12 days 360:00' / 10 as quotient_a
     , interval '-41 mon -12 days +360:00' / 10 as quotient_b
     , interval '-41 mon 12 days 360:00' / 10 as quotient_c
     , interval '-41 mon -12 days -360:00' / 10 as quotient_d;
        quotient_a       |        quotient_b         |
quotient_c         |        quotient_d
------------------------+---------------------------
+---------------------------+---------------------------
4 mons 4 days 40:48:00 | -4 mons -4 days +31:12:00 | -4 mons -2 days
+40:48:00 | -4 mons -4 days -40:48:00
(1 row)


select interval '-12 days' * 0.3;
      ?column?
-------------------
-3 days -14:24:00
(1 row)


select 10000 * '1000000 hours'::interval as "ten billion";
    ten billion
------------------
-00:00:00.000001
(1 row)


set time zone 'EST5EDT';
SET
select '2005-10-29 13:22:00-04'::timestamptz + '1 day'::interval as
"2005-01-30 13:22:00-05";
2005-01-30 13:22:00-05
------------------------
2005-10-30 13:22:00-05
(1 row)

select '2005-10-30 13:22:00-05'::timestamptz - '2005-10-29
13:22:00-04'::timestamptz as "a day";
      a day
----------------
1 day 01:00:00
(1 row)

set time zone local;
SET

I am unclear about this report.  The patch was not meant to fix every
interval issue, but merely to improve multiplication and division
computations.  Does it do that?  I think the 23:60 is a time rounding
issue that isn't covered in this patch.  I am not against fixing it, but
does the submitted patch improve things or not?  Given we are
post-feature freeze, we don't have time to fix all the interval issues.

---------------------------------------------------------------------------

Michael Glaesemann wrote:
>
> On Sep 1, 2006, at 5:05 , Bruce Momjian wrote:
>
> > Tom Lane wrote:
> >> Bruce Momjian <bruce@momjian.us> writes:
> >>> Well, the patch only multiplies by 30, so the interval would have to
> >>> span +5 million years to overflow.  I don't see any reason to add
> >>> rounding until we get an actual query that needs it
> >>
> >> Have you tried your patch against the various cases that have been
> >> discussed in the past?  In particular there were several distinct
> >> examples of this behavior posted at the beginning of the thread, and
> >> I'd not assume that a fix for one handles them all.
> >
> > Yes, it fixes all posted examples, except one that displays 23:60.  I
> > cannot reproduce that failure from Powerpc so am waiting for
> > Michael to
> > test it.
>
> Here's your patch tested on my machine, both with and without --
> enable-integer-datetimes. I've tweaked the ad hoc test suite to
> include a case where the days and time differ in sign and added a
> couple of queries to the ad hoc test suite to include the problems
> Tom referred to--not that this patch will fix them, but to keep the
> known problems together. I hope to add more to this to test more edge
> cases.

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

On Sep 1, 2006, at 11:03 , Bruce Momjian wrote:

> I am unclear about this report.  The patch was not meant to fix every
> interval issue, but merely to improve multiplication and division
> computations.  Does it do that?  I think the 23:60 is a time rounding
> issue that isn't covered in this patch.  I am not against fixing
> it, but
> does the submitted patch improve things or not?  Given we are
> post-feature freeze, we don't have time to fix all the interval
> issues.

Your patch doesn't fix the things Tom referenced (nor did you intend
it to). I just wanted to to collect examples of all the known issues
with the interval code in one place. Probably too ambitious for
September 1.

Is it worth looking into the overflow and subtraction issues for 8.2?
It seems to me they're bugs rather than features. Or are these 8.3
since it's so late?

Michael Glaesemann
grzm seespotcode net

Please ignore the patch I just sent. Much too quick with the send
button.

Michael Glaesemann
grzm seespotcode net

Michael Glaesemann <grzm@seespotcode.net> writes:
> Is it worth looking into the overflow and subtraction issues for 8.2?
> It seems to me they're bugs rather than features. Or are these 8.3
> since it's so late?

IMHO they're bugs not new features, and therefore perfectly fair game
to work on during beta.  But for the moment I'd suggest staying focused
on the interval_mul/interval_div roundoff issue.

            regards, tom lane