List:Falcon Storage Engine« Previous MessageNext Message »
From:Lars-Erik Bjørk Date:December 19 2008 10:47am
Subject:Re: Fix for bug#41582
View as plain text  
Hakan,

thanks for the reply! :)

Both of the tests you mention are in the falcon suite, which runs  
successfully. So my fix won't introduce a regression there.

/Lars-Erik

On Dec 19, 2008, at 11:41 AM, Hakan Kuecuekyilmaz wrote:

> On Fr, 2008-12-19 at 11:32 +0100, Lars-Erik Bjørk wrote:
>> 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'?   :)
>>
>
> I recall that we had some problems with decimal > 19. Like
>
> http://bugs.mysql.com/bug.php?id=28044
> http://bugs.mysql.com/bug.php?id=26468
>
> Please check those tests, whether your fix introduces a regression or
> not.
>
>> 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
>>
>>
>
> Best regards,
>
> Hakan
>
> -- 
> Hakan Küçükyılmaz, Senior Software Engineer DBTG/MySQL +49 160  
> 98953296
> Sun Microsystems GmbH     Sonnenallee 1, DE-85551 Kirchheim- 
> Heimstetten
> Geschaeftsfuehrer:  Thomas Schroeder, Wolfang Engels, Dr. Roland  
> Boemer
> Vorsitz d. Aufs.rat.: Martin Haering   HRB MUC 161028     49.011,  
> 8.376
>

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