Re: Re: [GENERAL] +/- Inf for float8's - Mailing list pgsql-hackers

From Thomas Lockhart
Subject Re: Re: [GENERAL] +/- Inf for float8's
Date
Msg-id 3998B8BB.2A8D7013@alumni.caltech.edu
Whole thread Raw
In response to Re: [GENERAL] +/- Inf for float8's  ("Ross J. Reedstrom" <reedstrm@rice.edu>)
Responses Re: Re: [GENERAL] +/- Inf for float8's  ("Ross J. Reedstrom" <reedstrm@rice.edu>)
Re: Re: [GENERAL] +/- Inf for float8's  ("Ross J. Reedstrom" <reedstrm@rice.edu>)
List pgsql-hackers
> > So, any suggestions as to how we can store +/- infinity as a valid float8
> > value in a database table?
> Right: the SQL standard doesn't say anything about what to do for these
> cases for floats (except by defining the syntax of an approximate numeric
> constant as basically a float), but the IEEE754 does: as you discovered
> below, they're NaN, -Infinity, and +Infinity.

Not all computers fully support IEEE754, though many new ones do.

> > I notice, btw, that 'NaN' is accepted as a valid float8. Is there any
> > particular reason why something similar for, eg '-Inf' and 'Inf' doesn't
> > also exist? Just discovered, there is a special number 'Infinity', which
> > seems to be recognised, except you can't insert it into a table because it
> > reports an overflow error. Getting warm, it seems, but not there yet. And
> > there doesn't seem to be a negative equivalent.
> And this is a bug. From looking at the source, I see that Thomas added
> code to accept 'NaN' and 'Infinity' (but not '-Infinity'), and Tom Lane
> tweaked it, but it's never been able to get an Infinity all the way to
> the table, as far as I can see: the value gets set to HUGE_VAL, but the
> call to CheckFloat8Val compares against FLOAT8_MAX (and FLOAT8_MIN),
> and complains, since HUGE_VAL is _defined_ to be larger than DBL_MAX.
> And, there's no test case in the regression tests for inserting NaN or
> Infinity. (Shame on Thomas ;-)

Ah, I'm just trying to leave some rewarding work for other folks ;)

> I think the right thing to do is move the call to CheckFloat8Val into a
> branch of the test for NaN and Infinity, thereby not calling it if we've
> been passed those constants. I'm compiling up a test of this right now,
> and I'll submit a patch to Bruce if it passes regression. Looks like
> that function hasn't been touch in a while, so the patch should apply
> to 7.0.X as well as current CVS.

istm that the existing protection (or something like it) is required for
some platforms, while other platforms may be able to handle NaN and
+/-Inf just fine. Seems like a job for autoconf to determine the FP
capabilities of a system, unless Posix defines some way to tell. Of
course, even then we'd need an autoconf test to deal with non-Posix
platforms.

> Looks like it works, and passes the regression tests as they are.  I'm
> patching the tests to include the cases 'NaN', 'Infinity', and '-Infinity'
> as valid float8s, and 'not a float' as an invalid representation, and
> rerunning to get output to submit with the patch. This might be a bit
> hairy, since there are 5 different expected/float8* files. Should I try
> to hand patch them to deal with the new rows, or let them be regenerated
> by people with the appropriate platforms?

How about setting up a separate test (say, ieee754.sql) so that
non-compliant platforms can still pass the original FP test suite. Then
other platforms can be added in as they are tested.

Some platforms may need their compiler switches tweaked; I haven't
checked the Alpha/DUnix configuration but I recall needing to fix some
flags to get compiled code to move these edge cases around even just
through subroutine calls. One example was in trying to call finite(),
which threw an error during the call to it if the number was NaN or
Infinity. Which sort of defeated the purpose of the call :)

> Bigger problem with changing the float8 regression tests: a lot of our
> math functions seem to be guarded with CheckFloat8Val(result), so, if we
> allow these values in a float8 column, most of the math functions with
> elog(). It strikes me that there must have been a reason for this at one
> time. There's even a #define UNSAFE_FLOATS, to disable these checks. By
> reading the comments in old copies of float.c, it looks like this was
> added for an old, buggy linux/Alpha libc that would throw floating point
> exceptions, otherwise.

There are still reasons on some platforms, as noted above...

> Is there an intrinsic problem with allowing values outside the range
> FLOAT8_MAX <= x =>FLOAT8_MIN ? 'ORDER BY' seems to still work, with
> 'Infinity' and '-Infinity' sorting properly. Having a 'NaN' in there
> breaks sorting however.  That's a current, live bug.  Could be fixed
> by treating 'NaN' as a different flavor of NULL. Probably a fairly deep
> change, however. Hmm, NULL in a float8 sorts to the end, regardless of
> ASC or DESC, is that right?

NULL and NaN are not quite the same thing imho. If we are allowing NaN
in columns, then it is *known* to be NaN.

> Anyway, here's the patch for just float.c , if anyone wants to look
> at it. As I said, it passes the existing float8 regression tests, but
> raises a lot of interesting questions.

Are you interested in pursuing this further? It seems like we might be
able to move in the direction you suggest on *some* platforms, but we
will need to scrub the math functions to be able to handle these edge
cases.
                   - Thomas


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: FWD: Bugs with GRANT ?
Next
From: Louis-David Mitterrand
Date:
Subject: Re: modifying a timestamp in a C trigger