Thread: Re: [PATCHES] [BUGS] BUG #2846: inconsistent and confusing
Bruce Momjian <bruce@momjian.us> writes:
> I wasn't excited about doing one isinf() call to avoid three, so I just
> made a fast isinf() macro:
> /* We call isinf() a lot, so we use a fast version in this file */
> #define fast_isinf(val) (((val) < DBL_MIN || (val) > DBL_MAX) && isinf(val))
This is *not* going in the right direction :-(
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > I wasn't excited about doing one isinf() call to avoid three, so I just > > made a fast isinf() macro: > > > /* We call isinf() a lot, so we use a fast version in this file */ > > #define fast_isinf(val) (((val) < DBL_MIN || (val) > DBL_MAX) && isinf(val)) > > This is *not* going in the right direction :-( Well, then show me what direction you think is better. -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> This is *not* going in the right direction :-(
> Well, then show me what direction you think is better.
Fewer restrictions, not more. The thrust of what I've been saying
(and I think Roman too) is to trust in the hardware float-arithmetic
implementation to be right. Every time you add an additional "error
check" you are going in the wrong direction.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Tom Lane wrote: > >> This is *not* going in the right direction :-( > > > Well, then show me what direction you think is better. > > Fewer restrictions, not more. The thrust of what I've been saying > (and I think Roman too) is to trust in the hardware float-arithmetic > implementation to be right. Every time you add an additional "error > check" you are going in the wrong direction. OK, are you saying that there is a signal we are ignoring for overflow/underflow, or that we should just silently overflow/underflow and not throw an error? -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> OK, are you saying that there is a signal we are ignoring for
> overflow/underflow, or that we should just silently overflow/underflow
> and not throw an error?
Silent underflow is fine with me; it's the norm in most all float
implementations and won't surprise anyone. For overflow I'm OK with
either returning infinity or throwing an error --- but if an error,
it should only be about inf-out-with-non-inf-in, not comparisons to any
artificial MAX/MIN values.
Anyone else have an opinion about this?
regards, tom lane
Bruce Momjian wrote:<br /><blockquote cite="mid200612291512.kBTFCVY02214@momjian.us" type="cite"><pre wrap="">Tom Lane wrote:</pre><blockquote type="cite"><pre wrap="">Bruce Momjian <a class="moz-txt-link-rfc2396E" href="mailto:bruce@momjian.us"><bruce@momjian.us></a>writes: </pre><blockquote type="cite"><pre wrap="">Tom Lanewrote: </pre><blockquote type="cite"><pre wrap="">This is *not* going in the right direction :-( </pre></blockquote></blockquote><blockquotetype="cite"><pre wrap="">Well, then show me what direction you think is better. </pre></blockquote><pre wrap="">Fewer restrictions, not more. The thrust of what I've been saying (and I think Roman too) is to trust in the hardware float-arithmetic implementation to be right. Every time you add an additional "error check" you are going in the wrong direction. </pre></blockquote><pre wrap=""> OK, are you saying that there is a signal we are ignoring for overflow/underflow, or that we should just silently overflow/underflow and not throw an error? </pre></blockquote> My understanding is that you have to actually set flags in the floating point environment to make overflows,underflows, infinities, NaNs, etc. raise signals. You might take a look at fenv.h (defined in the C99 spec) forthe functions to do this. My apologies if I'm recovering well trod ground here.<br /><br /> Note that taking a signalon an FP exception is a horribly expensive proposition- we're talking about hundreds or thousands of clock cycles here. But it's probably worthwhile vr.s the cost of testing every floating point result, as generally FP exceptions willbe rare (probably even more rare in database work than in general). So it's probably worthwhile.<br /><br /> Brian<br/><br />
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: >> OK, are you saying that there is a signal we are ignoring for >> overflow/underflow, or that we should just silently overflow/underflow >> and not throw an error? > > Silent underflow is fine with me; it's the norm in most all float > implementations and won't surprise anyone. For overflow I'm OK with > either returning infinity or throwing an error --- but if an error, > it should only be about inf-out-with-non-inf-in, not comparisons to any > artificial MAX/MIN values. > > Anyone else have an opinion about this? If an underflow is not reported (And thus silently treated as zero), then it'd make sense for me to deal with overflows in a similar way, and just return infinity. The most correct solution would IMHO be to provide a guc variable "strict_float_semantics" that defaults to "off", meaning that neather overflow nor underflow reports an error. If the variable was set to on, _both_ overflow and underflow would be reported. Just my €0.02 greetings, Florian Pflug
Brian Hurt <bhurt@janestcapital.com> writes:
> Note that taking a signal on an FP exception is a horribly expensive
> proposition- we're talking about hundreds or thousands of clock cycles
> here. But it's probably worthwhile vr.s the cost of testing every
> floating point result, as generally FP exceptions will be rare (probably
> even more rare in database work than in general). So it's probably
> worthwhile.
I think we should probably stay away from relying on signals for this
on portability grounds. The cost of checking the results is small, and
will get smaller if we eliminate or simplify CheckFloat[48]Val as is
being discussed here.
regards, tom lane
On 12/29/2006 12:23 AM, Bruce Momjian wrote:
> Well, then show me what direction you think is better.
Think about this idea please. This has no INF, NaN or range
checks and detects all "bad" cases with any floating point
math.
The only issue is that a bad case is detected only once.
You need to restart the postmaster. It can be fixed by
re-enabling FP exceptions in the FP exception handler.
Roman
-----------------------------
~/postgresql-8.2.0/src/backend/utils/adt>diff -U3 -p float.orig.c float.c
--- float.orig.c 2006-12-29 10:49:51.000000000 -0600
+++ float.c 2006-12-29 10:58:19.000000000 -0600
@@ -60,12 +60,21 @@
#ifdef HAVE_IEEEFP_H
#include <ieeefp.h>
#endif
+#include <fenv.h>
#include "catalog/pg_type.h"
#include "libpq/pqformat.h"
#include "utils/array.h"
#include "utils/builtins.h"
+static void __attribute__((__constructor__))
+enable_fp_exceptions()
+{
+ feclearexcept(FE_ALL_EXCEPT);
+ feenableexcept(FE_DIVBYZERO | FE_UNDERFLOW | FE_OVERFLOW | FE_INVALID);
+ printf("FP exceptions enabled\n");
+}
+
#ifndef M_PI
/* from my RH5.2 gcc math.h file - thomas 2000-04-03 */
@@ -783,11 +792,10 @@ float4pl(PG_FUNCTION_ARGS)
{
float4 arg1 = PG_GETARG_FLOAT4(0);
float4 arg2 = PG_GETARG_FLOAT4(1);
- double result;
+ float4 result;
result = arg1 + arg2;
- CheckFloat4Val(result);
- PG_RETURN_FLOAT4((float4) result);
+ PG_RETURN_FLOAT4(result);
}
Datum
@@ -795,11 +803,10 @@ float4mi(PG_FUNCTION_ARGS)
{
float4 arg1 = PG_GETARG_FLOAT4(0);
float4 arg2 = PG_GETARG_FLOAT4(1);
- double result;
+ float4 result;
result = arg1 - arg2;
- CheckFloat4Val(result);
- PG_RETURN_FLOAT4((float4) result);
+ PG_RETURN_FLOAT4(result);
}
Datum
@@ -807,11 +814,10 @@ float4mul(PG_FUNCTION_ARGS)
{
float4 arg1 = PG_GETARG_FLOAT4(0);
float4 arg2 = PG_GETARG_FLOAT4(1);
- double result;
+ float4 result;
result = arg1 * arg2;
- CheckFloat4Val(result);
- PG_RETURN_FLOAT4((float4) result);
+ PG_RETURN_FLOAT4(result);
}
Datum
@@ -819,18 +825,10 @@ float4div(PG_FUNCTION_ARGS)
{
float4 arg1 = PG_GETARG_FLOAT4(0);
float4 arg2 = PG_GETARG_FLOAT4(1);
- double result;
-
- if (arg2 == 0.0)
- ereport(ERROR,
- (errcode(ERRCODE_DIVISION_BY_ZERO),
- errmsg("division by zero")));
-
- /* Do division in float8, then check for overflow */
- result = (float8) arg1 / (float8) arg2;
+ float4 result;
- CheckFloat4Val(result);
- PG_RETURN_FLOAT4((float4) result);
+ result = arg1 / arg2;
+ PG_RETURN_FLOAT4(result);
}
/*
Roman Kononov <kononov195-pgsql@yahoo.com> writes:
> Think about this idea please. This has no INF, NaN or range
> checks and detects all "bad" cases with any floating point
> math.
Doesn't even compile here (no <fenv.h>).
regards, tom lane
On 12/29/2006 11:27 AM, Tom Lane wrote: > Doesn't even compile here (no <fenv.h>). Where do you compile? Roman