Re: [PATCHES] Interval aggregate regression failure - Mailing list pgsql-hackers

From Michael Glaesemann
Subject Re: [PATCHES] Interval aggregate regression failure
Date
Msg-id 8A607FA9-382F-4A5B-BEE5-E9B30FBF3233@seespotcode.net
Whole thread Raw
In response to Re: [PATCHES] Interval aggregate regression failure  (Michael Glaesemann <grzm@seespotcode.net>)
List pgsql-hackers
On Sep 1, 2006, at 20:39 , Michael Glaesemann wrote:

> Here's a patch that appears to work. Gives the same output with and
> without --enable-integer-datetimes. Answers look like they're correct.
>
> I'm basically treating the components as three different intervals
> (with the other two components zero), rounding them each to usecs,
> and adding them together.
>
> While it might be nice to carry a little extra precision around, it
> doesn't seem to be needed in these cases. If errors do arise, they
> should be at most 3 usec, which is pretty much noise for the
> floating point case, I suspect.

Okay. Here's the patch. Bruce, does it work for you?

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    1 Sep 2006 12:28:23 -0000
***************
*** 2494,2504 ****
       float8        factor = PG_GETARG_FLOAT8(1);
       double        month_remainder,
                   day_remainder,
!                 month_remainder_days;
       Interval   *result;

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

       month_remainder = span->month * factor;
       day_remainder = span->day * factor;
       result->month = (int32) month_remainder;
--- 2494,2507 ----
       float8        factor = PG_GETARG_FLOAT8(1);
       double        month_remainder,
                   day_remainder,
!                 month_remainder_days,
!                 month_remainder_time,
!                 day_remainder_time;
       Interval   *result;

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

+
       month_remainder = span->month * factor;
       day_remainder = span->day * factor;
       result->month = (int32) month_remainder;
***************
*** 2506,2511 ****
--- 2509,2553 ----
       month_remainder -= result->month;
       day_remainder -= result->day;

+     month_remainder_days = month_remainder * DAYS_PER_MONTH;
+
+     /*
+         if month_remainder_days is not an integer, check to see if it's an
+         integer when converted to SECS or USECS.
+         If it is, round month_remainder_days to the nearest integer
+
+      */
+
+     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;
+
+ #ifdef HAVE_INT64_TIMESTAMP
+     month_remainder_time = rint((month_remainder_days -
+                             (int32)month_remainder_days) * USECS_PER_DAY);
+
+     day_remainder_time = rint(day_remainder * USECS_PER_DAY);
+
+
+     result->time = rint(rint(span->time * factor) + day_remainder_time +
+                        month_remainder_time);
+ #else
+     month_remainder_time = rint((month_remainder_days -
+                            (int32)month_remainder_days) * SECS_PER_DAY);
+     day_remainder_time = rint(day_remainder * SECS_PER_DAY);
+
+     result->time = span->time * factor + day_remainder_time +
+                     month_remainder_time;
+ #endif
+
+
+     day_remainder = span->day * factor;
+     result->day = (int32) day_remainder;
+     day_remainder -= result->day;
+
       /*
        * The above correctly handles the whole-number part of the month
and day
        * products, but we have to do something with any fractional part
***************
*** 2518,2531 ****

       /* 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);
--- 2560,2599 ----

       /* 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.
!      */

   #ifdef HAVE_INT64_TIMESTAMP
!     if (month_remainder_days != (int32)month_remainder_days &&
!         rint(month_remainder_days * USECS_PER_DAY) ==
!           (int64)(month_remainder_days * USECS_PER_DAY))
!         month_remainder_days = rint(month_remainder_days);
!     result->day += (int32) month_remainder_days;
!     month_remainder_time = rint((month_remainder_days -
!                             (int32)month_remainder_days) * USECS_PER_DAY);
!
!     day_remainder_time = rint(day_remainder * USECS_PER_DAY);
!
!     result->time = rint(span->time * factor + day_remainder_time +
!                        month_remainder_time);
   #else
!     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;
!
!     month_remainder_time = rint((month_remainder_days -
!                            (int32)month_remainder_days) * SECS_PER_DAY);
!
!     day_remainder_time = rint(day_remainder * SECS_PER_DAY);
!
!     result->time = span->time * factor + day_remainder_time +
!                     month_remainder_time;
!
   #endif

       PG_RETURN_INTERVAL_P(result);
***************
*** 2548,2554 ****
       float8        factor = PG_GETARG_FLOAT8(1);
       double        month_remainder,
                   day_remainder,
!                 month_remainder_days;
       Interval   *result;

       result = (Interval *) palloc(sizeof(Interval));
--- 2616,2624 ----
       float8        factor = PG_GETARG_FLOAT8(1);
       double        month_remainder,
                   day_remainder,
!                 month_remainder_days,
!                 month_remainder_time,
!                 day_remainder_time;
       Interval   *result;

       result = (Interval *) palloc(sizeof(Interval));
***************
*** 2571,2584 ****

       /* 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);
--- 2641,2680 ----

       /* 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;

!     day_remainder_time = day_remainder * SECS_PER_DAY;
!     if (day_remainder_time != (int32)day_remainder_time &&
!         TSROUND(day_remainder_time) ==
!         rint(day_remainder_time))
!         day_remainder_time = rint(day_remainder_time);
!
! #ifdef HAVE_INT64_TIMESTAMP
!     day_remainder_time = day_remainder * USECS_PER_DAY;
!     if (day_remainder_time != (int32)day_remainder_time &&
!         TSROUND(day_remainder_time) == rint(day_remainder_time))
!         day_remainder_time = rint(day_remainder_time);
!
!     result->time = rint(span->time / factor + day_remainder_time +
!        (month_remainder_days - (int32)month_remainder_days) *
USECS_PER_DAY;
! #else
!     day_remainder_time = day_remainder * SECS_PER_DAY;
!     if (day_remainder_time != (int32)day_remainder_time &&
!         TSROUND(day_remainder_time) == rint(day_remainder_time))
!         day_remainder_time = rint(day_remainder_time);
!
!     result->time = span->time / factor + day_remainder_time +
!        (month_remainder_days - (int32)month_remainder_days) *
SECS_PER_DAY;
   #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    1 Sep 2006 12:28:24 -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))


pgsql-hackers by date:

Previous
From: Bernd Helmle
Date:
Subject: Re: [PATCHES] Updatable views
Next
From: "Jeroen T. Vermeulen"
Date:
Subject: Re: Prepared statements considered harmful