Обсуждение: unsafe floats

Поиск
Список
Период
Сортировка

unsafe floats

От
Dennis Bjorklund
Дата:
When UNSAFE_FLOATS is defined there is a check that float results are 
within the min and max limits, which excludes values like 'Infinity', 
'-Infinity' and 'Nan'.

Is the above something from the SQL standard or just a bug?

The input rules for float8 accepts 'Infinity' as a value, and then it just
triggers a overflow error directly after (unless UNSAFE_FLOATS is
defined).

At first I thought it was a bug, but this function that checks for
overflow wouldn't even be needed if not to stop such values. Without the
check every possible value would be accepted (on normal IEEE math). I can
see a use in not accepting Infinity and Nan, but I would rather put that
as constraints if I needed that in my database.

Any thoughts? Should I go ahead and make it accept 'Infinity' and the 
rest as numbers?

-- 
/Dennis Björklund



Re: unsafe floats

От
Tom Lane
Дата:
Dennis Bjorklund <db@zigo.dhs.org> writes:
> When UNSAFE_FLOATS is defined there is a check that float results are 
> within the min and max limits, which excludes values like 'Infinity', 
> '-Infinity' and 'Nan'.

> Is the above something from the SQL standard or just a bug?

I think it was probably reasonable when the code was written (I'd guess
this goes back to very early 90s).  Nowadays, IEEE float math is nearly
universal, and we would be offering better functionality if we allowed
access to Infinity and Nan by default.  So I'd vote for ripping out the
range check, or at least reversing the default state of UNSAFE_FLOATS.

We might end up with two sets of regression expected files, one for
machines that do not support NaN, but that seems acceptable to me.

A variant idea is to try to get configure to detect Infinity/NaN support
and set UNSAFE_FLOATS only if it's there.  But I don't know if we can do
that without a run-time check, which Peter probably will complain about...
        regards, tom lane


Re: unsafe floats

От
Neil Conway
Дата:
Dennis Bjorklund <db@zigo.dhs.org> writes:
> When UNSAFE_FLOATS is defined there is a check that float results
> are within the min and max limits, which excludes values like
> 'Infinity', '-Infinity' and 'Nan'.

No, 'NaN' is legal float4/float8/numeric input whether UNSAFE_FLOATS
is defined or not.

> At first I thought it was a bug, but this function that checks for
> overflow wouldn't even be needed if not to stop such values.

Well, CheckFloat4Val() is needed to ensure that the input fits in a
'float' (rather than just a 'double').

> Should I go ahead and make it accept 'Infinity' and the rest as
> numbers?

What number would you like 'Infinity'::float4 and 'Infinity'::float8
to produce? Is this actually useful functionality?

As for it being in the SQL standard, using Acrobat's "text search"
feature finds zero instances of "infinity" (case insensitive) in the
200x draft spec. I haven't checked any more thoroughly than that,
though.

