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

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;