From: Alexander Barkov
Date: October 8 2010 10:54am
Subject: Re: Please review the updated patch for bug51639
List-Archive: http://lists.mysql.com/commits/120358
Message-Id: <4CAEF874.9010404@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Hi Daogang,

Daogang Qu wrote:
> Hi Bar,
> Thanks for the review. See reply in-line.
> Please review:
> http://lists.mysql.com/commits/120340
> 
> 2010-09-30 20:49, Alexander Barkov wrote:
>> Hi Daogang,
>>
>>
>> Daogang Qu wrote:
>>> Hi Bar,
>>> Please review the updated patch:
>>> http://lists.mysql.com/commits/118275
>>
>> my_strcasecmp() in these two chunks is not correct.
>>
>> @@ -167,8 +172,13 @@ Rpl_filter::db_ok(const char* db)
>>
>>      while ((tmp=it++))
>>      {
>> -      if (!strcmp(tmp->ptr, db))
>> -    DBUG_RETURN(1); // match
>> +      /*
>> +        Filters will follow the setting of lower_case_table_name
>> +        to be case sensitive when setting lower_case_table_name=0.
>> +        Otherwise they will be case insensitive.
>> +      */
>> +      if (!my_strcasecmp(table_alias_charset, tmp->ptr, db))
>> +        DBUG_RETURN(1); // match
>>      }
>>      DBUG_RETURN(0);
>>    }
>> @@ -179,8 +189,13 @@ Rpl_filter::db_ok(const char* db)
>>
>>      while ((tmp=it++))
>>      {
>> -      if (!strcmp(tmp->ptr, db))
>> -    DBUG_RETURN(0); // match
>> +      /*
>> +        Filters will follow the setting of lower_case_table_name
>> +        to be case sensitive when setting lower_case_table_name=0.
>> +        Otherwise they will be case insensitive.
>> +      */
>> +      if (!my_strcasecmp(table_alias_charset, tmp->ptr, db))
>> +        DBUG_RETURN(0); // match
>>      }
>>      DBUG_RETURN(1);
>>    }
>>
>> This comparison will always be case insensitive,
> No. It will be case sensitive when setting lower_case_table_name=0.
> The 'rpl_do_db_filter.test' will verify the point. Please check.

I am sorry for confusion. You're right:
- In case of lower_case_table_names=0 it will
call my_strcasecmp_bin() which then calls strcmp(),
which is case sensitive.
- In case of lower_case_table_name=1 it will
call my_strcasecmp_utf8() which is case insensitive.
Everything is correct here.

(last time I checked my_strcasecmp(&my_charset_utf8_bin,a,b)
instead of my_strcasecmp(&my_charset_bin,a,b)
in a mistake).


> 
>> even if lower_case_table_name=0. This is not what we need.
>> You need to use my_strnncoll() instead.
>>
>>
>> Please also add a test covering this.
>> You need .opt file with these settings:
>>
>> --replicate-do-db=MYDB --lower-case-table-names=0
> Added in 'rpl_do_db_filter.test'
>>
>> This test will check that nothing is replicated,
>> because database name is "mydb" which is not equal to MYDB
>> when lower_case_table_name=0.
>>
>> Currently this combination is not covered.
>>
>>>
>>> Could you please add a test for testing new collation
>>> for supporting the case insensitive but accent sensitive?
>>
>> It will be very similar to how you test case sensitivity.
>> Just use some accented letters in db or table names in
>> SQL queries. For example,
>>
>> ý  in database name: mýdb
>> ţ  in table name: ţ1
>>
>> Use non-accented letters in .opt file.
>>
>> This is to make sure that filters recognize
>> that "mydb" and "mýdb" are different database names,
>> and that "t1" and "ţ1" are different table names.
>>
>> no matter what lower-case-table-name is.
> I tested the non-accented letters in rpl_do_table_filter-slave.opt file.
> But it didn't work as expected. The filters still will be case insensitive
> but non-accent sensitive when setting lower_case_table_name>0.
> Please double check the definition of 'my_charset_utf8_tolower_ci'.
> Thanks!

The problem is likely that
my_strcasecmp_utf8('mydb', 'mýdb')
my_strcasecmpsp_utf8('mydb', 'mýdb')

return TRUE for my_charset_utf8_tolower_ci for some reasons,
for the above strings, or get wrong parameters.
The idea is to set break-points in these functions and check.

I'll compile your patch later, and write a separate letter.

Is there a way to run slave under "gdb" in mysql-test?

> 
> Best Regards,
> 
> Daogang
>>
>>
>>
>>>
>>> Best Regards,
>>>
>>> Daogang
>>>
>>
>