My inclination is to get rid of UNSAFE_FLOATS entirely, and disallow
'Infinity' and '-Infinity' input to float8 (since it never worked to
begin with, and float4 doesn't accept those values either). But I'm
open to other comments.

-Neil



Re: unsafe floats

От
Neil Conway
Дата:
Tom Lane <tgl@sss.pgh.pa.us> writes:
> Nowadays, IEEE float math is nearly universal, and we would be
> offering better functionality if we allowed access to Infinity and
> Nan by default.

This is faulty reasoning: we *do* allow NaN by default (although
you're correct that we reject Infinity in float8 input, but it seems
not by design).

> So I'd vote for ripping out the range check, or at least reversing
> the default state of UNSAFE_FLOATS.

This would surely be wrong. Defining UNSAFE_FLOATS will make
float4in() not check that its input fits into a 'float', rather than a
'double'.

> We might end up with two sets of regression expected files, one for
> machines that do not support NaN, but that seems acceptable to me.

Are there any modern machines that actually do not support NAN? There
are various places in the code that do

#ifndef NAN
#define NAN        (0.0/0.0)
#endif

... and this hasn't caused any problems that I'm aware of.

-Neil



Re: unsafe floats

От
Dennis Bjorklund
Дата:
On Wed, 10 Mar 2004, Neil Conway wrote:

> No, 'NaN' is legal float4/float8/numeric input whether UNSAFE_FLOATS
> is defined or not.

Yes, the tests are:
 if (fabs(val) > FLOAT8_MAX) if (val != 0.0 && fabs(val) < FLOAT8_MIN)

and only infinity and not NaN will trigger the overflow. I read it wrong 
first.

> Well, CheckFloat4Val() is needed to ensure that the input fits in a
> 'float' (rather than just a 'double').

Sure, for Float4 (maybe working with float in C instead of double and this 
check would make a difference, but I havn't really thought about that).

> What number would you like 'Infinity'::float4 and 'Infinity'::float8
> to produce? Is this actually useful functionality?

I would like them to produce the IEEE 754 number 'infinity' (usually 
writte 'Inf' in other languages).

> As for it being in the SQL standard, using Acrobat's "text search"
> feature finds zero instances of "infinity" (case insensitive) in the
> 200x draft spec. I haven't checked any more thoroughly than that,
> though.

If they say that it should use IEEE 754 math, then they do say that
Infinity is a number, just like it is in C and lots of other languages
with IEEE 754 math. Being as old as it is, I would guess that the standard
doesn't really say much at all about floats.

Why should pg not do the same as most (all?) other language that use IEEE 
754 math?

-- 
/Dennis Björklund



Re: unsafe floats

От
Tom Lane
Дата:
Neil Conway <neilc@samurai.com> writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> So I'd vote for ripping out the range check, or at least reversing
>> the default state of UNSAFE_FLOATS.

> This would surely be wrong. Defining UNSAFE_FLOATS will make
> float4in() not check that its input fits into a 'float', rather than a
> 'double'.

Possibly the appropriate test involves using isfinite() (apparently
spelled finite() some places, but the C99 spec says isfinite).  If
that returns false, take the value as good without checking range.
        regards, tom lane


Re: unsafe floats

От
Tom Lane
Дата:
Neil Conway <neilc@samurai.com> writes:
> What number would you like 'Infinity'::float4 and 'Infinity'::float8
> to produce? Is this actually useful functionality?

On an IEEE-spec machine, it's highly useful functionality.  +Infinity
and -Infinity are special values.

BTW the float4out and float8out routines already cope with NANs and
infinities, so ISTM the input routines should be able to reverse that.
(At the moment you might only be able to get an infinity inside the
system as the result of certain math functions.)
        regards, tom lane


Re: unsafe floats

От
Neil Conway
Дата:
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.*;


Re: unsafe floats

От
Dennis Bjorklund
Дата:
On Thu, 11 Mar 2004, Neil Conway wrote:

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

That's the issue and I think we should allow infinity as a result of a 
float operation (like you got in the example). It's part of IEEE 754 
math, so not getting Infinity as a result would break that. If someone 
does not want infinity stored in a column he/she could add a constraint to 
disallow it.

-- 
/Dennis Björklund



Re: unsafe floats

От
Dennis Bjorklund
Дата:
On Thu, 11 Mar 2004, Neil Conway wrote:

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

I have one question about the use of HUGE_VAL in postgresql. I got the
impression that the whole thing was designed to work on computers and
compilers where there is no infinity value, and then HUGE_VAL is defined
as the biggest number and treated as a special value.

If that is the case then using isinf() would not work (unless we have our
own). Well, maybe it's not an issue at all. Everything is IEEE 754 anyway
today.

A more important question is if we should give errors or produce Infinity
and NaN on mathematical operations. That is, should operations like
sqrt(-1.0) produce NaN or give an error.

-- 
/Dennis Björklund



Re: unsafe floats

От
Peter Eisentraut
Дата:
Dennis Bjorklund wrote:
> A more important question is if we should give errors or produce
> Infinity and NaN on mathematical operations. That is, should
> operations like sqrt(-1.0) produce NaN or give an error.

This needs to be definable by the user.  IEEE 754 compliant systems 
should provide a way to set the behavior in case of exception 
conditions such as these.  Maybe the C library provides one, or we need 
to catch this ourselves.



Re: unsafe floats

От
Dennis Bjorklund
Дата:
On Thu, 11 Mar 2004, Peter Eisentraut wrote:

> This needs to be definable by the user.  IEEE 754 compliant systems 
> should provide a way to set the behavior in case of exception 
> conditions such as these.  Maybe the C library provides one, or we need 
> to catch this ourselves.

In C one can set a signal handler to catch floating point exceptions 
(SIGFPE). Without a handler you can get NaN and Infinity as the result of 
mathematical operations.

-- 
/Dennis Björklund



Re: unsafe floats

От
Neil Conway
Дата:
Dennis Bjorklund <db@zigo.dhs.org> writes:
> In C one can set a signal handler to catch floating point exceptions
> (SIGFPE). Without a handler you can get NaN and Infinity as the
> result of mathematical operations.

Okay, I think this would be a reasonable set of behavior:

    - define a new GUC var that controls how exceptional floating
      point values (NaN, inf, -inf) are handled.

    - by default, we still raise an error when a floating point
      operation results in NaN / inf / etc.; if the GUC var is toggled
      from its default value, no error is raised. This preserves
      backward compatibility with applications that expect floating
      point overflow to be reported, for example.

    - that also means that, by default, we should reject 'NaN',
      'Infinity', and '-Infinity' as input to float4/float8: if these
      values are illegal as the result of FP operations by default, it
      seems only logical to disallow them as input to the FP types.

Does that sound ok?

Unfortunately, I have more important things that I need to get wrapped
up in time for 7.5, so I can't finish this now. Dennis, would you like
to implement this?

Finally, I've attached a revised patch for 'Infinity' input to float4
and float8: this avoids breaking the regression tests by only allowing
'Infinity' and '-Infinity' as input, not as a legal result of FP
operations. This is obviously incomplete, as discussed above, but it
might be a good starting point. Should I commit this?

-Neil

Index: doc/src/sgml/syntax.sgml
===================================================================
RCS file: /var/lib/cvs/pgsql-server/doc/src/sgml/syntax.sgml,v
retrieving revision 1.89
diff -c -r1.89 syntax.sgml
*** a/doc/src/sgml/syntax.sgml    29 Nov 2003 19:51:37 -0000    1.89
--- b/doc/src/sgml/syntax.sgml    11 Mar 2004 21:18:57 -0000
***************
*** 359,364 ****
--- 359,382 ----
  </literallayout>
      </para>

+      <para>
+       In addition, there are several special constant values that are
+       accepted as numeric constants. The <type>float4</type> and
+       <type>float8</type> types allow the following special constants:
+ <literallayout>
+ Infinity
+ -Infinity
+ NaN
+ </literallayout>
+       These represnt the special values <quote>infinity</quote>,
+       <quote>negative infinity</quote>, and
+       <quote>not-a-number</quote>, respectively. The
+       <type>numeric</type> type only allows <literal>NaN</>, and the
+       integral types do not allow any of these constants. These
+       constants are treated without sensitivity to case. These values
+       should be enclosed in single quotes.
+      </para>
+
      <para>
       <indexterm><primary>integer</primary></indexterm>
       <indexterm><primary>bigint</primary></indexterm>
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 20:53:15 -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,127 ----


  /*
!  * 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 (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),
--- 130,146 ----
          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 (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 */
  }

  /*
--- 149,154 ----
***************
*** 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')
          {
--- 183,188 ----
***************
*** 217,222 ****
--- 195,204 ----
          }
          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),
***************
*** 239,245 ****
       * if we get here, we have a legal double, still need to check to see
       * if it's a legal float
       */
!     CheckFloat4Val(val);

      PG_RETURN_FLOAT4((float4) val);
  }
