Re: [HACKERS] Interval aggregate regression failure - Mailing list pgsql-patches

Bruce Momjian wrote:
> OK, updated patch.  It will fix the >=24:00:00 case because it cascades
> up if the remainder number of seconds is greater or equal to one day.
> One open item is that it still might show >24 hours if the seconds
> computation combined with the remaning seconds >24 hours.  Not sure if
> that should be handled or not.  If you fix that, you really are
> cascading up because the resulting seconds might be less than the
> computed value, e.g. result is 23:00:00, remainder is 02:00:00, cascade
> up would be 1 day, 01:00:00.  I am unsure we want to do that.  Right
> now, this will show 25:00:00.

Updated patch that uses TSROUND for partial month and days.  Michael
says the tests look good on his system.  I think we are done with this
bug.  :-)

--
  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.166
diff -c -c -r1.166 timestamp.c
*** src/backend/utils/adt/timestamp.c    3 Sep 2006 03:34:04 -0000    1.166
--- src/backend/utils/adt/timestamp.c    4 Sep 2006 17:54:25 -0000
***************
*** 2514,2541 ****
      /*
       *    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 * (double)DAYS_PER_MONTH) * factor -
!             result->month * (double)DAYS_PER_MONTH;
!     sec_remainder = (orig_day * (double)SECS_PER_DAY) * factor -
!             result->day * (double)SECS_PER_DAY +
!             (month_remainder_days - (int32) month_remainder_days) * SECS_PER_DAY;

      /* cascade units down */
      result->day += (int32) month_remainder_days;
  #ifdef HAVE_INT64_TIMESTAMP
      result->time = rint(span->time * factor + sec_remainder * USECS_PER_SEC);
  #else
!     /*
!      *    TSROUND() needed to prevent -146:23:60.00 output on PowerPC for
!      *    SELECT interval '-41 mon -12 days -360:00' * 0.3;
!      */
!     result->time = span->time * factor + TSROUND(sec_remainder);
  #endif

      PG_RETURN_INTERVAL_P(result);
--- 2514,2547 ----
      /*
       *    Fractional months full days into days.
       *
!      *    Floating point calculation are inherently inprecise, so these
!      *    calculations are crafted to produce the most reliable result
!      *    possible.  TSROUND() is needed to more accurately produce whole
!      *    numbers where appropriate.
       */
!     month_remainder_days = (orig_month * factor - result->month) * DAYS_PER_MONTH;
!     month_remainder_days = TSROUND(month_remainder_days);
!     sec_remainder = (orig_day * factor - result->day +
!             month_remainder_days - (int)month_remainder_days) * SECS_PER_DAY;
!     sec_remainder = TSROUND(sec_remainder);
!
!     /*
!      *    Might have 24:00:00 hours due to rounding, or >24 hours because of
!      *    time cascade from months and days.  It might still be >24 if the
!      *    combination of cascade and the seconds factor operation itself.
!      */
!     if (Abs(sec_remainder) >= SECS_PER_DAY)
!     {
!         result->day += (int)(sec_remainder / SECS_PER_DAY);
!         sec_remainder -= (int)(sec_remainder / SECS_PER_DAY) * SECS_PER_DAY;
!     }

      /* cascade units down */
      result->day += (int32) month_remainder_days;
  #ifdef HAVE_INT64_TIMESTAMP
      result->time = rint(span->time * factor + sec_remainder * USECS_PER_SEC);
  #else
!     result->time = span->time * factor + sec_remainder;
  #endif

      PG_RETURN_INTERVAL_P(result);
***************
*** 2574,2584 ****
       *    Fractional months full days into days.  See comment in
       *    interval_mul().
       */
!     month_remainder_days = (orig_month * (double)DAYS_PER_MONTH) / factor -
!             result->month * (double)DAYS_PER_MONTH;
!     sec_remainder = (orig_day * (double)SECS_PER_DAY) / factor -
!             result->day * (double)SECS_PER_DAY +
!             (month_remainder_days - (int32) month_remainder_days) * SECS_PER_DAY;

      /* cascade units down */
      result->day += (int32) month_remainder_days;
--- 2580,2595 ----
       *    Fractional months full days into days.  See comment in
       *    interval_mul().
       */
!     month_remainder_days = (orig_month / factor - result->month) * DAYS_PER_MONTH;
!     month_remainder_days = TSROUND(month_remainder_days);
!     sec_remainder = (orig_day / factor - result->day +
!             month_remainder_days - (int)month_remainder_days) * SECS_PER_DAY;
!     sec_remainder = TSROUND(sec_remainder);
!     if (Abs(sec_remainder) >= SECS_PER_DAY)
!     {
!         result->day += (int)(sec_remainder / SECS_PER_DAY);
!         sec_remainder -= (int)(sec_remainder / SECS_PER_DAY) * SECS_PER_DAY;
!     }

      /* cascade units down */
      result->day += (int32) month_remainder_days;
***************
*** 2586,2592 ****
      result->time = rint(span->time / factor + sec_remainder * USECS_PER_SEC);
  #else
      /* See TSROUND comment in interval_mul(). */
!     result->time = span->time / factor + TSROUND(sec_remainder);
  #endif

      PG_RETURN_INTERVAL_P(result);
--- 2597,2603 ----
      result->time = rint(span->time / factor + sec_remainder * USECS_PER_SEC);
  #else
      /* See TSROUND comment in interval_mul(). */
!     result->time = span->time / factor + sec_remainder;
  #endif

      PG_RETURN_INTERVAL_P(result);
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    4 Sep 2006 17:54:26 -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,174 ----

  typedef double fsec_t;

! #endif
!
! /*
!  *    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)

  #define TIMESTAMP_MASK(b) (1 << (b))
  #define INTERVAL_MASK(b) (1 << (b))