Re: unsafe floats - Mailing list pgsql-hackers

Dennis Bjorklund <db@zigo.dhs.org> writes:
> I would like them to produce the IEEE 754 number 'infinity' (usually
> writte 'Inf' in other languages).

Fair enough. Attached is a patch that implements this. I chose to
remove UNSAFE_FLOATS: if anyone thinks that is worth keeping, speak up
now.

Example behavior:

nconway=# select 'Infinity'::float4, '-Infinity'::float8;
  float4  |  float8
----------+-----------
 Infinity | -Infinity
(1 row)

However, this patch causes the regression tests to fail. The reason
for that is the following change in behavior:

nconway=# select '-1.2345678901234e+200'::float8 * '1e200';
 ?column?
-----------
 -Infinity
(1 row)

Without the patch, we bail out due to overflow:

nconway=# select '-1.2345678901234e+200'::float8 * '1e200';
ERROR:  type "double precision" value out of range: overflow

The problem is that CheckFloat8Val() is used in two places: in
float8in() to check that the input fits inside a float8 (which is a
little silly, since strtod() by definition returns something that fits
inside a double), and after various float8 operations (including
multiplication).

As part of the patch, I modified CheckFloat8Val() to not reject
infinite FP values. What happens in the example above is that the C
multiplication operation performed by float8mul() returns '-Infinity'
-- prior to the patch, CheckFloat8Val() rejected that as an overflow,
but with the patch it no longer does.

So, what is the correct behavior: if you multiply two values and get a
result that exceeds the range of a float8, should you get
'Infinity'/'-Infinity', or an overflow error?

(Either policy is implementable: in the former case, we'd check for an
infinite input in float8in() but outside of CheckFloat8Val(), and in
the latter case we'd just remove the overflow checking for floating
point ops.)

Comments?

