maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Alexander!

On Nov 02, Alexander Barkov wrote:
> >> commit dc7352fa2bc21fd9b66b12ec33cdacf88b478157
> >> Author: Alexander Barkov <bar@xxxxxxxxxxx>
> >> 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",
> >>     2. Removing the optimization that *some* character sets used in casedn()
> >>        and caseup(), which allowed (and required) to change the case in-place,
> > 
> > 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
> 
> There was a symmetry in the charset library in caseup() and casedn()
> implementations for various charsets:
> That was done specifically for Item_str_conv.
> Now with Item_str_conv returning the value in "str" anyway:
> 
> - This optimization is not needed any more.
>   There is no sense to do case conversion inplace and
>   then immediately copy the modified string to another String
>   (or the other way around: copy to a new String and then
>    convert inplace).
> 
> - It's good to make the charset library symmetric for all charsets

I see.

Could you split this commit in two, then? Should be as easy as using
'git citool' to move all strings/* and include/* changes into a separate
changeset with the comment like

  Simplify caseup() and casedn() in charsets

  After the MDEV-13118 fix there's no code in the server that
  wants caseup/casedn to change the argument in place for simple
  charsets.  Let's remove this logic and always return the result in a
  new string for all charsets, both simple and complex.

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx