Hi Daogang,
Daogang Qu wrote:
> 2010-10-08 18:54, Alexander Barkov wrote:
>> 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).
> No problem.
>>
>>
>>>
>>>> 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?
>
> If './mtr rpl_do_table_filter --gdb' does not work, use --manual-gdb
> follow the following four steps:
mtr --gdb did not work for me.
I tried to add some debug output.
The relevant piece of "bzr diff rpl_filter.cc" now
looks like this in my tree (notice DBUG_PRINT commands):
@@ -104,22 +109,35 @@ Rpl_filter::tables_ok(const char* db, TA
end= strmov(hash_key, tables->db ? tables->db : db);
*end++= '.';
len= (uint) (strmov(end, tables->table_name) - hash_key);
- if (do_table_inited) // if there are any do's
+ DBUG_PRINT("info", ("tables_ok:checking '%.*s'", len, hash_key));
+ if (do_table_hash_inited) // if there are any do's
{
- if (my_hash_search(&do_table, (uchar*) hash_key, len))
+ if (my_hash_search(&do_table_hash, (uchar*) hash_key, len))
+ {
+ DBUG_PRINT("info", ("tables_ok:1:do_table found '%.*s'", len,
hash_key));
DBUG_RETURN(1);
+ }
}
- if (ignore_table_inited) // if there are any ignores
+ if (ignore_table_hash_inited) // if there are any ignores
{
- if (my_hash_search(&ignore_table, (uchar*) hash_key, len))
+ if (my_hash_search(&ignore_table_hash, (uchar*) hash_key, len))
+ {
+ DBUG_PRINT("info", ("tables_ok:0:ignore_table found '%.*s'",
len, hash_key));
DBUG_RETURN(0);
+ }
}
if (wild_do_table_inited &&
find_wild(&wild_do_table, hash_key, len))
+ {
+ DBUG_PRINT("info", ("tables_ok:1:wild_do_table found '%.*s'",
len, hash_key));
DBUG_RETURN(1);
+ }
if (wild_ignore_table_inited &&
find_wild(&wild_ignore_table, hash_key, len))
+ {
+ DBUG_PRINT("info", ("tables_ok:0:wild_ignore_table found '%.*s'",
len, hash_key));
DBUG_RETURN(0);
+ }
}
/*
@@ -128,8 +146,14 @@ Rpl_filter::tables_ok(const char* db, TA
If no explicit rule found and there was a do list, do not replicate.
If there was no do list, go ahead
*/
+ DBUG_PRINT("info", ("tables_ok:%d:return some_tables_updating=%d
do_table_array_inited=%d wild_do_table_inited=%d",
+ some_tables_updating && !do_table_array_inited &&
+ !wild_do_table_inited,
+ some_tables_updating,
+ do_table_array_inited,
+ wild_do_table_inited));
Now I run these commands:
./mtr --debug rpl_do_table_filter
grep "tables_ok:" var/log/mysqld.2.trace
and get this output from grep:
T@7 : | | | | | | info: tables_ok:checking 'mydb.t1'
T@7 : | | | | | | info: tables_ok:1:return some_tables_updating=1
do_table_array_inited=0 wild_do_table_inited=0
T@7 : | | | | | | info: tables_ok:checking 'mydb.t2'
T@7 : | | | | | | info: tables_ok:1:do_table found 'mydb.t2'
T@7 : | | | | | | info: tables_ok:checking 'mydb.t1'
T@7 : | | | | | | info: tables_ok:1:return some_tables_updating=1
do_table_array_inited=0 wild_do_table_inited=0
T@7 : | | | | | | info: tables_ok:checking 'mydb.t2'
T@7 : | | | | | | info: tables_ok:1:do_table found 'mydb.t2'
From which I conclude that tables_ok() never returns 1 because
of wrong collation behaviour.
Collation correctly finds 'mydb.t2' in do_table.
Collation correctly skips 'mydb.t1'.
But then it returns 1 for 'mydb.t1' anyway,
in the very end of the function, in this line:
DBUG_RETURN(some_tables_updating &&
!do_table_array_inited && !wild_do_table_inited);
Perhaps this is something related to this change in your patch:
DBUG_RETURN(some_tables_updating &&
- !do_table_inited && !wild_do_table_inited);
+ !do_table_array_inited && !wild_do_table_inited);
}
>
> 1.
> ./mtr rpl_do_table_filter --mysqld=--log-bin=/var/tmp/masterdg-bin
> --record --manual-gdb --testcase-timeout=1000
>
> 2. open and rewrite var/tmp/gdbinit.mysqld.2
> vi var/tmp/gdbinit.mysqld.2
>
> Disable the break point in the file as following:
> set args --defaults-group-suffix=.1
>
> --defaults-file=/home/daogangqu/mysql/bzrwork/bug34739/mysql-5.1-bugteam/mysql-test/var/my.cnf
>
> --log-output=file --gdb --loose-skip-innodb
> --log-bin=/var/tmp/masterdg-bin --binlog-format=statement --core-file
> --loose-debug-sync-timeout=300
> #break mysql_parse
> #commands 1
> #disable 1
> #end
> #run
>
> 3. Run the following command copied from the result of first step.
> like this:
> libtool --mode execute gdb -cd
> /home/daogangqu/mysql/bzrwork/bug34739/mysql-5.1-bugteam/mysql-test -x
>
> /home/daogangqu/mysql/bzrwork/bug34739/mysql-5.1-bugteam/mysql-test/var/tmp/gdbinit.mysqld.1
>
> /home/daogangqu/mysql/bzrwork/bug34739/mysql-5.1-bugteam/sql/mysqld
>
> (libtool --mode execute) is added extra.
>
>
> 4. Set breakpoint and run it in gdb
>
> Best Regards,
>
> Daogang
>>
>>>
>>> Best Regards,
>>>
>>> Daogang
>>>>
>>>>
>>>>
>>>>>
>>>>> Best Regards,
>>>>>
>>>>> Daogang
>>>>>
>>>>
>>>
>>
>>
>