--- 221,228 ----
       * if we get here, we have a legal double, still need to check to see
       * if it's a legal float
       */
!     if (!isinf(val))
!         CheckFloat4Val(val);

      PG_RETURN_FLOAT4((float4) val);
  }
***************
*** 364,370 ****
                   errmsg("invalid input syntax for type double precision: \"%s\"",
                          num)));

!     CheckFloat8Val(val);

      PG_RETURN_FLOAT8(val);
  }
--- 347,354 ----
                   errmsg("invalid input syntax for type double precision: \"%s\"",
                          num)));

!     if (!isinf(val))
!         CheckFloat8Val(val);

      PG_RETURN_FLOAT8(val);
  }
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 20:51:46 -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/expected/float4.out
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/test/regress/expected/float4.out,v
retrieving revision 1.10
diff -c -r1.10 float4.out
*** a/src/test/regress/expected/float4.out    11 Mar 2004 02:11:13 -0000    1.10
--- b/src/test/regress/expected/float4.out    11 Mar 2004 21:04:58 -0000
***************
*** 50,58 ****
--- 50,88 ----
      NaN
  (1 row)

+ SELECT 'infinity'::float4;
+   float4
+ ----------
+  Infinity
+ (1 row)
+
+ SELECT '          -INFINiTY   '::float4;
+   float4
+ -----------
+  -Infinity
+ (1 row)
+
  -- bad special inputs
  SELECT 'N A N'::float4;
  ERROR:  invalid input syntax for type real: "N A N"
+ SELECT 'NaN x'::float4;
+ ERROR:  invalid input syntax for type real: "NaN x"
+ SELECT ' INFINITY    x'::float4;
+ ERROR:  invalid input syntax for type real: " INFINITY    x"
+ SELECT 'Infinity'::float4 + 100.0;
+ ERROR:  type "double precision" value out of range: overflow
+ SELECT 'Infinity'::float4 / 'Infinity'::float4;
+  ?column?
+ ----------
+       NaN
+ (1 row)
+
+ SELECT 'nan'::float4 / 'nan'::float4;
+  ?column?
+ ----------
+       NaN
+ (1 row)
+
  SELECT '' AS five, FLOAT4_TBL.*;
   five |     f1
  ------+-------------
