Hi Sergei, On 01/27/2014 05:52 PM, Sergei Golubchik wrote:
Hi, Alexander!
On Jan 27, Alexander Barkov wrote:
On 01/26/2014 11:23 PM, Sergei Golubchik wrote:
Okay, I understand what you did. But I don't understand why. What was the problem? Why there was a crash?
- Item_func_make_set::val_str() returns &my_empty_string ... After the fix my_empty_string: - remembers that it is null terminated and does not call realloc() from c_ptr_safe() - remembers that it is read-only and protects itself from realloc() by DBUG_ASSERT().
I'd rather fix Item_func_make_set instead. Now it seems to be the only Item that uses it (there's also SELECT ... OUTFILE, but that's different).
And Item_func_make_set has String tmp_str property, which is already used as an internal storage.
So, I'd remove my_empty_string usage from Item_func_make_set whatsoever. Simply with
- String *result=&my_empty_string; + String *result=&tmp_str; + tmp_str.length(0);
In fact, perhaps Item_func_make_set::tmp_str is not needed at all, and Item:str_value can be used instead:
- String *result=&my_empty_string; + String *result=make_empty_result();
These types of bug are annoying. We've fixed a lot of them. I hoped to extend the new code gradually for non-static strings as well: - to fix String::c_ptr*() not to use realloc if constructor knows that the value is null terminated. - protect use of c_ptr*() is unsafe contexts. and also put String::thread_specific into String::flags when merging to 10.0. But this all can be done in 10.0 only. No need to do it in 5.3. I will fix 5.3 simply to use make_empty_result().
Regards, Sergei