Michael Glaesemann wrote:
> 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)
Strange, I do not see that here. Is there something wrong with our
hour/minute display? Someone posted a patch a few days ago for that.
Here is a test program. What does it show for you?
#include <stdio.h>
int
main(int argc, char *argv[])
{
double x;
x = 41;
x = x / 10.0;
printf("%f\n", x);
x = x - (int)x;
x = x * 30;
printf("%15.15f\n", x);
x = 0.1 * 30;
printf("%15.15f\n", x);
return 0;
}
The output for me is:
4.100000000000000
2.999999999999989
3.000000000000000
>
> 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)
Powerpc. Hmmm. I am on Intel.
> 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)
>
Yea, I see that -122:23:60.00.
> > 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);
>
Good idea. I updated the patch to do that.
> 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?
Yea, just an optimization, but I was worried that the computations might
throw problems for certain numbers, so I figured I would only trigger it
when necessary.
> 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.
You don't want to do any conditional tests. SECS_PER_DAY is all you
want. Those two macros are useful only in representing the internal
storage format, not for float compuations of rounding.
Patch attached. It also fixes a regression test output too.
--
Bruce Momjian [EMAIL PROTECTED]
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 30 Aug 2006 03:45:41 -0000
***************
*** 2518,2523 ****
--- 2518,2532 ----
/* 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.
+ */
+ 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;
/* fractional months partial days into time */
day_remainder += month_remainder_days - (int32) month_remainder_days;
***************
*** 2571,2576 ****
--- 2580,2595 ----
/* 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;
/* fractional months partial days into time */
day_remainder += month_remainder_days - (int32) month_remainder_days;
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 30 Aug 2006 03:45:42 -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))
Index: src/test/regress/expected/interval.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/interval.out,v
retrieving revision 1.15
diff -c -c -r1.15 interval.out
*** src/test/regress/expected/interval.out 6 Mar 2006 22:49:17 -0000 1.15
--- src/test/regress/expected/interval.out 30 Aug 2006 03:45:43 -0000
***************
*** 218,224 ****
select avg(f1) from interval_tbl;
avg
-------------------------------------------------
! @ 4 years 1 mon 9 days 28 hours 18 mins 23 secs
(1 row)
-- test long interval input
--- 218,224 ----
select avg(f1) from interval_tbl;
avg
-------------------------------------------------
! @ 4 years 1 mon 10 days 4 hours 18 mins 23 secs
(1 row)
-- test long interval input
---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster
