Re: [HACKERS] Interval aggregate regression failure (expected - Mailing list pgsql-patches
From | Bruce Momjian |
---|---|
Subject | Re: [HACKERS] Interval aggregate regression failure (expected |
Date | |
Msg-id | 200608260240.k7Q2eL124053@momjian.us Whole thread Raw |
Responses |
Re: [HACKERS] Interval aggregate regression failure (expected seems
(Michael Glaesemann <grzm@seespotcode.net>)
|
List | pgsql-patches |
I used your ideas to make a patch to fix your example: test=> select '41 months'::interval / 10; ?column? --------------- 4 mons 3 days (1 row) and test=> select '41 months'::interval * 0.3; ?column? --------------- 1 year 9 days (1 row) The trick was not to play with the division, but to check if the number of seconds cascaded into full days and/or months. --------------------------------------------------------------------------- Tom Lane wrote: > Michael Glaesemann <grzm@seespotcode.net> writes: > > ... I think this just confirms that there is some kind of rounding (or > > lack of) in interval_div. Kind of frustrating that it's not visible > > in the result. > > I think the fundamental problem is that the float8 results of division > are inaccurate, and yet we're assuming that we can (for instance) coerce > them to integer and get exactly the right answer. For instance, in the > '41 months'/10 example, I get month_remainder_days being computed as > > (gdb) p month_remainder > $19 = 0.099999999999999645 > (gdb) s > 2575 result->day += (int32) month_remainder_days; > (gdb) p month_remainder_days > $20 = 2.9999999999999893 > > The only way we can really fix this is to be willing to round off > the numbers, and I think the only principled way to do that is to > settle on a specific target accuracy, probably 1 microsecond. > Then the thing to do would be to scale up all the intermediate > float results to microseconds and apply rint(). Something like > (untested) > > month_remainder = rint(span->month * USECS_PER_MONTH / factor); > day_remainder = rint(span->day * USECS_PER_DAY / factor); > result->month = (int32) (month_remainder / USECS_PER_MONTH); > result->day = (int32) (day_remainder / USECS_PER_DAY); > month_remainder -= result->month * USECS_PER_MONTH; > day_remainder -= result->day * USECS_PER_DAY; > > /* > * Handle any fractional parts the same way as in interval_mul. > */ > > /* fractional months full days into days */ > month_remainder_days = month_remainder * DAYS_PER_MONTH; > extra_days = (int32) (month_remainder_days / USECS_PER_DAY); > result->day += extra_days; > /* fractional months partial days into time */ > day_remainder += month_remainder_days - extra_days * USECS_PER_DAY; > > #ifdef HAVE_INT64_TIMESTAMP > result->time = rint(span->time / factor + day_remainder); > #else > result->time = rint(span->time * 1.0e6 / factor + day_remainder) / 1.0e6; > #endif > > This might need a few more rint() calls --- I'm assuming that float ops > with exact integral inputs will be OK, which is an assumption used > pretty widely in the datetime code, but ... > > Per the comment, if we do this here we probably want to make > interval_mul work similarly. > > regards, tom lane -- 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 26 Aug 2006 02:36:28 -0000 *************** *** 2526,2531 **** --- 2526,2546 ---- result->time = rint(span->time * factor + day_remainder * USECS_PER_DAY); #else result->time = span->time * factor + day_remainder * SECS_PER_DAY; + /* + * The imprecision of float8 causes unusual rounding of even integer + * division, so round up if we have full units. Check seconds only + * as far as microseconds. + */ + if (TSROUND(result->time) == SECS_PER_DAY) + { + result->day++; + result->time = 0; + } + if (result->day == DAYS_PER_MONTH) + { + result->month++; + result->day = 0; + } #endif PG_RETURN_INTERVAL_P(result); *************** *** 2579,2584 **** --- 2594,2614 ---- result->time = rint(span->time / factor + day_remainder * USECS_PER_DAY); #else result->time = span->time / factor + day_remainder * SECS_PER_DAY; + /* + * The imprecision of float8 causes unusual rounding of even integer + * division, so round up if we have full units. Check seconds only + * as far as microseconds. + */ + if (TSROUND(result->time) == SECS_PER_DAY) + { + result->day++; + result->time = 0; + } + if (result->day == DAYS_PER_MONTH) + { + result->month++; + result->day = 0; + } #endif PG_RETURN_INTERVAL_P(result);
pgsql-patches by date: