Re: [HACKERS] unsafe floats
Dennis Bjorklund [EMAIL PROTECTED] 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 - 1.89
--- b/doc/src/sgml/syntax.sgml 11 Mar 2004 21:18:57 -
***
*** 359,364
--- 359,382
/literallayout
/para
+ para
+ In addition, there are several special constant values that are
+ accepted as numeric constants. The typefloat4/type and
+ typefloat8/type types allow the following special constants:
+ literallayout
+ Infinity
+ -Infinity
+ NaN
+ /literallayout
+ These represnt the special values quoteinfinity/quote,
+ quotenegative infinity/quote, and
+ quotenot-a-number/quote, respectively. The
+ typenumeric/type type only allows literalNaN/, 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
indextermprimaryinteger/primary/indexterm
indextermprimarybigint/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 - 1.98
--- b/src/backend/utils/adt/float.c 11 Mar 2004 20:53:15 -
***
*** 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,
Re: [HACKERS] unsafe floats
Dennis Bjorklund [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] unsafe floats
Neil Conway [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [HACKERS] unsafe floats
Tom Lane [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [HACKERS] unsafe floats
Dennis Bjorklund [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [HACKERS] unsafe floats
Dennis Bjorklund [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] unsafe floats
Tom Lane [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [HACKERS] unsafe floats
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 ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] unsafe floats
Neil Conway [EMAIL PROTECTED] writes: Tom Lane [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [HACKERS] unsafe floats
Neil Conway [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [HACKERS] unsafe floats
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 ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [HACKERS] unsafe floats
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 ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
