← Back to team overview

maria-developers team mailing list archive

Re: MDEV-8360 Clean-up CHARSET_INFO: strnncollsp: diff_if_only_endspace_difference

 

Hi, Alexander!

On Mar 25, Alexander Barkov wrote:
> Hi Sergei,
> 
> please review a patch for MDEV-8360.

Looks good!
Ok to push.

> Btw, shouldn't we remove:
> 
> #define HA_END_SPACE_ARE_EQUAL       512
> 
> in ./include/my_base.h ?
> 
> It's not really needed.

Yes, please do.

> commit 75ae61719e43dd0924b3a0cc5c9b588b82daeec8
> Author: Alexander Barkov <bar@xxxxxxxxxxx>
> Date:   Fri Mar 25 13:50:41 2016 +0400
> 
>     MDEV-8360 Clean-up CHARSET_INFO: strnncollsp: diff_if_only_endspace_difference
>     
>     - Removing the "diff_if_only_endspace_difference" argument from
>       MY_COLLATION_HANDLER::strnncollsp(), my_strnncollsp_simple(),
>       as well as in the function template MY_FUNCTION_NAME(strnncollsp)
>       in strcoll.ic
>     
>     - Removing the "diff_if_only_space_different" from ha_compare_text(),
>       hp_rec_key_cmp().
>     
>     - Adding a new function my_strnncollsp_padspace_bin() and reusing
>       it instead of duplicate code pieces in my_strnncollsp_8bit_bin(),
>       my_strnncollsp_latin1_de(), my_strnncollsp_tis620(),
>       my_strnncollsp_utf8_cs().
>     
>     - Adding more tests for better coverage of the trailing space handling.
> 
> diff --git a/strings/ctype-bin.c b/strings/ctype-bin.c
> index 1027255..8331de3 100644
> --- a/strings/ctype-bin.c
> +++ b/strings/ctype-bin.c
> @@ -182,31 +192,10 @@ static int my_strnncollsp_8bit_bin(CHARSET_INFO * cs __attribute__((unused)),
>      if (*a++ != *b++)
>        return ((int) a[-1] - (int) b[-1]);
>    }
> -  res= 0;
> -  if (a_length != b_length)
> -  {
> -    int swap= 1;
> -    /*
> -      Check the next not space character of the longer key. If it's < ' ',
> -      then it's smaller than the other key.
> -    */
> -    if (diff_if_only_endspace_difference)
> -      res= 1;                                   /* Assume 'a' is bigger */
> -    if (a_length < b_length)
> -    {
> -      /* put shorter key in s */
> -      a_length= b_length;
> -      a= b;
> -      swap= -1;					/* swap sign of result */
> -      res= -res;
> -    }
> -    for (end= a + a_length-length; a < end ; a++)
> -    {
> -      if (*a != ' ')
> -	return (*a < ' ') ? -swap : swap;
> -    }
> -  }
> -  return res;
> +  return a_length == b_length ? 0 :
> +         a_length < b_length  ?
> +           -my_strnncollsp_padspace_bin(b, b_length - length) :
> +           my_strnncollsp_padspace_bin(a, a_length - length);

That's much easier to understand. Thanks!

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx


References