MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:August 11 2009 6:22pm
Subject:Re: bzr commit into mysql-5.1-bugteam branch (davi:3036) Bug#45261
View as plain text  
Hi Martin,

On 8/11/09 7:00 AM, Martin Hansson wrote:
> Davi Arnaut wrote:
>> # At a local mysql-5.1-bugteam repository of davi
>>
>> 3036 Davi Arnaut 2009-07-23
>> Bug#45261: Crash, stored procedure + decimal
>> The problem was that creating a DECIMAL column from decimal
>> value could lead to a assertion as decimal values can have[...]
> ...from *a* decimal value...
> ...could lead to a failed assertion...
>> This patch borrows code and ideas from Martin Hansson's patch.
>> @ mysql-test/r/type_newdecimal.result
>> Add test case result for Bug#45261. Also, update test case to
>> reflect that a additive operation increases the precision of
> ...an additive operation...
>> @ sql/item.cc
>> The precision should only be capped when storing the value
>> on a table. Also, this makes it impossible to calculate the
>> integer part if Item::decimals (the scale) is larger then the
>> precision.
> ...larger than...

Will fix.

>> [...]
>> === modified file 'sql/item.cc'
>> --- a/sql/item.cc 2009-07-19 12:49:40 +0000
>> +++ b/sql/item.cc 2009-07-23 22:11:01 +0000
>> @@ -437,15 +437,13 @@ Item::Item(THD *thd, Item *item):
>>
>> uint Item::decimal_precision() const
>> {
>> +
>
> Please document the fact that this function does not cap, as opposed to
> Item::decimals. Preferrably, add cross-references between them.

OK.

>> [...]
>> +Field *Item::make_new_decimal_field() const
> This method is truly brilliant and will make the code much more stable
> by having it in one single place. But why is it in the Item class?
> Wouldn't it be more natural in Field_new_decimal? Afaiu it doesn't
> access any private data in Item.

I was following the current style (see make_string_field)...

> Like:
>
> static Field_new_decimal *Field::make_new_decimal_field(const Item *item)

But i like this suggestion. Will implement.

> Or even a new constructor Field_new_decimal(const Item *item)
>
> A third option is just a standalone static function, although it's not
> to my taste.
>>
>> +uint Item_func_get_user_var::decimal_precision() const
>> +{
>> + uint precision= max_length;
>> + Item_result restype= result_type();
>> +
>> + /* Default to maximum as the precision is unknown a priori. */
>> + if ((restype == DECIMAL_RESULT) || (restype == INT_RESULT))
>> + precision= DECIMAL_MAX_PRECISION;
>> +
>> + return precision;
>> +}
>> +
>> +
> What made you add this?
>

Item::decimal_precision calculates the precision from the maximum 
length, yet the maximum length in the case of a decimal type user 
variable is the maximum string length of a decimal (see method 
fix_length_and_dec, class Item_func_get_user_var). Hence, it is not 
correct (for the decimal type user variable case) to calculate the 
precision from the length. Furthermore, the precision and scale for user 
variables are set to the maximum in any case.

Regards,

-- Davi Arnaut
Thread
bzr commit into mysql-5.1-bugteam branch (davi:3036) Bug#45261Davi Arnaut24 Jul
  • Re: bzr commit into mysql-5.1-bugteam branch (davi:3036) Bug#45261Martin Hansson11 Aug
    • Re: bzr commit into mysql-5.1-bugteam branch (davi:3036) Bug#45261Davi Arnaut11 Aug
      • Re: bzr commit into mysql-5.1-bugteam branch (davi:3036) Bug#45261Martin Hansson12 Aug
        • Re: bzr commit into mysql-5.1-bugteam branch (davi:3036) Bug#45261Davi Arnaut12 Aug
  • Re: bzr commit into mysql-5.1-bugteam branch (davi:3036) Bug#45261Sergei Golubchik24 Aug