Re: [BUGS] BUG #1927: incorrect timestamp returned - Mailing list pgsql-patches

From Bruce Momjian
Subject Re: [BUGS] BUG #1927: incorrect timestamp returned
Date
Msg-id 200510072118.j97LIkf12835@candle.pha.pa.us
Whole thread Raw
Responses Re: [BUGS] BUG #1927: incorrect timestamp returned
List pgsql-patches
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Right.  We allow leap seconds for any date/time.  Are you saying we
> > should only allow them for certain dates/times?
>
> No, his point is the funny roundoff behavior.
>
> regression=# select timestamp '2005-09-23 23:59:59.999999';
>          timestamp
> ----------------------------
>  2005-09-23 23:59:59.999999
> (1 row)
>
> regression=# select timestamp '2005-09-23 23:59:59.9999999';
>        timestamp
> ------------------------
>  2005-09-23 23:59:60.00
> (1 row)
>
> regression=# select timestamp '2005-09-23 23:59:59.99999999';
>       timestamp
> ---------------------
>  2005-09-24 00:00:00
> (1 row)

I did some research on this.  The difference is caused by the place in
the code where the rounding happens.  Here is the simple case.  The
second line is the return value, "double", from timestamp_in():

    test=> select timestamp '2005-09-23 23:59:59.999999';
             timestamp
    ----------------------------
     2005-09-23 23:59:59.999999
     180835199.99999899

Here is one where the rounding happens after timestamp_in() returns:

    test=> select timestamp '2005-09-23 23:59:59.9999999';
           timestamp
    ------------------------
     2005-09-23 23:59:60.00
     180835199.99999991

and in this case the rounding happens inside timestamp_in():

    test=> select timestamp '2005-09-23 23:59:59.99999999';
          timestamp
    ---------------------
     2005-09-24 00:00:00
     180835200

Looks like "time" has a similar problem:

    test=> select time '2005-09-23 23:59:59.99999999';
           time
    -------------------
     23:59:59.99999999
    (1 row)

    test=> select time '2005-09-23 23:59:59.99999999999';
        time
    -------------
     23:59:60.00
    (1 row)

    test=> select time '2005-09-23 23:59:59.999999999999';
       time
    ----------
     24:00:00
    (1 row)

I have gone through the code and identified all the places that need
JROUND, basically places where we do complex calculations that include
fsec (fractional seconds).  This only affects timestamp=double backends,
not timestamp=int64.

The patch fixes all the test cases above, and passes all regression
tests.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: src/backend/utils/adt/date.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/date.c,v
retrieving revision 1.120
diff -c -c -r1.120 date.c
*** src/backend/utils/adt/date.c    9 Sep 2005 02:31:49 -0000    1.120
--- src/backend/utils/adt/date.c    7 Oct 2005 21:00:34 -0000
***************
*** 920,926 ****
      *result = ((((tm->tm_hour * MINS_PER_HOUR + tm->tm_min) * SECS_PER_MINUTE) + tm->tm_sec)
                  * USECS_PER_SEC) + fsec;
  #else
!     *result = ((tm->tm_hour * MINS_PER_HOUR + tm->tm_min) * SECS_PER_MINUTE) + tm->tm_sec + fsec;
  #endif
      return 0;
  }
--- 920,926 ----
      *result = ((((tm->tm_hour * MINS_PER_HOUR + tm->tm_min) * SECS_PER_MINUTE) + tm->tm_sec)
                  * USECS_PER_SEC) + fsec;
  #else
!     *result = JROUND(((tm->tm_hour * MINS_PER_HOUR + tm->tm_min) * SECS_PER_MINUTE) + tm->tm_sec + fsec);
  #endif
      return 0;
  }
***************
*** 1345,1351 ****
      result = ((((tm->tm_hour * MINS_PER_HOUR + tm->tm_min) * SECS_PER_MINUTE) + tm->tm_sec) *
                  USECS_PER_SEC) + fsec;
  #else
!     result = ((tm->tm_hour * MINS_PER_HOUR + tm->tm_min) * SECS_PER_MINUTE) + tm->tm_sec + fsec;
  #endif

      PG_RETURN_TIMEADT(result);
--- 1345,1351 ----
      result = ((((tm->tm_hour * MINS_PER_HOUR + tm->tm_min) * SECS_PER_MINUTE) + tm->tm_sec) *
                  USECS_PER_SEC) + fsec;
  #else
!     result = JROUND(((tm->tm_hour * MINS_PER_HOUR + tm->tm_min) * SECS_PER_MINUTE) + tm->tm_sec + fsec);
  #endif

      PG_RETURN_TIMEADT(result);
***************
*** 1382,1388 ****
      result = ((((tm->tm_hour * MINS_PER_HOUR + tm->tm_min) * SECS_PER_MINUTE) + tm->tm_sec) *
                  USECS_PER_SEC) + fsec;
  #else
!     result = ((tm->tm_hour * MINS_PER_HOUR + tm->tm_min) * SECS_PER_MINUTE) + tm->tm_sec + fsec;
  #endif

      PG_RETURN_TIMEADT(result);
--- 1382,1388 ----
      result = ((((tm->tm_hour * MINS_PER_HOUR + tm->tm_min) * SECS_PER_MINUTE) + tm->tm_sec) *
                  USECS_PER_SEC) + fsec;
  #else
