Monday, September 22, 2008

[HACKERS] Interval literal rounding bug(?) and patch.

*** a/src/backend/utils/adt/datetime.c
--- b/src/backend/utils/adt/datetime.c
***************
*** 2888,2894 **** DecodeInterval(char **field, int *ftype, int nf, int range,
{
case DTK_MICROSEC:
#ifdef HAVE_INT64_TIMESTAMP
! *fsec += val + fval;
#else
*fsec += (val + fval) * 1e-6;
#endif
--- 2888,2894 ----
{
case DTK_MICROSEC:
#ifdef HAVE_INT64_TIMESTAMP
! *fsec += rint(val + fval);
#else
*fsec += (val + fval) * 1e-6;
#endif
***************
*** 2897,2903 **** DecodeInterval(char **field, int *ftype, int nf, int range,

case DTK_MILLISEC:
#ifdef HAVE_INT64_TIMESTAMP
! *fsec += (val + fval) * 1000;
#else
*fsec += (val + fval) * 1e-3;
#endif
--- 2897,2903 ----

case DTK_MILLISEC:
#ifdef HAVE_INT64_TIMESTAMP
! *fsec += rint((val + fval) * 1000);
#else
*fsec += (val + fval) * 1e-3;
#endif
***************
*** 2907,2913 **** DecodeInterval(char **field, int *ftype, int nf, int range,
case DTK_SECOND:
tm->tm_sec += val;
#ifdef HAVE_INT64_TIMESTAMP
! *fsec += fval * 1000000;
#else
*fsec += fval;
#endif
--- 2907,2913 ----
case DTK_SECOND:
tm->tm_sec += val;
#ifdef HAVE_INT64_TIMESTAMP
! *fsec += rint(fval * 1000000);
#else
*fsec += fval;
#endif
***************
*** 2932,2938 **** DecodeInterval(char **field, int *ftype, int nf, int range,
sec = fval;
tm->tm_sec += sec;
#ifdef HAVE_INT64_TIMESTAMP
! *fsec += (fval - sec) * 1000000;
#else
*fsec += fval - sec;
#endif
--- 2932,2938 ----
sec = fval;
tm->tm_sec += sec;
#ifdef HAVE_INT64_TIMESTAMP
! *fsec += rint((fval - sec) * 1000000);
#else
*fsec += fval - sec;
#endif
***************
*** 2950,2956 **** DecodeInterval(char **field, int *ftype, int nf, int range,
sec = fval;
tm->tm_sec += sec;
#ifdef HAVE_INT64_TIMESTAMP
! *fsec += (fval - sec) * 1000000;
#else
*fsec += fval - sec;
#endif
--- 2950,2956 ----
sec = fval;
tm->tm_sec += sec;
#ifdef HAVE_INT64_TIMESTAMP
! *fsec += rint((fval - sec) * 1000000);
#else
*fsec += fval - sec;
#endif
***************
*** 2969,2975 **** DecodeInterval(char **field, int *ftype, int nf, int range,
sec = fval;
tm->tm_sec += sec;
#ifdef HAVE_INT64_TIMESTAMP
! *fsec += (fval - sec) * 1000000;
#else
*fsec += fval - sec;
#endif
--- 2969,2975 ----
sec = fval;
tm->tm_sec += sec;
#ifdef HAVE_INT64_TIMESTAMP
! *fsec += rint((fval - sec) * 1000000);
#else
*fsec += fval - sec;
#endif
***************
*** 2995,3001 **** DecodeInterval(char **field, int *ftype, int nf, int range,
sec = fval;
tm->tm_sec += sec;
#ifdef HAVE_INT64_TIMESTAMP
! *fsec += (fval - sec) * 1000000;
#else
*fsec += fval - sec;
#endif
--- 2995,3001 ----
sec = fval;
tm->tm_sec += sec;
#ifdef HAVE_INT64_TIMESTAMP
! *fsec += rint((fval - sec) * 1000000);
#else
*fsec += fval - sec;
#endif
***************
*** 3022,3028 **** DecodeInterval(char **field, int *ftype, int nf, int range,
sec = fval;
tm->tm_sec += sec;
#ifdef HAVE_INT64_TIMESTAMP
! *fsec += (fval - sec) * 1000000;
#else
*fsec += fval - sec;
#endif
--- 3022,3028 ----
sec = fval;
tm->tm_sec += sec;
#ifdef HAVE_INT64_TIMESTAMP
! *fsec += rint((fval - sec) * 1000000);
#else
*fsec += fval - sec;
#endif
I think it's a bug that these 3 different ways of writing 0.7 seconds
produce different results from each other on HEAD.

head=# select interval '0:0:0.7', interval '@ 0.70 secs', interval '0.7 seconds';
interval | interval | interval
-------------+-----------------+-----------------
00:00:00.70 | 00:00:00.699999 | 00:00:00.699999
(1 row)

The attached patch will make all of those output "00:00:00.70" which.

Postgres 8.3 tended to output the "00:00:00.70" like this patch, I believe
because it didn't default to HAVE_INT64_TIMESTAMP like HEAD is. The patch
seems to pass the existing regression tests.

Does this seem reasonable?

Ron

No comments: