From: Lars-Erik Bjørk Date: December 19 2008 1:10pm Subject: Re: Fix for bug#41582 List-Archive: http://lists.mysql.com/falcon/340 Message-Id: MIME-Version: 1.0 Content-Type: text/plain; delsp=yes; format=flowed; charset=US-ASCII Content-Transfer-Encoding: 7BIT On Dec 19, 2008, at 12:53 PM, Vladislav Vaintroub wrote: > > >> -----Original Message----- >> From: Lars-Erik.Bjork@stripped [mailto:Lars-Erik.Bjork@stripped] >> Sent: Friday, December 19, 2008 11:32 AM >> To: falcon@stripped >> Subject: Fix for bug#41582 >> >> Hi all! >> >> I have a fix for the decimal/numeric bug 41582. >> >> As explained in the reference manual [1], decimals are stored using a >> binary format that packs nine decimal digits into four bytes, where >> any leftover digits require some fraction of four bytes. >> >> In StorageDatabase::getSegmentValue (yes, this method again(!)) we do >> a check on the precision, to see if the value is big enough to need >> special treatment: >> >> else if (field->precision < 19 && field->scale == 0) >> { >> int64 number = (signed char) (*ptr++ ^ 0x80); >> >> for (int n = 1; n < length; ++n) >> number = (number << 8) | *ptr++; >> >> if (number < 0) >> ++number; >> >> value->setValue(number); >> } >> else >> { >> BigInt bigInt; >> ScaledBinary::getBigIntFromBinaryDecimal((const char*) >> ptr, field->precision, field->scale, &bigInt); >> value->setValue(&bigInt); >> } >> >> break; >> >> This test is obviously wrong., what we want to test for is: >> >> else if (field->precision <= 9 && field->scale == 0) >> >> This will fix the problem. >> >> Even better, we can avoid the test all together and treat all decimal >> keys as if they *may* have a precision greater than 9. This way there >> will be less code and we can avoid a cluttering 'if'. However, this >> will result in an additional function call or more (see the else >> clause above). >> What do you think? Am I allowed to remove the entire 'else if'? :) > > If think additional function call is not a problem and should be > preferred > for reasons of simplicity and avoiding cluttering. Provided > ScaledBinary::getBigIntFromBinaryDecimal works correctly, which > might be a > big "if". > > Given what happens further down the stack, it will probably be more like 5-6 extra function calls, or so. Testing and code reading suggests that ScaledBinary::getBigintFromBinaryDecimal works fine, and I don't want to work under the assumption that everything is broken by default :) /Lars-Erik >> Btw, when I am done with the fix for bug#40607, there will be some >> more rewrite in this area as well, but that is a different bug ... >> >> >> /Lars-Erik >> >> [1] - http://dev.mysql.com/doc/refman/6.0/en/precision-math- >> decimal- >> changes.html >> >> >> -- >> Falcon Storage Engine Mailing List >> For list archives: http://lists.mysql.com/falcon >> To unsubscribe: http://lists.mysql.com/falcon?unsub=wlad@stripped > >