Index: src/test/regress/expected/float8.out
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/test/regress/expected/float8.out,v
retrieving revision 1.18
diff -c -r1.18 float8.out
*** a/src/test/regress/expected/float8.out    11 Mar 2004 02:11:13 -0000    1.18
--- b/src/test/regress/expected/float8.out    11 Mar 2004 21:04:56 -0000
***************
*** 50,58 ****
--- 50,88 ----
      NaN
  (1 row)

+ SELECT 'infinity'::float8;
+   float8
+ ----------
+  Infinity
+ (1 row)
+
+ SELECT '          -INFINiTY   '::float8;
+   float8
+ -----------
+  -Infinity
+ (1 row)
+
  -- bad special inputs
  SELECT 'N A N'::float8;
  ERROR:  invalid input syntax for type double precision: "N A N"
+ SELECT 'NaN x'::float8;
+ ERROR:  invalid input syntax for type double precision: "NaN x"
+ SELECT ' INFINITY    x'::float8;
+ ERROR:  invalid input syntax for type double precision: " INFINITY    x"
+ SELECT 'Infinity'::float8 + 100.0;
+ ERROR:  type "double precision" value out of range: overflow
+ SELECT 'Infinity'::float8 / 'Infinity'::float8;
+  ?column?
+ ----------
+       NaN
+ (1 row)
+
+ SELECT 'nan'::float8 / 'nan'::float8;
+  ?column?
+ ----------
+       NaN
+ (1 row)
+
  SELECT '' AS five, FLOAT8_TBL.*;
   five |          f1
  ------+----------------------
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 20:51:46 -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 20:51:46 -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.*;


Re: unsafe floats

От
Tom Lane
Дата:
Dennis Bjorklund <db@zigo.dhs.org> writes:
> On Thu, 11 Mar 2004, Neil Conway wrote:
>> 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?

> That's the issue and I think we should allow infinity as a result of a 
> float operation (like you got in the example). It's part of IEEE 754 
> math, so not getting Infinity as a result would break that.

This would still be infinitely (ahem) better than the behavior you get
when an integer operation overflows.  We return whatever the hardware
gives in such cases.  Returning whatever the hardware gives for a float
overflow doesn't seem out of line, particularly if it's a well-defined
behavior.

I am somewhat concerned about the prospect of implementation-dependent
results --- machines that do not implement IEEE-style math are going to
return something other than an Infinity.  However, I think that
providing access to the IEEE semantics is more useful than a (rather
vain) attempt to impose uniformity across IEEE and non-IEEE machines.
        regards, tom lane


Re: unsafe floats

От
Tom Lane
Дата:
Neil Conway <neilc@samurai.com> writes:
> Okay, I think this would be a reasonable set of behavior:

>     - define a new GUC var that controls how exceptional floating
>       point values (NaN, inf, -inf) are handled.

>     - by default, we still raise an error when a floating point
>       operation results in NaN / inf / etc.; if the GUC var is toggled
>       from its default value, no error is raised. This preserves
>       backward compatibility with applications that expect floating
>       point overflow to be reported, for example.

That sounds okay.  Also we might want to distinguish NaN from Infinity
--- I would expect most people to want zero-divide to continue to get
reported, for instance, even if they want to get Infinity for overflow.

>     - that also means that, by default, we should reject 'NaN',
>       'Infinity', and '-Infinity' as input to float4/float8: if these
>       values are illegal as the result of FP operations by default, it
>       seems only logical to disallow them as input to the FP types.

This I disagree with.  It would mean, for example, that you could not
dump and reload columns containing such data unless you remembered to
switch the variable first.  If you did this then I'd insist on pg_dump
scripts setting the variable to the permissive state.  In any case,
I don't see why a restriction that certain operations can't produce a
certain value should render the value illegal overall.
        regards, tom lane


Re: unsafe floats

От
Neil Conway
Дата:
Tom Lane <tgl@sss.pgh.pa.us> writes:
> That sounds okay.  Also we might want to distinguish NaN from
> Infinity --- I would expect most people to want zero-divide to
> continue to get reported, for instance, even if they want to get
> Infinity for overflow.

Yeah, good point.

> This I disagree with.  It would mean, for example, that you could not
> dump and reload columns containing such data unless you remembered to
> switch the variable first.

Hmmm... on reflection, you're probably correct.

Since that removes the potential objection to the previous patch, I've
applied it to CVS.

-Neil