Re: Re: Updated interval patches (SQL std output, ISO8601 intervals, and interval rounding) - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Re: Updated interval patches (SQL std output, ISO8601 intervals, and interval rounding)
Date
Msg-id 21192.1226454047@sss.pgh.pa.us
Whole thread Raw
In response to Re: Re: Updated interval patches (SQL std output, ISO8601 intervals, and interval rounding)  (Ron Mayer <rm_pg@cheapcomplexdevices.com>)
Responses Re: Re: Updated interval patches (SQL std output, ISO8601 intervals, and interval rounding)  (Ron Mayer <rm_pg@cheapcomplexdevices.com>)
List pgsql-hackers
Ron Mayer <rm_pg@cheapcomplexdevices.com> writes:
> Brendan Jurd wrote:
>> I don't have any further gripes regarding this patch, apart from the
>> code style stuff I sent through in my previous post.  Did you have any
>> response to those?

> Yup - you were right again.
> Applied them and updated the website and attaching the patch.

Applied with another round of mostly-stylistic revisions, plus a little
extra work to factor out some more code duplication (around strtod
calls, which were insufficiently error-checked too).

There was one part I left out because it worried me:

***************
*** 2980,3010 ****                 switch (type)                 {                     case DTK_MICROSEC:
! #ifdef HAVE_INT64_TIMESTAMP
!                         *fsec += rint(val + fval);
! #else
!                         *fsec += (val + fval) * 1e-6;
! #endif                         tmask = DTK_M(MICROSECOND);                         break;                      case
DTK_MILLISEC:
! #ifdef HAVE_INT64_TIMESTAMP
!                         *fsec += rint((val + fval) * 1000);
! #else
!                         *fsec += (val + fval) * 1e-3;
! #endif                         tmask = DTK_M(MILLISECOND);                         break;                      case
DTK_SECOND:                        tm->tm_sec += val;
 
! #ifdef HAVE_INT64_TIMESTAMP
!                         *fsec += rint(fval * 1000000);
! #else
!                         *fsec += fval;
! #endif
!                          /*                          * If any subseconds were specified, consider this
         * microsecond and millisecond input as well.
 
--- 2897,2914 ----                 switch (type)                 {                     case DTK_MICROSEC:
!                         AdjustFractSeconds((val + fval) * 1e-6, tm, fsec, 1);                         tmask =
DTK_M(MICROSECOND);                        break;                      case DTK_MILLISEC:
 
!                         AdjustFractSeconds((val + fval) * 1e-3, tm, fsec, 1);                         tmask =
DTK_M(MILLISECOND);                        break;                      case DTK_SECOND:
tm->tm_sec+= val;
 
!                         AdjustFractSeconds(fval, tm, fsec, 1);                         /*                          *
Ifany subseconds were specified, consider this                          * microsecond and millisecond input as well.
 

The original INT64 coding here is exact (at least for the common case
where fval is zero) but I'm not convinced that your revision can't
suffer from roundoff error.
        regards, tom lane


pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: [pgsql-www] Message-ID should surely not be shown as a mailto: URL
Next
From: Unicron
Date:
Subject: Result for "Automatically update view"