Re: [BUGS] BUG #2846: inconsistent and confusing handling of

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>
Cc: Roman Kononov <kononov195-pgsql(at)yahoo(dot)com>
Subject: Re: [BUGS] BUG #2846: inconsistent and confusing handling of
Date: 2006-12-27 18:44:31
Message-ID: 200612271844.kBRIiVb18465@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches


I have made some more progress on this patch. I have fixed the issue
with aggregates:

test=> select avg(ff) from tt;
ERROR: type "double precision" value out of range: overflow

and tested the performance overhead of the new CheckFloat8Val() calls
the fix requires using:

EXPLAIN ANALYZE SELECT AVG(x.COL)
FROM (SELECT 323.2 AS COL FROM generate_series(1,1000000)) AS x;

and could not measure any overhead.

I also found a few more bugs, this one with float4 negation:

test=> SELECT -('0'::float4);
?column?
----------
-0
(1 row)

test=> SELECT -('0'::float8);
?column?
----------
0
(1 row)

and this one with casting 'Nan' to an integer:

test=> SELECT 'Nan'::float8::int4;
int4
-------------
-2147483648
(1 row)

I have fixed these as well:

test=> SELECT -('0'::float4);
?column?
----------
0
(1 row)

test=> SELECT 'Nan'::float8::int4;
ERROR: integer out of range

The only unsolved issue is the one with underflow checks. I have added
comments explaining the problem in case someone ever figures out how to
address it.

If I don't receive further comments, I will apply the new attached patch
shortly.

---------------------------------------------------------------------------