!     result = JROUND(((tm->tm_hour * MINS_PER_HOUR + tm->tm_min) * SECS_PER_MINUTE) + tm->tm_sec + fsec);
  #endif

      PG_RETURN_TIMEADT(result);
***************
*** 1712,1718 ****
      result->time = ((((tm->tm_hour * MINS_PER_HOUR + tm->tm_min) * SECS_PER_MINUTE) + tm->tm_sec) *
                      USECS_PER_SEC) + fsec;
  #else
!     result->time = ((tm->tm_hour * MINS_PER_HOUR + tm->tm_min) * SECS_PER_MINUTE) + tm->tm_sec + fsec;
  #endif
      result->zone = tz;

--- 1712,1718 ----
      result->time = ((((tm->tm_hour * MINS_PER_HOUR + tm->tm_min) * SECS_PER_MINUTE) + tm->tm_sec) *
                      USECS_PER_SEC) + fsec;
  #else
!     result->time = JROUND(((tm->tm_hour * MINS_PER_HOUR + tm->tm_min) * SECS_PER_MINUTE) + tm->tm_sec + fsec);
  #endif
      result->zone = tz;

Index: src/backend/utils/adt/timestamp.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
retrieving revision 1.153
diff -c -c -r1.153 timestamp.c
*** src/backend/utils/adt/timestamp.c    9 Sep 2005 06:46:14 -0000    1.153
--- src/backend/utils/adt/timestamp.c    7 Oct 2005 21:00:38 -0000
***************
*** 1255,1261 ****
  static double
  time2t(const int hour, const int min, const int sec, const fsec_t fsec)
  {
!     return (((hour * MINS_PER_HOUR) + min) * SECS_PER_MINUTE) + sec + fsec;
  }    /* time2t() */
  #endif

--- 1255,1261 ----
  static double
  time2t(const int hour, const int min, const int sec, const fsec_t fsec)
  {
!     return JROUND((((hour * MINS_PER_HOUR) + min) * SECS_PER_MINUTE) + sec + fsec);
  }    /* time2t() */
  #endif

***************
*** 3505,3511 ****
                  result += ((((tm->tm_hour * MINS_PER_HOUR) + tm->tm_min) * SECS_PER_MINUTE) +
                              tm->tm_sec + (fsec / 1000000.0)) / (double)SECS_PER_DAY;
  #else
!                 result += ((((tm->tm_hour * MINS_PER_HOUR) + tm->tm_min) * SECS_PER_MINUTE) +
                              tm->tm_sec + fsec) / (double)SECS_PER_DAY;
  #endif
                  break;
--- 3505,3511 ----
                  result += ((((tm->tm_hour * MINS_PER_HOUR) + tm->tm_min) * SECS_PER_MINUTE) +
                              tm->tm_sec + (fsec / 1000000.0)) / (double)SECS_PER_DAY;
  #else
!                 result += JROUND((((tm->tm_hour * MINS_PER_HOUR) + tm->tm_min) * SECS_PER_MINUTE) +
                              tm->tm_sec + fsec) / (double)SECS_PER_DAY;
  #endif
                  break;
***************
*** 3733,3739 ****
                  result += ((((tm->tm_hour * MINS_PER_HOUR) + tm->tm_min) * SECS_PER_MINUTE) +
                              tm->tm_sec + (fsec / 1000000.0)) / (double)SECS_PER_DAY;
  #else
!                 result += ((((tm->tm_hour * MINS_PER_HOUR) + tm->tm_min) * SECS_PER_MINUTE) +
                              tm->tm_sec + fsec) / (double)SECS_PER_DAY;
  #endif
                  break;
--- 3733,3739 ----
                  result += ((((tm->tm_hour * MINS_PER_HOUR) + tm->tm_min) * SECS_PER_MINUTE) +
                              tm->tm_sec + (fsec / 1000000.0)) / (double)SECS_PER_DAY;
  #else
!                 result += JROUND((((tm->tm_hour * MINS_PER_HOUR) + tm->tm_min) * SECS_PER_MINUTE) +
                              tm->tm_sec + fsec) / (double)SECS_PER_DAY;
  #endif
                  break;
Index: src/interfaces/ecpg/pgtypeslib/timestamp.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/ecpg/pgtypeslib/timestamp.c,v
retrieving revision 1.31
diff -c -c -r1.31 timestamp.c
*** src/interfaces/ecpg/pgtypeslib/timestamp.c    22 Jul 2005 19:00:55 -0000    1.31
--- src/interfaces/ecpg/pgtypeslib/timestamp.c    7 Oct 2005 21:00:40 -0000
***************
*** 27,33 ****
  static double
  time2t(const int hour, const int min, const int sec, const fsec_t fsec)
  {
!     return (((hour * MINS_PER_HOUR) + min) * SECS_PER_MINUTE) + sec + fsec;
  }    /* time2t() */
  #endif

--- 27,33 ----
  static double
  time2t(const int hour, const int min, const int sec, const fsec_t fsec)
  {
!     return     JROUND((((hour * MINS_PER_HOUR) + min) * SECS_PER_MINUTE) + sec + fsec);
  }    /* time2t() */
  #endif


pgsql-patches by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: [HACKERS] Patching dblink.c to avoid warning about open
Next
From: Tom Lane
Date:
Subject: Re: [BUGS] BUG #1927: incorrect timestamp returned