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

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;