List:Commits« Previous MessageNext Message »
From:Alexander Barkov Date:July 9 2009 5:00am
Subject:Re: #37656 [Pnd]: lower_case_table_names=1 doesn't convert database
names in replicated statements
View as plain text  
Hi Luís,

Luís Soares wrote:
> Hi Bar,
> 
> 
> On Wed, 2009-07-08 at 18:52 +0500, Alexander Barkov wrote:
>> Hi Luis,
>>
>>
>>> MySQL
>>>
>>> Logout / Profile
>>>
>>>     * Developer Zone
>>>     * Lists Home
>>>     * FAQ
>>>
>>>
>>> List:	Commits	« Previous MessageNext Message »
>>> From:	Luis Soares 	Date:	July 3 2009 12:24pm
>>> Subject:	bzr commit into mysql-5.4 branch (luis.soares:2804) Bug#37656
>>> View as plain text  
>>>
>>> #At
> file:///home/lsoares/Workspace/mysql-server/bugfix/b37656/mysql-azalea-bugfixing/
>>> based on revid:alik@stripped
>>>
>>>  2804 Luis Soares	2009-07-03
>>>       BUG#37656: lower_case_table_names=1 doesn't convert database
>>>                  names in replicated statements
>>>       
>>>       This bug revealed itself while using case sensitive filesystems
>>>       and exhibited two symptoms:
>>>       
>>>         1. If setting lower_case_table_names=1 on the slave, but not on
>>>            the master, this setting will not convert database name in
>>>            replicated statements, ultimately breaking replication;
>>>       
>>>         2. The same problem for symptom 1. surfaced in RBR, but this
>>>            time for table names, as these would not be converted to
>>>            lower case for row based replication events.
>>>       
>>>       Symptom 1. is addressed by conditionally converting to lower case,
>>>       database name on Query_log_event constructor and
>>>       Load_log_event::do_apply_event.
>>>       
>>>       Symptom 2. is addressed by conditionally converting to lower case
>>>       database name and table name when processing Table_map_log_event.
>>>       
>>>       On top of these two fixes, this patch also provides functionality
>>>       to automatically turn into down case user defined replication
>>>       filtering rules. For example, if lower_case_table_names=1 is used
>>>       simultaneously with any replication filtering rule, say
>>>       --replicate-do-db=TEST, then all rules are automatically and
>>>       implicitly translated to lower case, in this example it would
>>>       turn do-db rule into --replicate-do-db=test. This was
>>>       accomplished by extending the Rpl_filter class with a
>>>       to_lower_case public method that gets called on mysqld startup if
>>>       lower_case_table_names=1 is set.
>>>      @ sql/log_event.cc
>>>         Changed Query_log_event constructor, Load_log_event::do_apply_event
>>>         and Table_map_log_event::do_apply_event to automatically turn
> database
>>>         and table names to down case.
>>>      @ sql/mysqld.cc
>>>         Added check to mysqld option initialization so that whenever 
>>>         lower_case_table_names==1 filtering rules are turned into lower
>>>         case.
>>>      @ sql/rpl_filter.cc
>>>         Extended Rpl_filter class by adding a to_lower_case public method
>>>         that turns all internal filter rules into lower case. Used in
>>>         mysqld.cc
>>>      @ sql/rpl_filter.h
>>>         Added to_lower_case(CHARSET_INFO * cs_info); to Rpl_filter
>>>         interface.
>>>
>>>     added:
>>>       mysql-test/extra/rpl_tests/rpl_lower_case_table_names.test
>>>       mysql-test/suite/rpl/r/rpl_lower_case_table_names_do_db.result
>>>       mysql-test/suite/rpl/r/rpl_lower_case_table_names_do_table.result
>>>       mysql-test/suite/rpl/r/rpl_lower_case_table_names_rewrite_db.result
>>>       mysql-test/suite/rpl/t/rpl_lower_case_table_names_do_db-slave.opt
>>>       mysql-test/suite/rpl/t/rpl_lower_case_table_names_do_db.test
>>>       mysql-test/suite/rpl/t/rpl_lower_case_table_names_do_table-slave.opt
>>>       mysql-test/suite/rpl/t/rpl_lower_case_table_names_do_table.test
>>>       mysql-test/suite/rpl/t/rpl_lower_case_table_names_rewrite_db-slave.opt
>>>       mysql-test/suite/rpl/t/rpl_lower_case_table_names_rewrite_db.test
>>>     modified:
>>>       sql/log_event.cc
>>>       sql/mysqld.cc
>>>       sql/rpl_filter.cc
>>>       sql/rpl_filter.h
>>> === added file 'mysql-test/extra/rpl_tests/rpl_lower_case_table_names.test'
>>> --- a/mysql-test/extra/rpl_tests/rpl_lower_case_table_names.test	1970-01-01
> 00:00:00 +0000
>>> +++ b/mysql-test/extra/rpl_tests/rpl_lower_case_table_names.test	2009-07-03
> 10:23:57 +0000
>>> @@ -0,0 +1,79 @@
>>> +# BUG#37656
>>> +#
>>> +#  This test aims at checking whether lower_case_table_names=1 option works
>>> +#  for database names and table names (on row mode logging).
>>> +#
>>> +#  This test checks the following (when lower_case_table_names=1 is set on
> slave):
>>> +#    (i) creating a database on upper case on master results in lower case
>>> +#        database name on slave
>>> +#   (ii) creating tables with upper case names on master results in lower
> case
>>> +#        table names on slave
>>> +#  (iii) loading data infile into capitalized table name on master
> replicates to
>>> +#        lower case table name on slave
>>> +#   (iv) Propagating changes from upper case table names on into
> correspondent 
>>> +#        lower case table names on slave works.
>>> +
>>> +
>>> +# setup: create database and tables
>>> +-- echo ******** [ MASTER ] ********
>>> +-- let $dbname_upper= BUG_37656
>>> +-- let $dbname_lower= `SELECT LOWER('$dbname_upper')`
>>> +-- eval CREATE DATABASE $dbname_upper
>>> +-- eval use $dbname_upper
>>> +
>>> +# assert: database names are in upper case in master and lower
>>> +#         case in slave
>>> +-- eval show databases like '$dbname_upper'
>>> +sync_slave_with_master;
>>> +-- echo ******** [ SLAVE ] ********
>>> +--eval show databases like '$dbname_lower'
>>> +
>>> +-- connection master
>>> +-- echo ******** [ MASTER ] ********
>>> +CREATE TABLE T1 (a int);
>>> +-- eval CREATE TABLE T2 (b int) ENGINE=$engine
>>> +CREATE TABLE T3 (txt TEXT);
>>> +
>>> +# assert: that tables exist on master with upper case names
>>> +show tables;
>>> +
>>> +# assert: that tables exist on slave but with lower case names
>>> +-- sync_slave_with_master
>>> +-- echo ******** [ SLAVE ] ********
>>> +-- eval use $dbname_lower
>>> +show tables;
>>> +
>>> +# action: fill data into tables
>>> +-- connection master
>>> +-- echo ******** [ MASTER ] ********
>>> +-- eval use $dbname_upper
>>> +INSERT INTO T1 VALUES (1);
>>> +INSERT INTO T2 VALUES (1);
>>> +use test;
>>> +-- eval INSERT INTO $dbname_upper.T1 VALUES (2)
>>> +-- eval INSERT INTO $dbname_upper.T2 VALUES (2)
>>> +-- eval LOAD DATA INFILE '../../std_data/words.dat' INTO TABLE
> $dbname_upper.T3
>>> +
>>> +# assert: lower case tables on lower case database on slave
>>> +#         get updates from upper case tables on upper case
>>> +#         database on master
>>> +-- sync_slave_with_master
>>> +-- echo ******** [ SLAVE ] ********
>>> +
>>> +-- let $diff_table_1=master:$dbname_upper.T1
>>> +-- let $diff_table_2=slave:$dbname_lower.t1
>>> +-- source include/diff_tables.inc
>>> +
>>> +-- let $diff_table_1=master:$dbname_upper.T2
>>> +-- let $diff_table_2=slave:$dbname_lower.t2
>>> +-- source include/diff_tables.inc
>>> +
>>> +-- let $diff_table_1=master:$dbname_upper.T3
>>> +-- let $diff_table_2=slave:$dbname_lower.t3
>>> +-- source include/diff_tables.inc
>>> +
>>> +# clean up
>>> +-- connection master
>>> +-- echo ******** [ MASTER ] ********
>>> +-- eval DROP DATABASE $dbname_upper
>>> +-- sync_slave_with_master
>>>
>>> === added file
> 'mysql-test/suite/rpl/r/rpl_lower_case_table_names_do_db.result'
>>> ---
> a/mysql-test/suite/rpl/r/rpl_lower_case_table_names_do_db.result	1970-01-01 00:00:00
>>> +0000
>>> +++
> b/mysql-test/suite/rpl/r/rpl_lower_case_table_names_do_db.result	2009-07-03 10:23:57
>>> +0000
>>> @@ -0,0 +1,56 @@
>>> +stop slave;
>>> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>>> +reset master;
>>> +reset slave;
>>> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>>> +start slave;
>>> +SHOW DATABASES;
>>> +Database
>>> +information_schema
>>> +mtr
>>> +mysql
>>> +test
>>> +CREATE TABLE T1 (a int);
>>> +INSERT INTO T1 VALUES (1);
>>> +Comparing tables master:test.T1 and slave:test.t1
>>> +DROP TABLE T1;
>>> +******** [ MASTER ] ********
>>> +CREATE DATABASE BUG_37656;
>>> +use BUG_37656;
>>> +show databases like 'BUG_37656';
>>> +Database (BUG_37656)
>>> +BUG_37656
>>> +******** [ SLAVE ] ********
>>> +show databases like 'bug_37656';
>>> +Database (bug_37656)
>>> +bug_37656
>>> +******** [ MASTER ] ********
>>> +CREATE TABLE T1 (a int);
>>> +CREATE TABLE T2 (b int) ENGINE=InnoDB;
>>> +CREATE TABLE T3 (txt TEXT);
>>> +show tables;
>>> +Tables_in_BUG_37656
>>> +T1
>>> +T2
>>> +T3
>>> +******** [ SLAVE ] ********
>>> +use bug_37656;
>>> +show tables;
>>> +Tables_in_bug_37656
>>> +t1
>>> +t2
>>> +t3
>>> +******** [ MASTER ] ********
>>> +use BUG_37656;
>>> +INSERT INTO T1 VALUES (1);
>>> +INSERT INTO T2 VALUES (1);
>>> +use test;
>>> +INSERT INTO BUG_37656.T1 VALUES (2);
>>> +INSERT INTO BUG_37656.T2 VALUES (2);
>>> +LOAD DATA INFILE '../../std_data/words.dat' INTO TABLE BUG_37656.T3;
>>> +******** [ SLAVE ] ********
>>> +Comparing tables master:BUG_37656.T1 and slave:bug_37656.t1
>>> +Comparing tables master:BUG_37656.T2 and slave:bug_37656.t2
>>> +Comparing tables master:BUG_37656.T3 and slave:bug_37656.t3
>>> +******** [ MASTER ] ********
>>> +DROP DATABASE BUG_37656;
>>>
>>> === added file
> 'mysql-test/suite/rpl/r/rpl_lower_case_table_names_do_table.result'
>>> ---
> a/mysql-test/suite/rpl/r/rpl_lower_case_table_names_do_table.result	1970-01-01
>>> 00:00:00 +0000
>>> +++
> b/mysql-test/suite/rpl/r/rpl_lower_case_table_names_do_table.result	2009-07-03
>>> 10:23:57 +0000
>>> @@ -0,0 +1,46 @@
>>> +stop slave;
>>> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>>> +reset master;
>>> +reset slave;
>>> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>>> +start slave;
>>> +******** [ MASTER ] ********
>>> +CREATE DATABASE BUG_37656;
>>> +use BUG_37656;
>>> +show databases like 'BUG_37656';
>>> +Database (BUG_37656)
>>> +BUG_37656
>>> +******** [ SLAVE ] ********
>>> +show databases like 'bug_37656';
>>> +Database (bug_37656)
>>> +bug_37656
>>> +******** [ MASTER ] ********
>>> +CREATE TABLE T1 (a int);
>>> +CREATE TABLE T2 (b int) ENGINE=InnoDB;
>>> +CREATE TABLE T3 (txt TEXT);
>>> +show tables;
>>> +Tables_in_BUG_37656
>>> +T1
>>> +T2
>>> +T3
>>> +******** [ SLAVE ] ********
>>> +use bug_37656;
>>> +show tables;
>>> +Tables_in_bug_37656
>>> +t1
>>> +t2
>>> +t3
>>> +******** [ MASTER ] ********
>>> +use BUG_37656;
>>> +INSERT INTO T1 VALUES (1);
>>> +INSERT INTO T2 VALUES (1);
>>> +use test;
>>> +INSERT INTO BUG_37656.T1 VALUES (2);
>>> +INSERT INTO BUG_37656.T2 VALUES (2);
>>> +LOAD DATA INFILE '../../std_data/words.dat' INTO TABLE BUG_37656.T3;
>>> +******** [ SLAVE ] ********
>>> +Comparing tables master:BUG_37656.T1 and slave:bug_37656.t1
>>> +Comparing tables master:BUG_37656.T2 and slave:bug_37656.t2
>>> +Comparing tables master:BUG_37656.T3 and slave:bug_37656.t3
>>> +******** [ MASTER ] ********
>>> +DROP DATABASE BUG_37656;
>>>
>>> === added file
> 'mysql-test/suite/rpl/r/rpl_lower_case_table_names_rewrite_db.result'
>>> ---
> a/mysql-test/suite/rpl/r/rpl_lower_case_table_names_rewrite_db.result	1970-01-01
>>> 00:00:00 +0000
>>> +++
> b/mysql-test/suite/rpl/r/rpl_lower_case_table_names_rewrite_db.result	2009-07-03
>>> 10:23:57 +0000
>>> @@ -0,0 +1,26 @@
>>> +stop slave;
>>> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>>> +reset master;
>>> +reset slave;
>>> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>>> +start slave;
>>> +SET SQL_LOG_BIN=0;
>>> +CREATE DATABASE B37656;
>>> +SET SQL_LOG_BIN=1;
>>> +CREATE DATABASE bug37656;
>>> +USE B37656;
>>> +CREATE TABLE T1 (a int);
>>> +INSERT INTO T1 VALUES (1);
>>> +### assertion: master contains capitalized case table
>>> +SHOW TABLES;
>>> +Tables_in_B37656
>>> +T1
>>> +use bug37656;
>>> +### assertion: slave contains lowered case table
>>> +SHOW TABLES;
>>> +Tables_in_bug37656
>>> +t1
>>> +### assertion: master and slave tables do not differ
>>> +Comparing tables master:B37656.T1 and slave:bug37656.t1
>>> +DROP DATABASE B37656;
>>> +DROP DATABASE bug37656;
>>>
>>> === added file
> 'mysql-test/suite/rpl/t/rpl_lower_case_table_names_do_db-slave.opt'
>>> ---
> a/mysql-test/suite/rpl/t/rpl_lower_case_table_names_do_db-slave.opt	1970-01-01
>>> 00:00:00 +0000
>>> +++
> b/mysql-test/suite/rpl/t/rpl_lower_case_table_names_do_db-slave.opt	2009-07-03
>>> 10:23:57 +0000
>>> @@ -0,0 +1 @@
>>> +--replicate-do-db=BuG_37656 --replicate-do-db=TESt
> --lower-case-table-names=1
>>>
>>> === added file
> 'mysql-test/suite/rpl/t/rpl_lower_case_table_names_do_db.test'
>>> --- a/mysql-test/suite/rpl/t/rpl_lower_case_table_names_do_db.test	1970-01-01
> 00:00:00
>>> +0000
>>> +++ b/mysql-test/suite/rpl/t/rpl_lower_case_table_names_do_db.test	2009-07-03
> 10:23:57
>>> +0000
>>> @@ -0,0 +1,41 @@
>>> +# BUG#37656
>>> +#  
>>> +#  For details look into extra/rpl_tests/rpl_lower_case_table_names.test
>>> +# 
>>> +#  Apart from the assertions present in the included file, this test also
> does
>>> +#  an extra assertion. Check below.
>>> +
>>> +-- source include/master-slave.inc
>>> +-- source include/have_innodb.inc
>>> +-- source include/not_windows.inc
>>> +
>>> +# assertion: assert that test.T1 replicates to test.t1 despite
>>> +#            --replicate-do-db=TESt is set (meaning that names of databases
>>> +#            are lowered case when using lower_case_table_names=1)
>>> +#
>>> +#            The same check is done for the included file, because
>>> +#            --replicate-db-db=BuG_37656 is set
>>> +
>>> +SHOW DATABASES;
>>> +CREATE TABLE T1 (a int);
>>> +INSERT INTO T1 VALUES (1);
>>> +
>>> +-- sync_slave_with_master
>>> +
>>> +-- let $diff_table_1=master:test.T1
>>> +-- let $diff_table_2=slave:test.t1
>>> +-- source include/diff_tables.inc
>>> +
>>> +-- connection master
>>> +
>>> +DROP TABLE T1;
>>> +
>>> +-- sync_slave_with_master
>>> +
>>> +-- connection master
>>> +
>>> +# check test details inside extra/rpl_tests/rpl_lower_case_table_names.test
>>> +#
>>> +
>>> +-- let $engine=InnoDB
>>> +-- source extra/rpl_tests/rpl_lower_case_table_names.test
>>>
>>> === added file
> 'mysql-test/suite/rpl/t/rpl_lower_case_table_names_do_table-slave.opt'
>>> ---
> a/mysql-test/suite/rpl/t/rpl_lower_case_table_names_do_table-slave.opt	1970-01-01
>>> 00:00:00 +0000
>>> +++
> b/mysql-test/suite/rpl/t/rpl_lower_case_table_names_do_table-slave.opt	2009-07-03
>>> 10:23:57 +0000
>>> @@ -0,0 +1 @@
>>> +--replicate-do-table=bug_37656.t1 --replicate-do-table=bug_37656.T2
>>> --replicate-do-table=Bug_37656.t3 --lower-case-table-names=1
>>>
>>> === added file
> 'mysql-test/suite/rpl/t/rpl_lower_case_table_names_do_table.test'
>>> ---
> a/mysql-test/suite/rpl/t/rpl_lower_case_table_names_do_table.test	1970-01-01 00:00:00
>>> +0000
>>> +++
> b/mysql-test/suite/rpl/t/rpl_lower_case_table_names_do_table.test	2009-07-03 10:23:57
>>> +0000
>>> @@ -0,0 +1,17 @@
>>> +# BUG#37656
>>> +#  
>>> +#  For details look into extra/rpl_tests/rpl_lower_case_table_names.test
>>> +# 
>>> +#  Apart from the assertions present in the included file, this test also:
>>> +#
>>> +#   (i) implicitly, checks that filter rules are turned to lower case
>>> +#       when lower_case_table_names=1 is used. Check -slave.opt file for 
>>> +#       rule definition.
>>> +#  
>>> +
>>> +-- source include/master-slave.inc
>>> +-- source include/have_innodb.inc
>>> +-- source include/not_windows.inc
>>> +
>>> +-- let $engine=InnoDB
>>> +-- source extra/rpl_tests/rpl_lower_case_table_names.test
>>>
>>> === added file
> 'mysql-test/suite/rpl/t/rpl_lower_case_table_names_rewrite_db-slave.opt'
>>> ---
> a/mysql-test/suite/rpl/t/rpl_lower_case_table_names_rewrite_db-slave.opt	1970-01-01
>>> 00:00:00 +0000
>>> +++
> b/mysql-test/suite/rpl/t/rpl_lower_case_table_names_rewrite_db-slave.opt	2009-07-03
>>> 10:23:57 +0000
>>> @@ -0,0 +1 @@
>>> +--lower-case-table-names=1 "--replicate-rewrite-db=b37656->BuG37656"
>>>
>>> === added file
> 'mysql-test/suite/rpl/t/rpl_lower_case_table_names_rewrite_db.test'
>>> ---
> a/mysql-test/suite/rpl/t/rpl_lower_case_table_names_rewrite_db.test	1970-01-01
>>> 00:00:00 +0000
>>> +++
> b/mysql-test/suite/rpl/t/rpl_lower_case_table_names_rewrite_db.test	2009-07-03
>>> 10:23:57 +0000
>>> @@ -0,0 +1,58 @@
>>> +# BUG#37656
>>> +#
>>> +# DESCRIPTION
>>> +#
>>> +#
>>> +#  This test case is tests whether replication works properly when
>>> +#  slave is configured with --lower-case-table-names=1 and replication
>>> +#  rewrite rules are in effect.
>>> +#
>>> +#  It checks four issues:
>>> +# 
>>> +#   (i) master contains capitalized table name
>>> +#
>>> +#  (ii) slave contains lowered case table name
>>> +#
>>> +# (iii) master and slave tables do not differ
>>> +#
>>> +#  (iv) implicitly, checks that rewrite rules are turned to lowercase
>>> +#       when lower_case_table_names=1 on slave. Check -slave.opt file
>>> +#       for rewrite rule: b37656->BuG37656
>>> +
>>> +-- source include/master-slave.inc
>>> +-- source include/not_windows.inc
>>> +
>>> +SET SQL_LOG_BIN=0;
>>> +CREATE DATABASE B37656;
>>> +SET SQL_LOG_BIN=1;
>>> +
>>> +-- connection slave
>>> +CREATE DATABASE bug37656;
>>> +
>>> +-- connection master
>>> +USE B37656;
>>> +CREATE TABLE T1 (a int);
>>> +INSERT INTO T1 VALUES (1);
>>> +
>>> +-- echo ### assertion: master contains capitalized case table
>>> +SHOW TABLES;
>>> +
>>> +-- sync_slave_with_master
>>> +
>>> +use bug37656;
>>> +
>>> +-- echo ### assertion: slave contains lowered case table
>>> +SHOW TABLES;
>>> +
>>> +
>>> +-- echo ### assertion: master and slave tables do not differ
>>> +let $diff_table_1=master:B37656.T1;
>>> +let $diff_table_2=slave:bug37656.t1;
>>> +
>>> +-- source include/diff_tables.inc
>>> +
>>> +-- connection master
>>> +DROP DATABASE B37656;
>>> +
>>> +-- sync_slave_with_master
>>> +DROP DATABASE bug37656;
>>>
>>> === modified file 'sql/log_event.cc'
>>> --- a/sql/log_event.cc	2009-06-09 16:53:34 +0000
>>> +++ b/sql/log_event.cc	2009-07-03 10:23:57 +0000
>>> @@ -2939,6 +2939,8 @@ int Query_log_event::do_apply_event(Rela
>>>  int Query_log_event::do_apply_event(Relay_log_info const *rli,
>>>                                        const char *query_arg, uint32
> q_len_arg)
>>>  {
>>> +  char db_buf[NAME_LEN + 1];
>>> +  strmov(db_buf, db);
>>>    LEX_STRING new_db;
>>>    int expected_error,actual_error= 0;
>>>    /*
>>> @@ -2950,7 +2952,9 @@ int Query_log_event::do_apply_event(Rela
>>>    */
>>>    thd->catalog= catalog_len ? (char *) catalog : (char *)"";
>>>    new_db.length= db_len;
>>> -  new_db.str= (char *) rpl_filter->get_rewrite_db(db,
> &new_db.length);
>>> +  if (lower_case_table_names == 1)
>>> +    my_casedn_str(system_charset_info, db_buf);
>>> +  new_db.str= (char *) rpl_filter->get_rewrite_db(db_buf,
> &new_db.length);
>> when lower_case_table_names is not 1, then you do unnecessary
>> copying to db_buf.
>>
>>
>> I suggest to do something like this:
>>
>> char db_buf[NAME_LEN +1];
>> ...
>> if (lower_case_table_names == 1)
>> {
>>    strmov(db_buf, db);
>>    new_db.str= (char *) rpl_filter->get_rewrite_db(db_buf,
> &new_db.length);
>> }
>> else
>>    new_db.str= (char *) rpl_filter->get_rewrite_db(db, &new_db.length);
>>
>>

I made a closer look into the patch
and found some duplicate code.

Perhaps a better idea is even to add a new method:


Log_event::set_rewrite_db()
{
   char lcase_db_buf[NAME_LEN +1];
   LEX_STRING new_db;
   new_db.length= db_len;
   if (lower_case_table_names == 1)
   {
     strmov(lcase_db_buf, db);
     my_casedn_str(system_charset_info, lcase_db_buf);
     newdb.str= lcase_db_buf;
   }
   else
   {
     newdb.str= db;
   }
   new_db.str= (char*) rpl_filter->get_rewrite_db(newdb.str,
                                                  &new_db.length);
   thd->set_db(new_db.str, new_db.length);
}


This will help to avoid unnecessary strmov() when
lower_case_table_names is 0, and also help to get
rid of duplicate code in
Query_log_event::do_apply_event() and Load_log_event::do_apply_event().


So instead of

   char db_buf[NAME_LEN + 1];
   strmov(db_buf, db);
   LEX_STRING new_db;
   int expected_error,actual_error= 0;
   thd->catalog= catalog_len ? (char *) catalog : (char *)"";
   new_db.length= db_len;
   if (lower_case_table_names == 1)
     my_casedn_str(system_charset_info, db_buf);
   new_db.str= (char *) rpl_filter->get_rewrite_db(db_buf,
                                                   &new_db.length);
   thd->set_db(new_db.str, new_db.length); /* allocates a copy of 'db' */


you could just do:

   thd->catalog= catalog_len ? (char *) catalog : (char *)"";
   set_rewrite_db();


in both Query_log_event::do_apply_event() and 
Load_log_event::do_apply_event().


Please also find one comment below:


>> And in many other places below.
>>
>>
>> Otherwise the patch looks fine for me.
>>
>> I have ticked the checkbox.
> 
> Thanks for the input I will check this. Also, I was mostly concerned
> about the charset issue which you provided very good feedback, again
> thanks.
> 
>>>    thd->set_db(new_db.str, new_db.length);       /* allocates a copy of
> 'db' */
>>>    thd->variables.auto_increment_increment= auto_increment_increment;
>>>    thd->variables.auto_increment_offset=    auto_increment_offset;
>>> @@ -4445,9 +4449,13 @@ void Load_log_event::set_fields(const ch
>>>  int Load_log_event::do_apply_event(NET* net, Relay_log_info const *rli,
>>>                                     bool use_rli_only_for_errors)
>>>  {
>>> +  char db_buf[NAME_LEN + 1];
>>> +  strmov(db_buf, db);
>>>    LEX_STRING new_db;
>>>    new_db.length= db_len;
>>> -  new_db.str= (char *) rpl_filter->get_rewrite_db(db,
> &new_db.length);
>>> +  if (lower_case_table_names == 1)
>>> +    my_casedn_str(system_charset_info, db_buf);
>>> +  new_db.str= (char *) rpl_filter->get_rewrite_db(db_buf,
> &new_db.length);
>>>    thd->set_db(new_db.str, new_db.length);
>>>    DBUG_ASSERT(thd->query == 0);
>>>    thd->query_length= 0;                         // Should not be needed
>>> @@ -4505,9 +4513,13 @@ int Load_log_event::do_apply_event(NET* 
>>>      thd->warning_info->opt_clear_warning_info(thd->query_id);
>>>  
>>>      TABLE_LIST tables;
>>> +    char table_buf[NAME_LEN + 1];
>>> +    strmov(table_buf, table_name);
>>> +    if (lower_case_table_names == 1)
>>> +      my_casedn_str(system_charset_info, table_buf);
>>>      bzero((char*) &tables,sizeof(tables));
>>>      tables.db= thd->strmake(thd->db, thd->db_length);
>>> -    tables.alias = tables.table_name = (char*) table_name;
>>> +    tables.alias = tables.table_name = table_buf;
>>>      tables.lock_type = TL_WRITE;
>>>      tables.updating= 1;
>>>  
>>> @@ -8051,7 +8063,7 @@ Table_map_log_event::~Table_map_log_even
>>>  int Table_map_log_event::do_apply_event(Relay_log_info const *rli)
>>>  {
>>>    RPL_TABLE_LIST *table_list;
>>> -  char *db_mem, *tname_mem;
>>> +  char *db_mem, *tname_mem, *ptr;
>>>    MDL_request *mdl_request;
>>>    size_t dummy_len;
>>>    void *memory;
>>> @@ -8078,8 +8090,18 @@ int Table_map_log_event::do_apply_event(
>>>    table_list->next_global= table_list->next_local= 0;
>>>    table_list->table_id= m_table_id;
>>>    table_list->updating= 1;
>>> -  strmov(table_list->db, rpl_filter->get_rewrite_db(m_dbnam,
> &dummy_len));
>>> +  strmov(table_list->db, m_dbnam);
>>>    strmov(table_list->table_name, m_tblnam);
>>> +  if (lower_case_table_names == 1)
>>> +  {
>>> +    my_casedn_str(system_charset_info, table_list->db);
>>> +    my_casedn_str(system_charset_info, table_list->table_name);
>>> +  }
>>> +
>>> +  /* rewrite rules changed the database. */
>>> +  if ((ptr= (char*) rpl_filter->get_rewrite_db(table_list->db,
> &dummy_len)) != 
>>> +      table_list->db)
>>> +    strmov(table_list->db, ptr);
>>>    mdl_request->init(0, table_list->db, table_list->table_name);
>>>    table_list->mdl_request= mdl_request;
>>>  
>>>
>>> === modified file 'sql/mysqld.cc'
>>> --- a/sql/mysqld.cc	2009-06-17 07:30:19 +0000
>>> +++ b/sql/mysqld.cc	2009-07-03 10:23:57 +0000
>>> @@ -3696,6 +3696,12 @@ You should consider changing lower_case_
>>>        (test_if_case_insensitive(mysql_real_data_home) == 1);
>>>    }
>>>  
>>> +  if (lower_case_table_names == 1)
>>> +  {
>>> +    rpl_filter->to_lower_case(system_charset_info);
>>> +    binlog_filter->to_lower_case(system_charset_info);
>>> +  }
>>> +
>>>    /* Reset table_alias_charset, now that lower_case_table_names is set. */
>>>    table_alias_charset= (lower_case_table_names ?
>>>  			files_charset_info :
>>>
>>> === modified file 'sql/rpl_filter.cc'
>>> --- a/sql/rpl_filter.cc	2009-04-29 03:50:14 +0000
>>> +++ b/sql/rpl_filter.cc	2009-07-03 10:23:57 +0000
>>> @@ -299,7 +299,9 @@ Rpl_filter::add_wild_ignore_table(const 
>>>  void
>>>  Rpl_filter::add_db_rewrite(const char* from_db, const char* to_db)
>>>  {
>>> -  i_string_pair *db_pair = new i_string_pair(from_db, to_db);
>>> +  char *from_db_cp= my_strdup(from_db, MYF(MY_WME | ME_FATALERROR));
>>> +  char *to_db_cp= my_strdup(to_db, MYF(MY_WME | ME_FATALERROR));
>>> +  i_string_pair *db_pair = new i_string_pair(from_db_cp, to_db_cp);

I can see new my_strdups, but didn't find new my_frees.
Please make sure that the space allocated in my_strdups is
freed.


>>>    rewrite_db.push_back(db_pair);
>>>  }
>>>  
>>> @@ -348,7 +350,8 @@ void
>>>  Rpl_filter::add_do_db(const char* table_spec)
>>>  {
>>>    DBUG_ENTER("Rpl_filter::add_do_db");
>>> -  i_string *db = new i_string(table_spec);
>>> +  char *tbl_spec_cp= my_strdup(table_spec, MYF(MY_WME | ME_FATALERROR));
>>> +  i_string *db = new i_string(tbl_spec_cp);
>>>    do_db.push_back(db);
>>>    DBUG_VOID_RETURN;
>>>  }
>>> @@ -358,7 +361,8 @@ void
>>>  Rpl_filter::add_ignore_db(const char* table_spec)
>>>  {
>>>    DBUG_ENTER("Rpl_filter::add_ignore_db");
>>> -  i_string *db = new i_string(table_spec);
>>> +  char *tbl_spec_cp= my_strdup(table_spec, MYF(MY_WME | ME_FATALERROR));
>>> +  i_string *db = new i_string(tbl_spec_cp);
>>>    ignore_db.push_back(db);
>>>    DBUG_VOID_RETURN;
>>>  }
>>> @@ -546,3 +550,72 @@ Rpl_filter::get_ignore_db()
>>>  {
>>>    return &ignore_db;
>>>  }
>>> +
>>> +void 
>>> +Rpl_filter::to_lower_case(CHARSET_INFO *cs_info, DYNAMIC_ARRAY* a)
>>> +{
>>> +  for (uint i= 0; i < a->elements; i++)
>>> +  {
>>> +    TABLE_RULE_ENT* e;
>>> +    get_dynamic(a, (uchar*)&e, i);
>>> +    my_casedn_str(cs_info, e->db);
>>> +    my_casedn_str(cs_info, e->tbl_name);
>>> +  }
>>> +}
>>> +
>>> +void 
>>> +Rpl_filter::to_lower_case(CHARSET_INFO *cs_info, HASH* h)
>>> +{
>>> +  for (uint i= 0; i < h->records; i++)
>>> +  {
>>> +    TABLE_RULE_ENT* e= (TABLE_RULE_ENT*) my_hash_element(h, i);
>>> +    my_casedn_str(cs_info, e->db);
>>> +    my_casedn_str(cs_info, e->tbl_name);
>>> +  }
>>> +}
>>> +
>>> +void 
>>> +Rpl_filter::to_lower_case(CHARSET_INFO *cs_info, I_List<i_string>* l)
>>> +{
>>> +  I_List_iterator<i_string> it(*l);
>>> +  i_string *tmp= NULL;
>>> +
>>> +  while ((tmp=it++))
>>> +    my_casedn_str(cs_info, const_cast<char *>(tmp->ptr));
>>> +}
>>> +
>>> +void
>>> +Rpl_filter::to_lower_case(CHARSET_INFO * cs_info)
>>> +{
>>> +  // rewrite_db
>>> +  if (!rewrite_db.is_empty())
>>> +  {
>>> +    I_List_iterator<i_string_pair> it(rewrite_db);
>>> +    i_string_pair* tmp= NULL;
>>> +
>>> +    while ((tmp=it++))
>>> +    {
>>> +      my_casedn_str(cs_info, const_cast<char *>(tmp->key));
>>> +      my_casedn_str(cs_info, const_cast<char *>(tmp->val));
>>> +    }
>>> +  }
>>> +
>>> +  if (!do_db.is_empty())
>>> +    to_lower_case(cs_info, &do_db);
>>> +
>>> +  if (!ignore_db.is_empty())
>>> +    to_lower_case(cs_info, &ignore_db);
>>> +
>>> +  if (do_table_inited)
>>> +    to_lower_case(cs_info, &do_table);
>>> +
>>> +  if (ignore_table_inited)
>>> +    to_lower_case(cs_info, &ignore_table);
>>> +
>>> +  if (wild_do_table_inited)
>>> +    to_lower_case(cs_info, &wild_do_table);
>>> +
>>> +  if (wild_ignore_table_inited)
>>> +    to_lower_case(cs_info, &wild_ignore_table);
>>> +
>>> +}
>>>
>>> === modified file 'sql/rpl_filter.h'
>>> --- a/sql/rpl_filter.h	2007-05-10 09:59:39 +0000
>>> +++ b/sql/rpl_filter.h	2009-07-03 10:23:57 +0000
>>> @@ -74,6 +74,13 @@ public:
>>>    I_List<i_string>* get_do_db();
>>>    I_List<i_string>* get_ignore_db();
>>>  
>>> +  /* 
>>> +    To lower case method.
>>> +
>>> +    This method will down case all filter contents.
>>> +  */
>>> +  void to_lower_case(CHARSET_INFO * cs_info);
>>> +
>>>  private:
>>>    bool table_rules_on;
>>>  
>>> @@ -90,6 +97,10 @@ private:
>>>                                             bool inited);
>>>    TABLE_RULE_ENT* find_wild(DYNAMIC_ARRAY *a, const char* key, int len);
>>>  
>>> +  void to_lower_case(CHARSET_INFO *cs_info, I_List<i_string>* l);
>>> +  void to_lower_case(CHARSET_INFO *cs_info, HASH* h);
>>> +  void to_lower_case(CHARSET_INFO *cs_info, DYNAMIC_ARRAY* a);
>>> +
>>>    /*
>>>      Those 4 structures below are uninitialized memory unless the
>>>      corresponding *_inited variables are "true".
>>>
>>>
>>> Attachment: [text/bzr-bundle]
> bzr/luis.soares@stripped
>>>
>>> Thread
>>> • bzr commit into mysql-5.4 branch (luis.soares:2804) Bug#37656	Luis
> Soares	3 Jul 2009
>>>   • Re: bzr commit into mysql-5.4 branch (luis.soares:2804) Bug#37656	He
> Zhenxing	6 Jul 2009
>>>
>>> © 1995-2008 MySQL AB, 2008-2009 Sun Microsystems, Inc.
>>>
>>>     * Online Shop
>>>     * Privacy Policy
>>>     * Legal
>>>     * Contact Us
>>>     * No Software Patents!
>>>
>>>
>>> Page generated in 1.258 sec. using MySQL 5.1.14-beta-log

Thread
Re: #37656 [Pnd]: lower_case_table_names=1 doesn't convert databasenames in replicated statementsAlexander Barkov8 Jul
  • Re: #37656 [Pnd]: lower_case_table_names=1 doesn't convert databasenames in replicated statementsLuís Soares8 Jul
    • Re: #37656 [Pnd]: lower_case_table_names=1 doesn't convert databasenames in replicated statementsAlexander Barkov9 Jul
      • Re: #37656 [Pnd]: lower_case_table_names=1 doesn't convert databasenames in replicated statementsLuís Soares20 Jul
        • Re: #37656 [Pnd]: lower_case_table_names=1 doesn't convert databasenames in replicated statementsAlexander Barkov21 Jul
          • Re: #37656 [Pnd]: lower_case_table_names=1 doesn't convert databasenames in replicated statementsLuís Soares21 Jul
          • Re: #37656 [Pnd]: lower_case_table_names=1 doesn't convert databasenames in replicated statementsLuís Soares2 Mar
            • Re: #37656 [Pnd]: lower_case_table_names=1 doesn't convert databasenames in replicated statementsHe Zhenxing3 Mar
              • Re: #37656 [Pnd]: lower_case_table_names=1 doesn't convert databasenames in replicated statementsLuís Soares4 Mar