Re: [HACKERS] BUG #5237: strange int-bit and bit-int conversions
The bitfromint8() and bitfromint4() are hosed. They produce wrong
results when the BIT size is more than 64 and 32 respectively, and the
BIT size is not multiple of 8, and the most significant byte of the
integer value is not 0x00 or 0xff.
For example:
test=# select (11::int423 | 11::int4)::bit(32);
010110001011
test=# select (11::int423 | 11::int4)::bit(33);
0101110001011
test=# select (11::int423 | 11::int4)::bit(39);
010110110001011
test=# select (11::int423 | 11::int4)::bit(40);
010110001011
The ::bit(33) and ::bit(39) conversions are wrong.
The patch re-implements the conversion functions.
diff -urp -x '*.o' -x '*.txt' -x '*.so' postgresql-8.4.1-orig/src/backend/utils/adt/varbit.c postgresql-8.4.1/src/backend/utils/adt/varbit.c
--- postgresql-8.4.1-orig/src/backend/utils/adt/varbit.c 2009-12-12 09:19:13.0 -0600
+++ postgresql-8.4.1/src/backend/utils/adt/varbit.c 2009-12-12 10:29:59.0 -0600
@@ -1321,8 +1321,8 @@ bitfromint4(PG_FUNCTION_ARGS)
VarBit *result;
bits8 *r;
int rlen;
- int destbitsleft,
-srcbitsleft;
+ int const srcbits=sizeof(a)*BITS_PER_BYTE;
+ int i;
if (typmod = 0)
typmod = 1;/* default bit length */
@@ -1333,32 +1333,21 @@ bitfromint4(PG_FUNCTION_ARGS)
VARBITLEN(result) = typmod;
r = VARBITS(result);
- destbitsleft = typmod;
- srcbitsleft = 32;
- /* drop any input bits that don't fit */
- srcbitsleft = Min(srcbitsleft, destbitsleft);
- /* sign-fill any excess bytes in output */
- while (destbitsleft = srcbitsleft + 8)
- {
- *r++ = (bits8) ((a 0) ? BITMASK : 0);
- destbitsleft -= 8;
- }
- /* store first fractional byte */
- if (destbitsleft srcbitsleft)
- {
- *r++ = (bits8) ((a (srcbitsleft - 8)) BITMASK);
- destbitsleft -= 8;
- }
- /* Now srcbitsleft and destbitsleft are the same, need not track both */
- /* store whole bytes */
- while (destbitsleft = 8)
- {
- *r++ = (bits8) ((a (destbitsleft - 8)) BITMASK);
- destbitsleft -= 8;
+ rlen=(typmod+BITS_PER_BYTE-1)/BITS_PER_BYTE;
+
+ if (srcbits=typmod) {
+ a=srcbits-typmod;
+for (i=0; i!=rlen; ++i, a=BITS_PER_BYTE) r[i]=a(srcbits-BITS_PER_BYTE);
+ } else {
+ int sh=typmod%BITS_PER_BYTE;
+ int32 h=ash;
+ int32 l=a(srcbits-sh);
+ size_t const zsize=rlen-sizeof(a)-(sh!=0);
+ bits8 sx=(a=0)-1;
+ memset(r,sx,zsize);
+for (i=0; i!=sizeof(a); ++i, h=8) r[zsize+i]=h(srcbits-BITS_PER_BYTE);
+ if (sh!=0) r[zsize+sizeof(a)]=l(srcbits-BITS_PER_BYTE);
}
- /* store last fractional byte */
- if (destbitsleft 0)
- *r = (bits8) ((a (8 - destbitsleft)) BITMASK);
PG_RETURN_VARBIT_P(result);
}
@@ -1396,8 +1385,8 @@ bitfromint8(PG_FUNCTION_ARGS)
VarBit *result;
bits8 *r;
int rlen;
- int destbitsleft,
-srcbitsleft;
+ int const srcbits=sizeof(a)*BITS_PER_BYTE;
+ int i;
if (typmod = 0)
typmod = 1;/* default bit length */
@@ -1408,36 +1397,21 @@ bitfromint8(PG_FUNCTION_ARGS)
VARBITLEN(result) = typmod;
r = VARBITS(result);
- destbitsleft = typmod;
-#ifndef INT64_IS_BUSTED
- srcbitsleft = 64;
-#else
- srcbitsleft = 32; /* don't try to shift more than 32 */
-#endif
- /* drop any input bits that don't fit */
- srcbitsleft = Min(srcbitsleft, destbitsleft);
- /* sign-fill any excess bytes in output */
- while (destbitsleft = srcbitsleft + 8)
- {
- *r++ = (bits8) ((a 0) ? BITMASK : 0);
- destbitsleft -= 8;
- }
- /* store first fractional byte */
- if (destbitsleft srcbitsleft)
- {
- *r++ = (bits8) ((a (srcbitsleft - 8)) BITMASK);
- destbitsleft -= 8;
- }
- /* Now srcbitsleft and destbitsleft are the same, need not track both */
- /* store whole bytes */
- while (destbitsleft = 8)
- {
- *r++ = (bits8) ((a (destbitsleft - 8)) BITMASK);
- destbitsleft -= 8;
+ rlen=(typmod+BITS_PER_BYTE-1)/BITS_PER_BYTE;
+
+ if (srcbits=typmod) {
+ a=srcbits-typmod;
+for (i=0; i!=rlen; ++i, a=BITS_PER_BYTE) r[i]=a(srcbits-BITS_PER_BYTE);
+ } else {
+ int sh=typmod%BITS_PER_BYTE;
+ int64 h=ash;
+ int64 l=a(srcbits-sh);
+ size_t const zsize=rlen-sizeof(a)-(sh!=0);
+ bits8 sx=(a=0)-1;
+ memset(r,sx,zsize);
+for (i=0; i!=sizeof(a); ++i, h=8) r[zsize+i]=h(srcbits-BITS_PER_BYTE);
+ if (sh!=0) r[zsize+sizeof(a)]=l(srcbits-BITS_PER_BYTE);
}
- /* store last fractional byte */
- if (destbitsleft 0)
- *r = (bits8) ((a (8 - destbitsleft)) BITMASK);
PG_RETURN_VARBIT_P(result);
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] [BUGS] BUG #2846: inconsistent and confusing handling
On 12/27/2006 01:15 PM, Tom Lane wrote: I'm not convinced that you're fixing things so much as doing your best to destroy IEEE-compliant float arithmetic behavior. I think what we should probably consider is removing CheckFloat4Val and CheckFloat8Val altogether, and just letting the float arithmetic have its head. Most modern hardware gets float arithmetic right per spec, and we shouldn't be second-guessing it. I vote for complete IEEE-compliance. No exceptions with pure floating point math. Float - int conversions should reject overflow, INF and NaN. Float - numeric conversions should reject INF. A slightly less radical proposal is to reject only the case where isinf(result) and neither input isinf(); and perhaps likewise with respect to NaNs. This might look like another possibility for confusion. For example INF-INF=NaN. Regards, Roman. ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] [BUGS] BUG #2846: inconsistent and confusing
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/adtdiff -U3 -p float.orig.c float.c
--- float.orig.c2006-12-29 10:49:51.0 -0600
+++ float.c 2006-12-29 10:58:19.0 -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);
}
/*
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] [BUGS] BUG #2846: inconsistent and confusing
On 12/29/2006 11:27 AM, Tom Lane wrote: Doesn't even compile here (no fenv.h). Where do you compile? Roman ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] [BUGS] BUG #2846: inconsistent and confusing handling
On 12/27/2006 05:19 PM, Tom Lane wrote: Roman Kononov [EMAIL PROTECTED] writes: On 12/27/2006 03:23 PM, Bruce Momjian wrote: Are you sure? As I remember, computation automatically upgrades to 'double'. See this program and output: This is platform- and compiler- dependent: ... and probably irrelevant, too. We should store the result into a float4 variable and then test for isinf() on that; that eliminates the question of whether the compiler did the multiply in a wider format or not. You are right provided that you want to ignore underflows and silently produce zeros instead. If you go this way, I recommend to ignore overflows as well, and silently produce infinities and NaNs. Roman ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
