List:Commits« Previous MessageNext Message »
From:Luís Soares Date:July 3 2009 10:08am
Subject:Re: bzr commit into mysql-5.4 branch (luis.soares:2804) Bug#37656
View as plain text  
Hi Zhenxing,

  Thanks for the review. Please, check some comments inline.

On Fri, 2009-07-03 at 13:59 +0800, He Zhenxing wrote:
> Hi Luis,
> 
> Nice work! and sorry for delay on review!
> 
> The patch is OK, and the test case is very constructive, thanks! I only
> have some minor comments on the coding, please have a look! 
> 
> Patch approved after consideration!
> 
> Luis Soares wrote:
> > #At
> file:///home/lsoares/Workspace/mysql-server/bugfix/b37656/mysql-azalea-bugfixing/ based on
> revid:alik@stripped
> > 
> >  2804 Luis Soares	2009-06-22
> >       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-06-22
> 18:26:59 +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-06-22
> 18:26:59 +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-06-22 18:26:59
> +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-06-22 18:26:59
> +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-06-22 18:26:59
> +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-06-22
> 18:26:59 +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-06-22 18:26:59
> +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-06-22
> 18:26:59 +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-06-22
> 18:26:59 +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-06-22 18:26:59
> +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-06-22 18:26:59 +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[db_len];
> > +  strmov(db_buf, db);
> >    LEX_STRING new_db;
> >    int expected_error,actual_error= 0;
> >    /*
> > @@ -2950,7 +2952,26 @@ 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)
> > +  {
> > +    CHARSET_INFO *cs;
> > +    if (charset_database_number)
> > +    {
> > +      if (!(cs= get_charset(charset_database_number, MYF(0))))
> > +      {
> > +        char buf[20];
> > +        int10_to_str((int) charset_database_number, buf, -10);
> > +        my_error(ER_UNKNOWN_COLLATION, MYF(0), buf);
> > +        goto compare_errors;
> > +      }
> > +    }
> > +    else
> > +      cs= thd->db_charset;
> > +    
> > +    my_casedn_str(cs, 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'
> */
> >    thd->variables.auto_increment_increment= auto_increment_increment;
> >    thd->variables.auto_increment_offset=    auto_increment_offset;
> > @@ -4445,9 +4466,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[db_len];
> > +  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(thd->variables.collation_database, db_buf);
> > +  new_db.str= (char *) rpl_filter->get_rewrite_db(db_buf,
> &new_db.length);
> 
> 
> I'm wandering why 'charset_database_number' is not used for LOAD DATA
> statement, I'm suspecting this is bug#45516.

FYI, Bar asked me to not use thd->variables.collation_database and use
system_charset_info.

> 
> >    thd->set_db(new_db.str, new_db.length);
> >    DBUG_ASSERT(thd->query == 0);
> >    thd->query_length= 0;                         // Should not be needed
> > @@ -4505,9 +4530,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[strlen(table_name)+1];
> > +    strmov(table_buf, table_name);
> > +    if (lower_case_table_names == 1)
> > +      my_casedn_str(thd->variables.collation_database, 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 +8080,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 +8107,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(thd->variables.collation_database, table_list->db);
> > +    my_casedn_str(thd->variables.collation_database,
> 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-06-22 18:26:59 +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(global_system_variables.collation_database);
> > +   
> binlog_filter->to_lower_case(global_system_variables.collation_database);
> > +  }
> > +
> >    /* 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-06-22 18:26:59 +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);
> >    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,82 @@ Rpl_filter::get_ignore_db()
> >  {
> >    return &ignore_db;
> >  }
> > +
> > +void 
> > +Rpl_filter::to_lower_case_dyn_array(CHARSET_INFO *cs_info, DYNAMIC_ARRAY* a,
> bool inited)
> 
> I think you can make use of the function overloading mechanism of C++,
> and name all these functions 'to_lower_case'.

I don't have a strong opinion on this one. Will do as you suggested.

> And I'd also suggest to remove argument 'inited', and only call this
> when the array is inited.
> 
> if (inited)
>   to_lower_case....

Will do.

> See comments below.
> 
> > +{
> > +  if (inited)
> > +  {
> > +    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_hash(CHARSET_INFO *cs_info, HASH* h, bool inited)
> 
> See above comment.
> 
> > +{
> > +  if (inited)
> > +  {
> > +    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_list_str(CHARSET_INFO *cs_info,
> I_List<i_string>* l)
> > +{
> > +  // do_db
> > +  if (!l->is_empty())
> > +  {
> > +    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));
> > +    }
> > +  }
> > +
> > +  // do_db
> > +  to_lower_case_list_str(cs_info, &do_db);
> > +
> > +  // ignore_db
> > +  to_lower_case_list_str(cs_info, &ignore_db);
> > +
> > +  // do_table
> > +  to_lower_case_hash(cs_info, &do_table, do_table_inited);
> 
> I'd suggest:
> if (do_table_inited)
>   to_lower_case(...);

Ok.

> > +
> > +  // ignore_table
> > +  to_lower_case_hash(cs_info, &ignore_table, ignore_table_inited);
> > +
> > +  // wild_do_table
> > +  to_lower_case_dyn_array(cs_info, &wild_do_table, wild_do_table_inited);
> > +
> > +  // wild_ignore_table
> > +  to_lower_case_dyn_array(cs_info, &wild_ignore_table,
> wild_ignore_table_inited);
> > +
> > +}
> > 
> > === modified file 'sql/rpl_filter.h'
> > --- a/sql/rpl_filter.h	2007-05-10 09:59:39 +0000
> > +++ b/sql/rpl_filter.h	2009-06-22 18:26:59 +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,11 @@ private:
> >                                             bool inited);
> >    TABLE_RULE_ENT* find_wild(DYNAMIC_ARRAY *a, const char* key, int len);
> >  
> > +  void to_lower_case_list_str(CHARSET_INFO *cs_info, I_List<i_string>*
> l);
> > +  void to_lower_case_hash(CHARSET_INFO *cs_info, HASH* h, bool inited);
> > +  void to_lower_case_dyn_array(CHARSET_INFO *cs_info, DYNAMIC_ARRAY* a, 
> > +                               bool inited);
> > +
> >    /*
> >      Those 4 structures below are uninitialized memory unless the
> >      corresponding *_inited variables are "true".
> > 
> 
> 
-- 
Luís

Thread
bzr commit into mysql-5.4 branch (luis.soares:2804) Bug#37656Luis Soares22 Jun
  • Re: bzr commit into mysql-5.4 branch (luis.soares:2804) Bug#37656He Zhenxing3 Jul
    • Re: bzr commit into mysql-5.4 branch (luis.soares:2804) Bug#37656Luís Soares3 Jul