Hi Sergey,
Sergey Glukhov skrev 2010-12-23 12.44:
> Hi Martin, see comments below
>
> On 12/22/2010 03:30 PM, Martin Hansson wrote:
>> #At file:///data0/martin/bzrroot/bug58165/5.1bt-lazy_copy/ based on
>> revid:sergey.glukhov@stripped
>>
>>
>> modified:
>> mysql-test/r/func_str.result
>> mysql-test/t/func_str.test
>> sql/item_strfunc.cc
>> sql/sql_string.cc
> ...
>> === modified file 'sql/item_strfunc.cc'
>> --- a/sql/item_strfunc.cc 2010-11-11 10:25:23 +0000
>> +++ b/sql/item_strfunc.cc 2010-12-22 12:30:13 +0000
>> @@ -1305,8 +1305,15 @@ String *Item_func_substr_index::val_str(
>> null_value=0;
>> uint delimiter_length= delimiter->length();
>> if (!res->length() || !delimiter_length || !count)
>> - return&my_empty_string; // Wrong parameters
>> -
>> + {
>> + if (str->copy("", 0, default_charset_info)) // Wrong
>> parameters
>> + {
>> + /* Out of memory */
>> + null_value= true;
>> + return NULL;
>> + }
>> + return str;
>> + }
>> res->set_charset(collation.collation);
>>
>> #ifdef USE_MB
>>
>
> Using my_empty_string is not safe, the bug shows it.
> So we should remove my_empty_string from all places
> where it is used.
Removing my_empty_string altogether is way too large to be done as an
MRU bug fix (Believe me, I tried.) I suggest filing a separate report
about it. Let's make this customer happy first and then we'll deal with
the other problems we discovered while working on it.
> I discussed the problem with Bar,
> he suggested the following fix:
>
> === modified file 'sql/item_strfunc.cc'
> --- old/sql/item_strfunc.cc 2010-11-11 10:25:23 +0000
> +++ new/sql/item_strfunc.cc 2010-12-23 11:29:58 +0000
> @@ -1305,7 +1305,10 @@
> null_value=0;
> uint delimiter_length= delimiter->length();
> if (!res->length() || !delimiter_length || !count)
> - return &my_empty_string; // Wrong parameters
> + {
> + str_value.set("", 0, collation.collation); // Also safe in case
> of OOM error as no allocations happen
> + return &str_value;
> + }
Much better solution, I admit. Done.
>
> PS I assigned Bar as second reviewer, hope you don't mind...
Not at all, probably a wise move in fact. :-)
/Martin
