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
>>>
>>
>
