Hi Daogang,
Nice work! The fix looks good, only have some comments on the test.
STATUS
------
Not Approved
REQUIRED
--------
N/A
REQUESTES
---------
R1. Please add the test cases to the test file:
suite/binlog/t/binlog_stm_unsafe_warning.test
And also please use the method in this test file to check that warnings
are not written when --log-warnings < 1, and are written to the error
log when --log-warnings >= 2.
SUGGESTIONS
-----------
N/A
Dao-Gang.Qu@stripped wrote:
> #At file:///home/daogangqu/mysql/bzrwork/bugtest/mysql-5.1-bugteam/ based on
> revid:dao-gang.qu@stripped
>
> 3233 Dao-Gang.Qu@stripped 2009-12-03
> Bug #42851 Spurious "Statement is not safe to log in statement format."
> warnings
>
> The unsafe warnings in error log make error log grow too large.
>
> To fix the problem, the unsafe warnings only will be written to error log,
> when the level of --log-warnings option >= 2.
> @ mysql-test/suite/rpl/r/rpl_unsafe_warnings.result
> Test Result for BUG#42851
> @ mysql-test/suite/rpl/t/rpl_unsafe_warnings.test
> Added the test file to verify the unsafe warnings will be written to error
> log
> when the level of --log-warnings option >= 2, and the 'SHOW WARNINGS' is
> not
> controlled by --log-warnings option.
>
> added:
> mysql-test/suite/rpl/r/rpl_unsafe_warnings.result
> mysql-test/suite/rpl/t/rpl_unsafe_warnings.test
> modified:
> sql/sql_class.cc
> === added file 'mysql-test/suite/rpl/r/rpl_unsafe_warnings.result'
> --- a/mysql-test/suite/rpl/r/rpl_unsafe_warnings.result 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_unsafe_warnings.result 2009-12-03 02:30:51 +0000
> @@ -0,0 +1,77 @@
> +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 @save_log_warnings= @@global.log_warnings;
> +CREATE TABLE `t1` (
> +`recNo` int(10) unsigned NOT NULL AUTO_INCREMENT,
> +`string` varchar(64) NOT NULL,
> +`inUseBy` varchar(38) NOT NULL DEFAULT '',
> +`tsLastUpdated` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE
> +CURRENT_TIMESTAMP,
> +PRIMARY KEY (`recNo`),
> +KEY `tsLastUpdated` (`tsLastUpdated`),
> +KEY `inUseBy` (`inUseBy`)
> +);
> +INSERT INTO t1 SET string='one';
> +INSERT INTO t1 SET string='two';
> +INSERT INTO t1 SET string='three';
> +set @@global.log_warnings= 0;
> +select @@global.log_warnings;
> +@@global.log_warnings
> +0
> +UPDATE t1 SET inUseBy='me' WHERE inUseBy='' limit 1;
> +# When the --log-warnings option is set to 0, Test the unsafe
> +# warning will be displayed by 'SHOW WARNINGS', but it will not
> +# be written to error log.
> +SHOW WARNINGS;
> +Level Code Message
> +Note 1592 Statement may not be safe to log in statement format.
> +set @@global.log_warnings= 1;
> +select @@global.log_warnings;
> +@@global.log_warnings
> +1
> +UPDATE t1 SET inUseBy='me' WHERE inUseBy='' limit 2;
> +# When the --log-warnings option is set to 1, Test the unsafe
> +# warning will be displayed by 'SHOW WARNINGS', but it will not
> +# be written to error log.
> +SHOW WARNINGS;
> +Level Code Message
> +Note 1592 Statement may not be safe to log in statement format.
> +set @@global.log_warnings= 2;
> +select @@global.log_warnings;
> +@@global.log_warnings
> +2
> +UPDATE t1 SET inUseBy='me' WHERE inUseBy='' limit 3;
> +# When the --log-warnings option is set to 2, Test the unsafe
> +# warning will be displayed by 'SHOW WARNINGS', and it also will
> +# be written to error log.
> +SHOW WARNINGS;
> +Level Code Message
> +Note 1592 Statement may not be safe to log in statement format.
> +set @@global.log_warnings= 3;
> +select @@global.log_warnings;
> +@@global.log_warnings
> +3
> +UPDATE t1 SET inUseBy='me' WHERE inUseBy='' limit 4;
> +# When the --log-warnings option is set to 3, Test the unsafe
> +# warning will be displayed by 'SHOW WARNINGS', and it also will
> +# be written to error log.
> +SHOW WARNINGS;
> +Level Code Message
> +Note 1592 Statement may not be safe to log in statement format.
> +set @@global.log_warnings= 500;
> +select @@global.log_warnings;
> +@@global.log_warnings
> +500
> +UPDATE t1 SET inUseBy='me' WHERE inUseBy='' limit 5;
> +# When the --log-warnings option is set to 500, Test the unsafe
> +# warning will be displayed by 'SHOW WARNINGS', and it also will
> +# be written to error log.
> +SHOW WARNINGS;
> +Level Code Message
> +Note 1592 Statement may not be safe to log in statement format.
> +set @@global.log_warnings= @save_log_warnings;
> +drop table t1;
>
> === added file 'mysql-test/suite/rpl/t/rpl_unsafe_warnings.test'
> --- a/mysql-test/suite/rpl/t/rpl_unsafe_warnings.test 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_unsafe_warnings.test 2009-12-03 02:30:51 +0000
> @@ -0,0 +1,75 @@
> +#
> +# BUG#42851
> +# This test verifies if the unsafe warnings will be written to error log
> +# when the level of --log-warnings option >= 2, and the 'SHOW WARNINGS'
> +# is not controlled by --log-warnings option.
> +#
> +
> +source include/master-slave.inc;
> +source include/have_binlog_format_statement.inc;
> +
> +set @save_log_warnings= @@global.log_warnings;
> +# create table
> +CREATE TABLE `t1` (
> + `recNo` int(10) unsigned NOT NULL AUTO_INCREMENT,
> + `string` varchar(64) NOT NULL,
> + `inUseBy` varchar(38) NOT NULL DEFAULT '',
> + `tsLastUpdated` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE
> +CURRENT_TIMESTAMP,
> + PRIMARY KEY (`recNo`),
> + KEY `tsLastUpdated` (`tsLastUpdated`),
> + KEY `inUseBy` (`inUseBy`)
> +);
> +# insert test data
> +INSERT INTO t1 SET string='one';
> +INSERT INTO t1 SET string='two';
> +INSERT INTO t1 SET string='three';
> +
> +--disable_warnings
> +
> +set @@global.log_warnings= 0;
> +select @@global.log_warnings;
> +UPDATE t1 SET inUseBy='me' WHERE inUseBy='' limit 1;
> +--echo # When the --log-warnings option is set to 0, Test the unsafe
> +--echo # warning will be displayed by 'SHOW WARNINGS', but it will not
> +--echo # be written to error log.
> +SHOW WARNINGS;
> +
> +set @@global.log_warnings= 1;
> +select @@global.log_warnings;
> +UPDATE t1 SET inUseBy='me' WHERE inUseBy='' limit 2;
> +--echo # When the --log-warnings option is set to 1, Test the unsafe
> +--echo # warning will be displayed by 'SHOW WARNINGS', but it will not
> +--echo # be written to error log.
> +SHOW WARNINGS;
> +
> +set @@global.log_warnings= 2;
> +select @@global.log_warnings;
> +UPDATE t1 SET inUseBy='me' WHERE inUseBy='' limit 3;
> +--echo # When the --log-warnings option is set to 2, Test the unsafe
> +--echo # warning will be displayed by 'SHOW WARNINGS', and it also will
> +--echo # be written to error log.
> +SHOW WARNINGS;
> +
> +set @@global.log_warnings= 3;
> +select @@global.log_warnings;
> +UPDATE t1 SET inUseBy='me' WHERE inUseBy='' limit 4;
> +--echo # When the --log-warnings option is set to 3, Test the unsafe
> +--echo # warning will be displayed by 'SHOW WARNINGS', and it also will
> +--echo # be written to error log.
> +SHOW WARNINGS;
> +
> +set @@global.log_warnings= 500;
> +select @@global.log_warnings;
> +UPDATE t1 SET inUseBy='me' WHERE inUseBy='' limit 5;
> +--echo # When the --log-warnings option is set to 500, Test the unsafe
> +--echo # warning will be displayed by 'SHOW WARNINGS', and it also will
> +--echo # be written to error log.
> +SHOW WARNINGS;
> +
> +--enable_warnings
> +
> +set @@global.log_warnings= @save_log_warnings;
> +drop table t1;
> +sync_slave_with_master;
> +
>
> === modified file 'sql/sql_class.cc'
> --- a/sql/sql_class.cc 2009-11-01 23:13:11 +0000
> +++ b/sql/sql_class.cc 2009-12-03 02:30:51 +0000
> @@ -3859,7 +3859,11 @@ int THD::binlog_query(THD::enum_binlog_q
> push_warning(this, MYSQL_ERROR::WARN_LEVEL_NOTE,
> ER_BINLOG_UNSAFE_STATEMENT,
> ER(ER_BINLOG_UNSAFE_STATEMENT));
> - if (global_system_variables.log_warnings &&
> + /*
> + The unsafe warning will be written to error log, when
> + the level of --log-warnings option >= 2.
> + */
> + if (global_system_variables.log_warnings >= 2 &&
> !(binlog_flags & BINLOG_FLAG_UNSAFE_STMT_PRINTED))
> {
> sql_print_warning("%s Statement: %.*s",
>
