Thread: Fix for timestamp rouding

Michael Fuhr wrote:
> I'm getting time, timetz, and horology regression failures in HEAD
> on Solaris 9 / gcc 3.4.2.  So are other machines in the build farm,
> such as this one:
>
> http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=shark&dt=2005-05-26%2004:21:00
>
> I'm getting the same regression failures shown in that link; here's
> an example:

OK, I have a new patch, which simplifies the code by using
TrimTrailingZeros(), gives more consistent subsecond display, and
subpresses the rounding problem:


    test=> select '2005 years 4 mons 20 days 15 hours 57 mins 12.1 secs ago'::interval;
                     interval
    -------------------------------------------
     -2005 years -4 mons -20 days -15:57:12.10
    (1 row)

    test=> select '2005 years 4 mons 20 days 15 hours 57 mins 12.13 secs ago'::interval;
                     interval
    -------------------------------------------
     -2005 years -4 mons -20 days -15:57:12.13
    (1 row)

    test=> select '2005 years 4 mons 20 days 15 hours 57 mins 12.134 secs ago'::interval;
                      interval
    --------------------------------------------
     -2005 years -4 mons -20 days -15:57:12.134
    (1 row)

--
  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/datetime.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/datetime.c,v
retrieving revision 1.147
diff -c -c -r1.147 datetime.c
*** src/backend/utils/adt/datetime.c    26 May 2005 15:26:00 -0000    1.147
--- src/backend/utils/adt/datetime.c    26 May 2005 18:10:03 -0000
***************
*** 3461,3472 ****
  #ifdef HAVE_INT64_TIMESTAMP
          sprintf(str + strlen(str), ":%02d.%06d", tm->tm_sec, fsec);
  #else
!         sprintf(str + strlen(str), ":%013.10f", tm->tm_sec + fsec);
  #endif
!         /* chop off trailing pairs of zeros... */
!         while (strcmp((str + strlen(str) - 2), "00") == 0 &&
!             *(str + strlen(str) - 3) != '.')
!             *(str + strlen(str) - 2) = '\0';
      }
      else
          sprintf(str + strlen(str), ":%02d", tm->tm_sec);
--- 3461,3469 ----
  #ifdef HAVE_INT64_TIMESTAMP
          sprintf(str + strlen(str), ":%02d.%06d", tm->tm_sec, fsec);
  #else
!         sprintf(str + strlen(str), ":%012.9f", tm->tm_sec + fsec);
  #endif
!         TrimTrailingZeros(str);
      }
      else
          sprintf(str + strlen(str), ":%02d", tm->tm_sec);
***************
*** 3804,3810 ****
                      sprintf(cp, ".%06d", Abs(fsec));
  #else
                      fsec += tm->tm_sec;
!                     sprintf(cp, ":%013.10f", fabs(fsec));
  #endif
                      TrimTrailingZeros(cp);
                      cp += strlen(cp);
--- 3801,3807 ----
                      sprintf(cp, ".%06d", Abs(fsec));
  #else
                      fsec += tm->tm_sec;
!                     sprintf(cp, ":%012.9f", fabs(fsec));
  #endif
                      TrimTrailingZeros(cp);
                      cp += strlen(cp);
Index: src/interfaces/ecpg/pgtypeslib/interval.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/ecpg/pgtypeslib/interval.c,v
retrieving revision 1.19
diff -c -c -r1.19 interval.c
*** src/interfaces/ecpg/pgtypeslib/interval.c    26 May 2005 16:44:05 -0000    1.19
--- src/interfaces/ecpg/pgtypeslib/interval.c    26 May 2005 18:10:04 -0000
***************
*** 511,517 ****
                      sprintf(cp, ".%06d", Abs(fsec));
  #else
                      fsec += tm->tm_sec;
!                     sprintf(cp, ":%013.10f", fabs(fsec));
  #endif
                      TrimTrailingZeros(cp);
                      cp += strlen(cp);
--- 511,517 ----
                      sprintf(cp, ".%06d", Abs(fsec));
  #else
                      fsec += tm->tm_sec;
!                      sprintf(cp, ":%012.9f", fabs(fsec));
  #endif
                      TrimTrailingZeros(cp);
                      cp += strlen(cp);
Index: src/test/regress/expected/horology.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/horology.out,v
retrieving revision 1.53
diff -c -c -r1.53 horology.out
*** src/test/regress/expected/horology.out    7 Apr 2005 01:51:40 -0000    1.53
--- src/test/regress/expected/horology.out    26 May 2005 18:10:07 -0000
***************
*** 205,266 ****
  -- As of 7.4, allow time without time zone having a time zone specified
  SELECT time without time zone '040506.789+08';
       time
! ---------------
!  04:05:06.7890
  (1 row)

  SELECT time without time zone '040506.789-08';
       time
! ---------------
!  04:05:06.7890
  (1 row)

  SELECT time without time zone 'T040506.789+08';
       time
! ---------------
!  04:05:06.7890
  (1 row)

  SELECT time without time zone 'T040506.789-08';
       time