bruce wrote:
> Roman Kononov wrote:
> >
> > The following bug has been logged online:
> >
> > Bug reference: 2846
> > Logged by: Roman Kononov
> > Email address: kononov195-pgsql(at)yahoo(dot)com
> > PostgreSQL version: 8.2.0 and older
> > Operating system: linux 2.6.15-27-amd64 ubuntu
> > Description: inconsistent and confusing handling of underflows, NaNs
> > and INFs
> > Details:
>
> This is a very interesting bug report. It seems you have done some good
> analysis of PostgreSQL and how it handles certain corner cases,
> infinity, and NaN.
>
> I have researched your findings and will show some fixes below:
>
> > Please compare the results of the simple queries.
> > ==============================================
> > test=# select ('NaN'::float4)::int2;
> > int2
> > ------
> > 0
> > (1 row)
>
> There certainly should be an isnan() test when converting to int2
> because while float can represent NaN, int2 cannot. The fix shows:
>
> test=> select ('NaN'::float4)::int2;
> ERROR: smallint out of range
>
> > test=# select ('NaN'::float4)::int4;
> > int4
> > -------------
> > -2147483648
> > (1 row)
>
> Same for int4:
>
> test=> select ('NaN'::float4)::int4;
> ERROR: integer out of range
>
> > test=# select ('NaN'::float4)::int8;
> > ERROR: bigint out of range
>
> This one was correct because it uses rint() internally.
>
> > test=# select ('nan'::numeric)::int4;
> > ERROR: cannot convert NaN to integer
> > ==============================================
> > test=# select abs('INF'::float4);
> > abs
> > ----------
> > Infinity
> > (1 row)
>
> Correct.
>
> > test=# select abs('INF'::float8);
> > ERROR: type "double precision" value out of range: overflow
>
> This one was more complicated. float4/8 operations test for
> results > FLOAT[84]_MAX. This is because if you do this:
>
> test=> select (1e201::float8)*(1e200::float8);
>
> the result internally is Infinity, so they check for Inf as a check for
> overflow. The bottom line is that while the current code allows
> infinity to be entered, it does not allow the value to operate in many
> context because it is assumes Inf to be an overflow indicator. I have
> fixed this by passing a boolean to indicate if any of the operands were
> infinity, and if so, allow an infinite result, so this now works:
>
> test=> select abs('INF'::float8);
> abs
> ----------
> Infinity
> (1 row)
>
> > ==============================================
> > test=# select -('INF'::float4);
> > ?column?
> > -----------
> > -Infinity
> > (1 row)
> >
> > test=# select -('INF'::float8);
> > ERROR: type "double precision" value out of range: overflow
>
> And this now works too:
>
> test=> select -('INF'::float8);
> ?column?
> -----------
> -Infinity
> (1 row)
>
> > ==============================================
> > test=# select (1e-37::float4)*(1e-22::float4);
> > ?column?
> > ----------
> > 0
> > (1 row)
>
> This one is quite complex. For overflow, there is a range of values
> that is represented as > FLOAT8_MAX, but for values very large, the
> result becomes Inf. The old code assumed an Inf result was an overflow,
> and threw an error, as I outlined above. The new code does a better
> job.
>
> Now, for underflow. For underflow, we again have a range slightly
> smaller than DBL_MIN where we can detect an underflow, and throw an
> error, but just like overflow, if the underflow is too small, the result
> becomes zero. The bad news is that unlike Inf, zero isn't a special
> value. With Inf, we could say if we got an infinite result from
> non-infinite arguments, we had an overflow, but for underflow, how do we
> know if zero is an underflow or just the correct result? For
> multiplication, we could say that a zero result for non-zero arguments
> is almost certainly an underflow, but I don't see how we can handle the
> other operations as simply.
>
> I was not able to fix the underflow problems you reported.
>
> > test=# select (1e-37::float4)*(1e-2::float4);
> > ERROR: type "real" value out of range: underflow
> > ==============================================
> > test=# select (1e-300::float8)*(1e-30::float8);
> > ?column?
> > ----------
> > 0
> > (1 row)
> >
> > test=# select (1e-300::float8)*(1e-20::float8);
> > ERROR: type "double precision" value out of range: underflow
> > ==============================================
> > test=# select ('INF'::float8-'INF'::float8);
> > ?column?
> > ----------
> > NaN
> > (1 row)
> >
> > test=# select ('INF'::float8+'INF'::float8);
> > ERROR: type "double precision" value out of range: overflow
>
> This works fine now:
>
> test=> select ('INF'::float8+'INF'::float8);
> ?column?
> ----------
> Infinity
> (1 row)
>
> > ==============================================
> > test=# select ('INF'::float4)::float8;
> > float8
> > ----------
> > Infinity
> > (1 row)
> >
> > test=# select ('INF'::float8)::float4;
> > ERROR: type "real" value out of range: overflow
> > ==============================================
> > test=# select cbrt('INF'::float4);
> > cbrt
> > ----------
> > Infinity
> > (1 row)
> >
> > test=# select sqrt('INF'::float4);
> > ERROR: type "double precision" value out of range: overflow
>
> This works fine too:
>
> test=> select ('INF'::float8)::float4;
> float4
> ----------
> Infinity
> (1 row)
>
> > ==============================================
> > test=# select ((-32768::int8)::int2)%(-1::int2);
> > ?column?
> > ----------
> > 0
> > (1 row)
> >
> > test=# select ((-2147483648::int8)::int4)%(-1::int4);
> > ERROR: floating-point exception
> > DETAIL: An invalid floating-point operation was signaled. This probably
> > means an out-of-range result or an invalid operation, such
> > as division by zero.
>
> This was an interesting case. It turns out the value has to be INT_MIN,
> and the second value has to be -1. The exception happens, I think,
> because the CPU does the division first before getting the remainder,
> and INT_MIN / -1 is > INT_MAX, hence the error. I just special-cased it
> to return zero in the int4mod() code:
>
> test=> select ((-2147483648::int8)::int4)%(-1::int4);
> ?column?
> ----------
> 0
> (1 row)
>
> You can actually show the error without using int8:
>
> test=> select ((-2147483648)::int4) % (-1);
> ?column?
> ----------
> 0
> (1 row)
>
> The parentheses are required to make the value negative before the cast
> to int4.
>
> > ==============================================
> > test=# create table tt (ff float8);
> > CREATE TABLE
> > test=# insert into tt values (1e308),(1e308),(1e308);
> > INSERT 0 3
> > test=# select * from tt;
> > ff
> > --------
> > 1e+308
> > 1e+308
> > 1e+308
> > (3 rows)
> >
> > test=# select avg(ff) from tt;
> > avg
> > ----------
> > Infinity
> > (1 row)
> >
> > test=# select stddev(ff) from tt;
> > stddev
> > --------
> > NaN
> > (1 row)
>
> I didn't study the aggregate cases. Does someone want to look those
> over?
>
> The attached patch fixes all the items I mentioned above.

--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Attachment Content-Type Size
/pgpatches/float text/x-diff 26.2 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message dakotali kasap 2006-12-27 19:08:24 Re: how to add my source file?
Previous Message Andrew Dunstan 2006-12-27 18:06:27 Re: how to add my source file?

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2006-12-27 19:15:02 Re: [BUGS] BUG #2846: inconsistent and confusing handling of
Previous Message Bruce Momjian 2006-12-27 15:10:47 Re: Patch(es) to expose n_live_tuples and