Fix parsing of ISO-8601 interval fields with exponential notation.

Historically we've accepted interval input like 'P.1e10D'.  This
is probably an accident of having used strtod() to do the parsing,
rather than something anyone intended, but it's been that way for
a long time.  Commit e39f99046 broke this by trying to parse the
integer and fractional parts separately, without accounting for
the possibility of an exponent.  In principle that coding allowed
for precise conversions of field values wider than 15 decimal
digits, but that does not seem like a goal worth sweating bullets
for.  So, rather than trying to manage an exponent on top of the
existing complexity, let's just revert to the previous coding that
used strtod() by itself.  We can still improve on the old code to
the extent of allowing the value to range up to 1.0e15 rather than
only INT_MAX.  (Allowing more than that risks creating problems
due to precision loss: the converted fractional part might have
absolute value more than 1.  Perhaps that could be dealt with in
some way, but it really does not seem worth additional effort.)

Per bug #17795 from Alexander Lakhin.  Back-patch to v15 where
the faulty code came in.

Discussion: https://postgr.es/m/17795-748d6db3ed95d313@postgresql.org

diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index b74889039d89754df8cacee262a16e6217ce98bc..01660637a24d69db94ca750d66760748c319152c 100644 (file)
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -3702,46 +3702,38 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 static int
 ParseISO8601Number(char *str, char **endptr, int64 *ipart, double *fpart)
 {
-       int                     sign = 1;
+       double          val;
 
-       *ipart = 0;
-       *fpart = 0.0;
-
-       /* Parse sign if there is any */
-       if (*str == '-')
-       {
-               sign = -1;
-               str++;
-       }
-
-       *endptr = str;
+       /*
+        * Historically this has accepted anything that strtod() would take,
+        * notably including "e" notation, so continue doing that.  This is
+        * slightly annoying because the precision of double is less than that of
+        * int64, so we would lose accuracy for inputs larger than 2^53 or so.
+        * However, historically we rejected inputs outside the int32 range,
+        * making that concern moot.  What we do now is reject abs(val) above
+        * 1.0e15 (a round number a bit less than 2^50), so that any accepted
+        * value will have an exact integer part, and thereby a fraction part with
+        * abs(*fpart) less than 1.  In the absence of field complaints it doesn't
+        * seem worth working harder.
+        */
+       if (!(isdigit((unsigned char) *str) || *str == '-' || *str == '.'))
+               return DTERR_BAD_FORMAT;
        errno = 0;
-
-       /* Parse int64 part if there is any */
-       if (isdigit((unsigned char) **endptr))
-               *ipart = strtoi64(*endptr, endptr, 10) * sign;
-
-       /* Parse fractional part if there is any */
-       if (**endptr == '.')
-       {
-               /*
-                * As in ParseFraction, some versions of strtod insist on seeing some
-                * digits after '.', but some don't.  We want to allow zero digits
-                * after '.' as long as there were some before it.
-                */
-               if (isdigit((unsigned char) *(*endptr + 1)))
-                       *fpart = strtod(*endptr, endptr) * sign;
-               else
-               {
-                       (*endptr)++;            /* advance over '.' */
-                       str++;                          /* so next test will fail if no digits */
-               }
-       }
-
-       /* did we not see anything that looks like a number? */
+       val = strtod(str, endptr);
+       /* did we not see anything that looks like a double? */
        if (*endptr == str || errno != 0)
                return DTERR_BAD_FORMAT;
-
+       /* watch out for overflow, including infinities; reject NaN too */
+       if (isnan(val) || val < -1.0e15 || val > 1.0e15)
+               return DTERR_FIELD_OVERFLOW;
+       /* be very sure we truncate towards zero (cf dtrunc()) */
+       if (val >= 0)
+               *ipart = (int64) floor(val);
+       else
+               *ipart = (int64) -floor(-val);
+       *fpart = val - *ipart;
+       /* Callers expect this to hold */
+       Assert(*fpart > -1.0 && *fpart < 1.0);
        return 0;
 }
 

diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index c7ac408bec803d60c4802b10e4f3a823d047f134..a154840c85701acd1a5c9ddad311b178b4bc10f4 100644 (file)
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -975,6 +975,12 @@ select interval 'P.1Y0M3DT4H5M6S';
  1 mon 3 days 04:05:06
 (1 row)
 
+select interval 'P10.5e4Y';  -- not per spec, but we've historically taken it
+   interval   
+--------------
+ 105000 years
+(1 row)
+
 select interval 'P.Y0M3DT4H5M6S';  -- error
 ERROR:  invalid input syntax for type interval: "P.Y0M3DT4H5M6S"
 LINE 1: select interval 'P.Y0M3DT4H5M6S';
@@ -1081,13 +1087,13 @@ select interval 'PT2562047788:00:54.775807';
 select interval 'PT2562047788.0152155019444';
              interval              
 -----------------------------------
- @ 2562047788 hours 54.775807 secs
+ @ 2562047788 hours 54.775429 secs
 (1 row)
 
 select interval 'PT-2562047788.0152155022222';
                interval                
 ---------------------------------------
- @ 2562047788 hours 54.775808 secs ago
+ @ 2562047788 hours 54.775429 secs ago
 (1 row)
 
 -- overflow each date/time field

diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql
index 54745c40d6d435efb4a28b72df14f0bf028b3614..af8e1ca0f4b724a89e358da1b9d21e8f36002f4f 100644 (file)
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -328,6 +328,7 @@ select interval 'P1.0Y0M3DT4H5M6S';
 select interval 'P1.1Y0M3DT4H5M6S';
 select interval 'P1.Y0M3DT4H5M6S';
 select interval 'P.1Y0M3DT4H5M6S';
+select interval 'P10.5e4Y';  -- not per spec, but we've historically taken it
 select interval 'P.Y0M3DT4H5M6S';  -- error
 
 -- test a couple rounding cases that changed since 8.3 w/ HAVE_INT64_TIMESTAMP.