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 <b...@mariadb.org> > 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 secur...@mariadb.org _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp