List:Falcon Storage Engine« Previous MessageNext Message »
From:Lars-Erik Bjørk Date:December 19 2008 1:10pm
Subject:Re: Fix for bug#41582
View as plain text  
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=1
>
>

Thread
Fix for bug#41582Lars-Erik Bjørk19 Dec
  • Re: Fix for bug#41582Hakan Kuecuekyilmaz19 Dec
    • Re: Fix for bug#41582Lars-Erik Bjørk19 Dec
  • RE: Fix for bug#41582Vladislav Vaintroub19 Dec
    • Re: Fix for bug#41582Lars-Erik Bjørk19 Dec
      • RE: Fix for bug#41582Vladislav Vaintroub19 Dec