Re: [HACKERS] Interval aggregate regression failure

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;