-Neil
Index: src/backend/utils/adt/float.c
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/backend/utils/adt/float.c,v
retrieving revision 1.98
diff -c -r1.98 float.c
*** a/src/backend/utils/adt/float.c    11 Mar 2004 02:11:13 -0000    1.98
--- b/src/backend/utils/adt/float.c    11 Mar 2004 02:43:43 -0000
***************
*** 114,134 ****


  /*
!  * check to see if a float4 val is outside of
!  * the FLOAT4_MIN, FLOAT4_MAX bounds.
   *
!  * raise an ereport warning if it is
! */
  static void
  CheckFloat4Val(double val)
  {
!     /*
!      * defining unsafe floats's will make float4 and float8 ops faster at
!      * the cost of safety, of course!
!      */
! #ifdef UNSAFE_FLOATS
!     return;
! #else
      if (fabs(val) > FLOAT4_MAX)
          ereport(ERROR,
                  (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
--- 114,130 ----


  /*
!  * check to see if a float4 val is outside of the FLOAT4_MIN,
!  * FLOAT4_MAX bounds.
   *
!  * raise an ereport() error if it is
!  */
  static void
  CheckFloat4Val(double val)
  {
!     if (isinf(val))
!         return;
!
      if (fabs(val) > FLOAT4_MAX)
          ereport(ERROR,
                  (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
***************
*** 137,163 ****
          ereport(ERROR,
                  (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                   errmsg("type \"real\" value out of range: underflow")));
-
-     return;
- #endif   /* UNSAFE_FLOATS */
  }

  /*
!  * check to see if a float8 val is outside of
!  * the FLOAT8_MIN, FLOAT8_MAX bounds.
   *
!  * raise an ereport error if it is
   */
  static void
  CheckFloat8Val(double val)
  {
!     /*
!      * defining unsafe floats's will make float4 and float8 ops faster at
!      * the cost of safety, of course!
!      */
! #ifdef UNSAFE_FLOATS
!     return;
! #else
      if (fabs(val) > FLOAT8_MAX)
          ereport(ERROR,
                  (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
--- 133,152 ----
          ereport(ERROR,
                  (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                   errmsg("type \"real\" value out of range: underflow")));
  }

  /*
!  * check to see if a float8 val is outside of the FLOAT8_MIN,
!  * FLOAT8_MAX bounds.
   *
!  * raise an ereport() error if it is
   */
  static void
  CheckFloat8Val(double val)
  {
!     if (isinf(val))
!         return;
!
      if (fabs(val) > FLOAT8_MAX)
          ereport(ERROR,
                  (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
***************
*** 166,172 ****
          ereport(ERROR,
                  (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                   errmsg("type \"double precision\" value out of range: underflow")));
- #endif   /* UNSAFE_FLOATS */
  }

  /*
--- 155,160 ----
***************
*** 201,210 ****
           * empty strings, but emit a warning noting that the feature
           * is deprecated. In 7.6+, the warning should be replaced by
           * an error.
-          *
-          * XXX we should accept "Infinity" and "-Infinity" too, but
-          * what are the correct values to assign?  HUGE_VAL will
-          * provoke an error from CheckFloat4Val.
           */
          if (*num == '\0')
          {
--- 189,194 ----
***************
*** 217,222 ****
--- 201,210 ----
          }
          else if (strcasecmp(num, "NaN") == 0)
              val = NAN;
+         else if (strcasecmp(num, "Infinity") == 0)
+             val = HUGE_VAL;
+         else if (strcasecmp(num, "-Infinity") == 0)
+             val = -HUGE_VAL;
          else
              ereport(ERROR,
                      (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
Index: src/include/pg_config_manual.h
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/include/pg_config_manual.h,v
retrieving revision 1.10
diff -c -r1.10 pg_config_manual.h
*** a/src/include/pg_config_manual.h    11 Feb 2004 22:55:26 -0000    1.10
--- b/src/include/pg_config_manual.h    11 Mar 2004 04:48:08 -0000
***************
*** 176,187 ****
  #define DEFAULT_PGSOCKET_DIR  "/tmp"

  /*
-  * Defining this will make float4 and float8 operations faster by
-  * suppressing overflow/underflow checks.
-  */
- /* #define UNSAFE_FLOATS */
-
- /*
   * The random() function is expected to yield values between 0 and
   * MAX_RANDOM_VALUE.  Currently, all known implementations yield
   * 0..2^31-1, so we just hardwire this constant.  We could do a
--- 176,181 ----
Index: src/test/regress/sql/float4.sql
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/test/regress/sql/float4.sql,v
retrieving revision 1.5
diff -c -r1.5 float4.sql
*** a/src/test/regress/sql/float4.sql    11 Mar 2004 02:11:13 -0000    1.5
--- b/src/test/regress/sql/float4.sql    11 Mar 2004 04:59:32 -0000
***************
*** 29,36 ****
--- 29,45 ----
  SELECT 'NaN'::float4;
  SELECT 'nan'::float4;
  SELECT '   NAN  '::float4;
+ SELECT 'infinity'::float4;
+ SELECT '          -INFINiTY   '::float4;
  -- bad special inputs
  SELECT 'N A N'::float4;
+ SELECT 'NaN x'::float4;
+ SELECT ' INFINITY    x'::float4;
+
+ SELECT 'Infinity'::float4 + 100.0;
+ SELECT 'Infinity'::float4 / 'Infinity'::float4;
+ SELECT 'nan'::float4 / 'nan'::float4;
+

  SELECT '' AS five, FLOAT4_TBL.*;

Index: src/test/regress/sql/float8.sql
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/test/regress/sql/float8.sql,v
retrieving revision 1.9
diff -c -r1.9 float8.sql
*** a/src/test/regress/sql/float8.sql    11 Mar 2004 02:11:13 -0000    1.9
--- b/src/test/regress/sql/float8.sql    11 Mar 2004 05:38:11 -0000
***************
*** 29,36 ****
--- 29,44 ----
  SELECT 'NaN'::float8;
  SELECT 'nan'::float8;
  SELECT '   NAN  '::float8;
+ SELECT 'infinity'::float8;
+ SELECT '          -INFINiTY   '::float8;
  -- bad special inputs
  SELECT 'N A N'::float8;
+ SELECT 'NaN x'::float8;
+ SELECT ' INFINITY    x'::float8;
+
+ SELECT 'Infinity'::float8 + 100.0;
+ SELECT 'Infinity'::float8 / 'Infinity'::float8;
+ SELECT 'nan'::float8 / 'nan'::float8;

  SELECT '' AS five, FLOAT8_TBL.*;