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@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 security@mariadb.org