Hi, Alexander! On Jul 22, Alexander Barkov wrote:
On 06/20/2017 12:45 PM, Alexander Barkov wrote:
Hello Sergei,
Please review a patch for MDEV-13118.
Thanks!
commit dc7352fa2bc21fd9b66b12ec33cdacf88b478157 Author: Alexander Barkov <bar@mariadb.org> Date: Tue Jun 20 12:44:36 2017 +0400
MDEV-13118 Wrong results with LOWER and UPPER and subquery
This problem is similar to MDEV-10306.
1. Fixing Item_str_conv::val_str(String *str) to return the result in "str", and to use tmp_value only as a temporary buffer for args[0]->val_str(). The new code version now guarantees that the result is always returned in "str". The trick with copy_if_not_alloced() is not used any more.
2. Removing the optimization that *some* character sets used in casedn() and caseup(), which allowed (and required) to change the case in-place, overwriting the string passed as the "src" argument. Now all CHARSET_INFO's work in the same way: non of them change the source string in-place, all of them now convert case from the source string to the destination string, leaving the source string untouched.
I don't understand why it was needed. If the goal is to return the result in 'str' and never in 'tmp_value', it could've been achieved without doing 1. or 2., something like diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc index 89e55234482..f1837a4477b 100644 --- a/sql/item_strfunc.cc +++ b/sql/item_strfunc.cc @@ -1560,30 +1560,28 @@ String *Item_str_conv::val_str(String *str) { DBUG_ASSERT(fixed == 1); String *res; - if (!(res=args[0]->val_str(str))) - { - null_value=1; /* purecov: inspected */ - return 0; /* purecov: inspected */ - } - null_value=0; if (multiply == 1) { uint len; - res= copy_if_not_alloced(&tmp_value, res, res->length()); + if ((null_value= !(res= args[0]->val_str(str)))) + return 0; + res= copy_if_not_alloced(str, res, res->length()); len= converter(collation.collation, (char*) res->ptr(), res->length(), (char*) res->ptr(), res->length()); DBUG_ASSERT(len <= res->length()); res->length(len); } else { + if ((null_value= !(res= args[0]->val_str(&tmp_value)))) + return 0; uint len= res->length() * multiply; - tmp_value.alloc(len); - tmp_value.set_charset(collation.collation); + str->alloc(len); + str->set_charset(collation.collation); len= converter(collation.collation, (char*) res->ptr(), res->length(), - (char*) tmp_value.ptr(), len); - tmp_value.length(len); - res= &tmp_value; + (char*) str->ptr(), len); + str->length(len); + res= str; } return res; } Regards, Sergei Chief Architect MariaDB and security@mariadb.org