! ---------------
!  04:05:06.7890
  (1 row)

  SELECT time with time zone '040506.789+08';
        timetz
! ------------------
!  04:05:06.7890+08
  (1 row)

  SELECT time with time zone '040506.789-08';
        timetz
! ------------------
!  04:05:06.7890-08
  (1 row)

  SELECT time with time zone 'T040506.789+08';
        timetz
! ------------------
!  04:05:06.7890+08
  (1 row)

  SELECT time with time zone 'T040506.789-08';
        timetz
! ------------------
!  04:05:06.7890-08
  (1 row)

  SELECT time with time zone 'T040506.789 +08';
        timetz
! ------------------
!  04:05:06.7890+08
  (1 row)

  SELECT time with time zone 'T040506.789 -08';
        timetz
! ------------------
!  04:05:06.7890-08
  (1 row)

  SET DateStyle = 'Postgres, MDY';
--- 205,266 ----
  -- As of 7.4, allow time without time zone having a time zone specified
  SELECT time without time zone '040506.789+08';
       time
! --------------
!  04:05:06.789
  (1 row)

  SELECT time without time zone '040506.789-08';
       time
! --------------
!  04:05:06.789
  (1 row)

  SELECT time without time zone 'T040506.789+08';
       time
! --------------
!  04:05:06.789
  (1 row)

  SELECT time without time zone 'T040506.789-08';
       time
! --------------
!  04:05:06.789
  (1 row)

  SELECT time with time zone '040506.789+08';
        timetz
! -----------------
!  04:05:06.789+08
  (1 row)

  SELECT time with time zone '040506.789-08';
        timetz
! -----------------
!  04:05:06.789-08
  (1 row)

  SELECT time with time zone 'T040506.789+08';
        timetz
! -----------------
!  04:05:06.789+08
  (1 row)

  SELECT time with time zone 'T040506.789-08';
        timetz
! -----------------
!  04:05:06.789-08
  (1 row)

  SELECT time with time zone 'T040506.789 +08';
        timetz
! -----------------
!  04:05:06.789+08
  (1 row)

  SELECT time with time zone 'T040506.789 -08';
        timetz
! -----------------
!  04:05:06.789-08
  (1 row)

  SET DateStyle = 'Postgres, MDY';

Bruce Momjian <pgman@candle.pha.pa.us> writes:
> OK, I have a new patch, which simplifies the code by using
> TrimTrailingZeros(), gives more consistent subsecond display, and
> subpresses the rounding problem:

Does anyone have any idea why the existing code is designed to keep the
number of displayed fractional digits even?   It seems a bit silly to
me too, but we probably ought not remove what was clearly an intentional
behavior without understanding why it was intentional.

Also, I will point out once more that the problem is not "we only have
nine digits of precision not ten".  The problem is that the precision
degrades as the interval gets larger.

regression=# select '20 days 15 hours 57 mins 12.1 secs ago'::interval;
           interval
-------------------------------
 -20 days -15:57:12.1000000001
(1 row)

regression=# select '100020 days 15 hours 57 mins 12.1 secs ago'::interval;
             interval
-----------------------------------
 -100020 days -15:57:12.1000003815
(1 row)

regression=# select '100000020 days 15 hours 57 mins 12.1 secs ago'::interval;
              interval
-------------------------------------
 -100000020 days -15:57:12.099609375
(1 row)

regression=#

Without accounting for that fundamental fact, you will not have a
solution, only a kluge.

            regards, tom lane

Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > OK, I have a new patch, which simplifies the code by using
> > TrimTrailingZeros(), gives more consistent subsecond display, and
> > subpresses the rounding problem:
>
> Does anyone have any idea why the existing code is designed to keep the
> number of displayed fractional digits even?   It seems a bit silly to
> me too, but we probably ought not remove what was clearly an intentional
> behavior without understanding why it was intentional.

If you look at TrimTrailingZeros(), you will see it keeps at least two
decimal digits of display, so I suppose the special case while() loop
was just an older version of that.  I can't imagine why they would only
want even digits, but given the other weird things in the datetime code,
it isn't surprising.

> Also, I will point out once more that the problem is not "we only have
> nine digits of precision not ten".  The problem is that the precision
> degrades as the interval gets larger.
>
> regression=# select '20 days 15 hours 57 mins 12.1 secs ago'::interval;
>            interval
> -------------------------------
>  -20 days -15:57:12.1000000001
> (1 row)
>
> regression=# select '100020 days 15 hours 57 mins 12.1 secs ago'::interval;
>              interval
> -----------------------------------
>  -100020 days -15:57:12.1000003815
> (1 row)
>
> regression=# select '100000020 days 15 hours 57 mins 12.1 secs ago'::interval;
>               interval
> -------------------------------------
>  -100000020 days -15:57:12.099609375
> (1 row)
>
> regression=#
>
> Without accounting for that fundamental fact, you will not have a
> solution, only a kluge.

Yep, it just hits the most common complaint for <30 days --- larger
values are going to round funny.  I can't think of how to round it
cleanly without adding a lot of hacky code dealing with floats and
exponents.

--
  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