List:Commits« Previous MessageNext Message »
From:Sven Sandberg Date:December 2 2008 6:23pm
Subject:Re: bzr commit into mysql-5.1 branch (mats:2771) Bug#39934
View as plain text  
Hi Mats,

Great work with this bug. Source patch is very clean and readable and
you made lots of improvements to test cases. I think this will both make
replication more robust, error messages more understandable, and tests
run faster (since more combinations are skipped).

Could you add a test case for this bug? Would be good to ensure that at
least all warnings and errors are tested.

I have a few comments inline. It's mostly polishing, but also suggestion
for some small refactoring and a bug in the logic for determining the
error message.


Mats Kindahl wrote:
> #At file:///home/bzr/bugs/b39934-5.1-5.1.29-rc/
> 
>  2771 Mats Kindahl	2008-11-17
>       Bug#39934: Slave stops for engine that only support row-based logging
>       
>       It was possible to get a row-based event in the binary log on the slave
>       even if the slave is running in STATEMENT mode. 
>       
>       This patch clarifies the rules for what format the slave can log in and
>       under what circumstances as well as adding some missing error messages
>       and clarifying some other.
> added:
>   mysql-test/suite/rpl/r/rpl_found_rows_mixed.result
>   mysql-test/suite/rpl/r/rpl_mix_insert_delayed.result
>   mysql-test/suite/rpl/t/rpl_found_rows_mixed.test
>   mysql-test/suite/rpl/t/rpl_mix_insert_delayed.test
>   mysql-test/suite/rpl_ndb/combinations
> modified:
>   mysql-test/extra/rpl_tests/rpl_foreign_key.test
>   mysql-test/extra/rpl_tests/rpl_insert_delayed.test
>   mysql-test/extra/rpl_tests/rpl_truncate.test
>   mysql-test/extra/rpl_tests/rpl_truncate_helper.test
>   mysql-test/include/rpl_udf.inc
>   mysql-test/lib/mtr_report.pl
>   mysql-test/suite/binlog/r/binlog_stm_ps.result
>   mysql-test/suite/binlog/r/binlog_unsafe.result
>   mysql-test/suite/rpl/r/rpl_events.result
>   mysql-test/suite/rpl/r/rpl_extraColmaster_innodb.result
>   mysql-test/suite/rpl/r/rpl_extraColmaster_myisam.result
>   mysql-test/suite/rpl/r/rpl_foreign_key_innodb.result
>   mysql-test/suite/rpl/r/rpl_found_rows.result
>   mysql-test/suite/rpl/r/rpl_idempotency.result
>   mysql-test/suite/rpl/r/rpl_rbr_to_sbr.result
>   mysql-test/suite/rpl/r/rpl_row_max_relay_size.result
>   mysql-test/suite/rpl/r/rpl_skip_error.result
>   mysql-test/suite/rpl/r/rpl_stm_insert_delayed.result
>   mysql-test/suite/rpl/r/rpl_temporary_errors.result
>   mysql-test/suite/rpl/r/rpl_truncate_2myisam.result
>   mysql-test/suite/rpl/r/rpl_truncate_3innodb.result
>   mysql-test/suite/rpl/r/rpl_udf.result
>   mysql-test/suite/rpl/t/rpl_bug31076.test
>   mysql-test/suite/rpl/t/rpl_events.test
>   mysql-test/suite/rpl/t/rpl_extraColmaster_innodb.test
>   mysql-test/suite/rpl/t/rpl_extraColmaster_myisam.test
>   mysql-test/suite/rpl/t/rpl_found_rows.test
>   mysql-test/suite/rpl/t/rpl_idempotency.test
>   mysql-test/suite/rpl/t/rpl_rbr_to_sbr.test
>   mysql-test/suite/rpl/t/rpl_row_max_relay_size.test
>   mysql-test/suite/rpl/t/rpl_slave_skip.test
>   mysql-test/suite/rpl/t/rpl_stm_insert_delayed.test
>   mysql-test/suite/rpl/t/rpl_switch_stm_row_mixed.test
>   mysql-test/suite/rpl/t/rpl_temporary_errors.test
>   mysql-test/suite/rpl/t/rpl_truncate_2myisam.test
>   mysql-test/suite/rpl/t/rpl_truncate_3innodb.test
>   mysql-test/suite/rpl/t/rpl_udf.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_2innodb.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_2myisam.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_basic.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_blob.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_blob2.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_commit_afterflush.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_ctype_ucs2_def.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_delete_nowhere.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_do_db.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_do_table.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_func003.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_innodb_trans.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_insert_ignore.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_multi_update3.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_rep_ignore.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_row_001.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_sp003.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_sp006.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_transaction.test
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_trig004.test
>   sql/log_event.cc
>   sql/share/errmsg.txt
>   sql/sql_base.cc
>   sql/sql_class.cc
>   sql/sql_class.h
>   sql/sql_lex.h
>   sql/sql_prepare.cc
> 
> per-file messages:
>   mysql-test/extra/rpl_tests/rpl_foreign_key.test
>     Removing explicit set to row format.
>   mysql-test/extra/rpl_tests/rpl_insert_delayed.test
>     Adding code to filter out table id from table map events in binlog listing.
>   mysql-test/extra/rpl_tests/rpl_truncate.test
>     Test now only test one binlog format instead of all three
>     (the combinations file take care of testing all combinations).
>   mysql-test/extra/rpl_tests/rpl_truncate_helper.test
>     Removing explicit set of binlog format. Removing redundant binlog
>     listing causing difference in result file between binlog formats.
>   mysql-test/include/rpl_udf.inc
>     Disabling warnings over code that generates warnings for some
>     binlog formats but not for some.
>   mysql-test/lib/mtr_report.pl
>     rpl_udf and rpl_user_variables can generate harmless warnings.
>   mysql-test/suite/rpl/t/rpl_bug31076.test
>     Test only valid for mixed and row mode.
>   mysql-test/suite/rpl/t/rpl_events.test
>     Removing explicit set of binlog format and removing duplicate test
>     (combinations file take care of testing all combinations).
>   mysql-test/suite/rpl/t/rpl_extraColmaster_innodb.test
>     Removing explicit set of binlog format and removing duplicate test
>     (combinations file take care of testing all combinations).
>   mysql-test/suite/rpl/t/rpl_extraColmaster_myisam.test
>     Removing explicit set of binlog format and removing duplicate test
>     (combinations file take care of testing all combinations).
>   mysql-test/suite/rpl/t/rpl_found_rows.test
>     Factoring out part that depends on mixed mode into separate test case.
>   mysql-test/suite/rpl/t/rpl_found_rows_mixed.test
>     New test case for parts of rpl_found_rows that are only relevant for mixed mode.
>   mysql-test/suite/rpl/t/rpl_idempotency.test
>     Idempotency test is only relevant for row mode.
>     Removing explicit test of row format.
>   mysql-test/suite/rpl/t/rpl_mix_insert_delayed.test
>     Version of rpl_stm_insert_delayed to test mixed mode.
>   mysql-test/suite/rpl/t/rpl_rbr_to_sbr.test
>     Removing explicit set of mixed mode and turning test into a mixed-mode only
> test.
>   mysql-test/suite/rpl/t/rpl_row_max_relay_size.test
>     Test only relevant for row mode, but requirement was missing.
>   mysql-test/suite/rpl/t/rpl_slave_skip.test
>     Test only relevant for mixed mode.
>   mysql-test/suite/rpl/t/rpl_stm_insert_delayed.test
>     Removing part of test that is only relevant for mixed mode and
>     putting into separate file.
>   mysql-test/suite/rpl/t/rpl_switch_stm_row_mixed.test
>     Test only relevant for mixed or row format.
>   mysql-test/suite/rpl/t/rpl_temporary_errors.test
>     Removing explicit set of row format and turning test into
>     row-mode only test.
>   mysql-test/suite/rpl/t/rpl_truncate_2myisam.test
>     Test only relevant for row mode.
>   mysql-test/suite/rpl/t/rpl_truncate_3innodb.test
>     Test only relevant for row mode.
>   mysql-test/suite/rpl/t/rpl_udf.test
>     Removing explicit set of binlog format and removing duplicate testing.
>   mysql-test/suite/rpl_ndb/combinations
>     Adding missing combinations file.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_2innodb.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_2myisam.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_basic.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_blob.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_blob2.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_commit_afterflush.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_ctype_ucs2_def.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_delete_nowhere.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_do_db.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_do_table.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_func003.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_innodb_trans.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_insert_ignore.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_multi_update3.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_rep_ignore.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_row_001.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_sp003.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_sp006.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_transaction.test
>     Test only relevant for mixed or row mode.
>   mysql-test/suite/rpl_ndb/t/rpl_ndb_trig004.test
>     Test only relevant for mixed or row mode.
>   sql/log_event.cc
>     Removing code to switch to row format when in statement mode.
>     Adding code to mark statement executing Rows_log_event as a
>     row injection.
>   sql/share/errmsg.txt
>     Changing error message for ER_BINLOG_UNSAFE_STATEMENT to support more
>     informative format.
>   sql/sql_base.cc
>     Changing logic and adding more informative error messages when logging
>     is not possible. Moving code to switch to row format for a row injection
>     here (it was in log_event.cc before, but now all switching to row format
>     is here).
>   sql/sql_class.cc
>     Doxygenifying comment to THD::binlog_query() and adding code to print
>     warnings when unsafe statements are logged as statements because either
>     the binlog format is STATEMENT or the engines are not capable of row-
>     based logging.
>   sql/sql_class.h
>     Adding THD::m_flags_all_set field with getter and setter.
>   sql/sql_lex.h
>     Adding flags and functions to mark a statement as a row injection.
> === modified file 'mysql-test/extra/rpl_tests/rpl_foreign_key.test'
> --- a/mysql-test/extra/rpl_tests/rpl_foreign_key.test	2007-12-12 10:14:59 +0000
> +++ b/mysql-test/extra/rpl_tests/rpl_foreign_key.test	2008-11-17 10:34:29 +0000
> @@ -47,7 +47,6 @@ insert into t1 set b=1;
>  insert into t2 set a=1, b=1;
>  
>  set foreign_key_checks=0;
> -set @@session.binlog_format=row;
>  delete from t1;
>  
>  --echo must sync w/o a problem (could not with the buggy code)
> 
> === modified file 'mysql-test/extra/rpl_tests/rpl_insert_delayed.test'
> --- a/mysql-test/extra/rpl_tests/rpl_insert_delayed.test	2007-07-27 14:29:48 +0000
> +++ b/mysql-test/extra/rpl_tests/rpl_insert_delayed.test	2008-11-17 10:34:29 +0000
> @@ -108,6 +108,7 @@ if  (`SELECT @@global.binlog_format != '
>  {
>    #must show two INSERT DELAYED
>    --replace_column 1 x 2 x 3 x 4 x 5 x
> +  --replace_regex /table_id: [0-9]+/table_id: #/
>    show binlog events in 'master-bin.000002' LIMIT 2,2;
>  }
>  select * from t1;
> @@ -118,6 +119,7 @@ if  (`SELECT @@global.binlog_format != '
>  {
>    #must show two INSERT DELAYED
>    --replace_column 1 x 2 x 3 x 4 x 5 x
> +  --replace_regex /table_id: [0-9]+/table_id: #/
>    show binlog events in 'slave-bin.000002' LIMIT 2,2;
>  }
>  select * from t1;
> 
> === modified file 'mysql-test/extra/rpl_tests/rpl_truncate.test'
> --- a/mysql-test/extra/rpl_tests/rpl_truncate.test	2006-12-07 14:18:35 +0000
> +++ b/mysql-test/extra/rpl_tests/rpl_truncate.test	2008-11-17 10:34:29 +0000
> @@ -9,27 +9,9 @@
>  
>  --source include/master-slave.inc
>  
> -let $format = STATEMENT;
>  let $stmt   = TRUNCATE TABLE;
>  --source extra/rpl_tests/rpl_truncate_helper.test
>  
> -let $format = MIXED;
> -let $stmt   = TRUNCATE TABLE;
> ---source extra/rpl_tests/rpl_truncate_helper.test
> -
> -let $format = ROW;
> -let $stmt   = TRUNCATE TABLE;
> ---source extra/rpl_tests/rpl_truncate_helper.test
> -
> -let $format = STATEMENT;
> -let $stmt   = DELETE FROM;
> ---source extra/rpl_tests/rpl_truncate_helper.test
> -
> -let $format = MIXED;
> -let $stmt   = DELETE FROM;
> ---source extra/rpl_tests/rpl_truncate_helper.test
> -
> -let $format = ROW;
>  let $stmt   = DELETE FROM;
>  --source extra/rpl_tests/rpl_truncate_helper.test
>  
> 
> === modified file 'mysql-test/extra/rpl_tests/rpl_truncate_helper.test'
> --- a/mysql-test/extra/rpl_tests/rpl_truncate_helper.test	2007-11-02 12:00:38 +0000
> +++ b/mysql-test/extra/rpl_tests/rpl_truncate_helper.test	2008-11-17 10:34:29 +0000
> @@ -14,9 +14,6 @@ START SLAVE;
>  
>  --echo **** On Master ****
>  connection master;
> -eval SET SESSION BINLOG_FORMAT=$format;
> -eval SET GLOBAL  BINLOG_FORMAT=$format;
> -
>  eval CREATE TABLE t1 (a INT, b LONG) ENGINE=$engine;
>  INSERT INTO t1 VALUES (1,1), (2,2);
>  SELECT * FROM t1;
> @@ -35,8 +32,6 @@ SELECT * FROM t1;
>  --echo **** On Master ****
>  connection master;
>  DROP TABLE t1;
> -let $SERVER_VERSION=`select version()`;
> -source include/show_binlog_events.inc;
>  
>  connection master;
>  RESET MASTER;
> 
> === modified file 'mysql-test/include/rpl_udf.inc'
> --- a/mysql-test/include/rpl_udf.inc	2007-08-29 21:36:08 +0000
> +++ b/mysql-test/include/rpl_udf.inc	2008-11-17 10:34:29 +0000
> @@ -54,12 +54,14 @@ connection master;
>  # Use the UDFs to do something
>  --echo "Running on the master"
>  --enable_info
> +--disable_warnings
>  eval CREATE TABLE t1(sum INT, price FLOAT(24)) ENGINE=$engine_type;
>  INSERT INTO t1 VALUES(myfunc_int(100), myfunc_double(50.00));
>  INSERT INTO t1 VALUES(myfunc_int(10), myfunc_double(5.00));
>  INSERT INTO t1 VALUES(myfunc_int(200), myfunc_double(25.00));
>  INSERT INTO t1 VALUES(myfunc_int(1), myfunc_double(500.00));
>  SELECT * FROM t1 ORDER BY sum;
> +--enable_warnings
>  --disable_info
>  
>  sync_slave_with_master;
> 
> === modified file 'mysql-test/lib/mtr_report.pl'
> --- a/mysql-test/lib/mtr_report.pl	2008-10-02 07:46:14 +0000
> +++ b/mysql-test/lib/mtr_report.pl	2008-11-17 10:34:29 +0000
> @@ -382,6 +382,13 @@ sub mtr_report_stats ($) {
>                    /Slave: Cannot add or update a child row: a foreign key constraint
> fails .* Error_code: 1452/
>  		 )) or
>  
> +                # These tests generate warnings since they write
> +                # unsafe statements to the binary log
> +                (($testname eq 'rpl.rpl_udf' or
> +                  $testname eq 'rpl.rpl_user_variables') and
> +                  /Unsafe statement binlogged as statement/
> +                ) or
> +
>  		# These tests does "kill" on queries, causing sporadic errors when writing to
> logs
>  		(($testname eq 'rpl.rpl_skip_error' or
>  		  $testname eq 'rpl.rpl_err_ignoredtable' or
> 
> === modified file 'mysql-test/suite/binlog/r/binlog_stm_ps.result'
> --- a/mysql-test/suite/binlog/r/binlog_stm_ps.result	2008-03-25 13:28:12 +0000
> +++ b/mysql-test/suite/binlog/r/binlog_stm_ps.result	2008-11-17 10:34:29 +0000
[...]
> === modified file 'mysql-test/suite/binlog/r/binlog_unsafe.result'
> --- a/mysql-test/suite/binlog/r/binlog_unsafe.result	2008-03-26 09:56:03 +0000
> +++ b/mysql-test/suite/binlog/r/binlog_unsafe.result	2008-11-17 10:34:29 +0000
[...]
> === modified file 'mysql-test/suite/rpl/r/rpl_events.result'
> --- a/mysql-test/suite/rpl/r/rpl_events.result	2008-04-30 18:41:50 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_events.result	2008-11-17 10:34:29 +0000
[...]
> === modified file 'mysql-test/suite/rpl/r/rpl_extraColmaster_innodb.result'
> --- a/mysql-test/suite/rpl/r/rpl_extraColmaster_innodb.result	2008-01-11 17:39:45
> +0000
> +++ b/mysql-test/suite/rpl/r/rpl_extraColmaster_innodb.result	2008-11-17 10:34:29
> +0000
[...]
> === modified file 'mysql-test/suite/rpl/r/rpl_extraColmaster_myisam.result'
> --- a/mysql-test/suite/rpl/r/rpl_extraColmaster_myisam.result	2008-01-11 17:39:45
> +0000
> +++ b/mysql-test/suite/rpl/r/rpl_extraColmaster_myisam.result	2008-11-17 10:34:29
> +0000
[...]
> === modified file 'mysql-test/suite/rpl/r/rpl_foreign_key_innodb.result'
> --- a/mysql-test/suite/rpl/r/rpl_foreign_key_innodb.result	2007-12-12 10:14:59 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_foreign_key_innodb.result	2008-11-17 10:34:29 +0000
[...]
> === modified file 'mysql-test/suite/rpl/r/rpl_found_rows.result'
> --- a/mysql-test/suite/rpl/r/rpl_found_rows.result	2008-09-02 09:14:13 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_found_rows.result	2008-11-17 10:34:29 +0000
[...]
> === added file 'mysql-test/suite/rpl/r/rpl_found_rows_mixed.result'
> --- a/mysql-test/suite/rpl/r/rpl_found_rows_mixed.result	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_found_rows_mixed.result	2008-11-17 10:34:29 +0000
[...]
> === modified file 'mysql-test/suite/rpl/r/rpl_idempotency.result'
> --- a/mysql-test/suite/rpl/r/rpl_idempotency.result	2007-12-12 10:14:59 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_idempotency.result	2008-11-17 10:34:29 +0000
[...]
> === added file 'mysql-test/suite/rpl/r/rpl_mix_insert_delayed.result'
> --- a/mysql-test/suite/rpl/r/rpl_mix_insert_delayed.result	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_mix_insert_delayed.result	2008-11-17 10:34:29 +0000
[...]
> === modified file 'mysql-test/suite/rpl/r/rpl_rbr_to_sbr.result'
> --- a/mysql-test/suite/rpl/r/rpl_rbr_to_sbr.result	2008-03-28 12:16:41 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_rbr_to_sbr.result	2008-11-17 10:34:29 +0000
[...]
> === modified file 'mysql-test/suite/rpl/r/rpl_row_max_relay_size.result'
> --- a/mysql-test/suite/rpl/r/rpl_row_max_relay_size.result	2008-01-14 07:38:02 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_row_max_relay_size.result	2008-11-17 10:34:29 +0000
[...]
> === modified file 'mysql-test/suite/rpl/r/rpl_skip_error.result'
> --- a/mysql-test/suite/rpl/r/rpl_skip_error.result	2008-03-14 16:52:57 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_skip_error.result	2008-11-17 10:34:29 +0000
[...]
> === modified file 'mysql-test/suite/rpl/r/rpl_stm_insert_delayed.result'
> --- a/mysql-test/suite/rpl/r/rpl_stm_insert_delayed.result	2007-07-27 11:28:36 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_stm_insert_delayed.result	2008-11-17 10:34:29 +0000
[...]
> === modified file 'mysql-test/suite/rpl/r/rpl_temporary_errors.result'
> --- a/mysql-test/suite/rpl/r/rpl_temporary_errors.result	2008-01-14 07:38:02 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_temporary_errors.result	2008-11-17 10:34:29 +0000
[...]
> === modified file 'mysql-test/suite/rpl/r/rpl_truncate_2myisam.result'
> --- a/mysql-test/suite/rpl/r/rpl_truncate_2myisam.result	2008-03-28 12:16:41 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_truncate_2myisam.result	2008-11-17 10:34:29 +0000
[...]
> === modified file 'mysql-test/suite/rpl/r/rpl_truncate_3innodb.result'
> --- a/mysql-test/suite/rpl/r/rpl_truncate_3innodb.result	2007-12-14 13:40:45 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_truncate_3innodb.result	2008-11-17 10:34:29 +0000
[...]
> === modified file 'mysql-test/suite/rpl/r/rpl_udf.result'
> --- a/mysql-test/suite/rpl/r/rpl_udf.result	2007-10-26 20:40:48 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_udf.result	2008-11-17 10:34:29 +0000
[...]
> === modified file 'mysql-test/suite/rpl/t/rpl_bug31076.test'
> --- a/mysql-test/suite/rpl/t/rpl_bug31076.test	2007-12-14 18:02:02 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_bug31076.test	2008-11-17 10:34:29 +0000
> @@ -1,4 +1,5 @@
>  source include/master-slave.inc;
> +source include/have_binlog_format_mixed_or_row.inc;
>  
>  CREATE DATABASE track;
>  USE track;
> 
> === modified file 'mysql-test/suite/rpl/t/rpl_events.test'
> --- a/mysql-test/suite/rpl/t/rpl_events.test	2008-02-14 09:53:12 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_events.test	2008-11-17 10:34:29 +0000
> @@ -7,19 +7,12 @@
>  
>  set global event_scheduler=1;
>  
> +# Embedded server doesn't support binlogging
>  --source include/not_embedded.inc

I think you can remove the above line. master-slave.inc does 'source
include/have_log_bin.inc', so all replication tests are skipped in
embedded mode because of that.

>  --source include/master-slave.inc
>  
>  let $engine_type= MyISAM;
>  
> -set binlog_format=row;
> -
> -# Embedded server doesn't support binlogging
> ---source include/rpl_events.inc
> -
> -set binlog_format=statement;
> -
> -# Embedded server doesn't support binlogging
>  --source include/rpl_events.inc
>  
>  #
> 
> === modified file 'mysql-test/suite/rpl/t/rpl_extraColmaster_innodb.test'
> --- a/mysql-test/suite/rpl/t/rpl_extraColmaster_innodb.test	2007-10-10 14:43:20
> +0000
> +++ b/mysql-test/suite/rpl/t/rpl_extraColmaster_innodb.test	2008-11-17 10:34:29
> +0000
> @@ -3,14 +3,7 @@
>  #############################################################
>  -- source include/master-slave.inc
>  -- source include/have_innodb.inc
> +source include/have_binlog_format_row.inc;
>  
>  let $engine_type = 'InnoDB';
> -
> -set binlog_format=row;
> --- source extra/rpl_tests/rpl_extraMaster_Col.test
> -
> -set binlog_format=statement;
> --- source extra/rpl_tests/rpl_extraMaster_Col.test
> -
> -set binlog_format=mixed;
> --- source extra/rpl_tests/rpl_extraMaster_Col.test
> +--source extra/rpl_tests/rpl_extraMaster_Col.test
> 
> === modified file 'mysql-test/suite/rpl/t/rpl_extraColmaster_myisam.test'
> --- a/mysql-test/suite/rpl/t/rpl_extraColmaster_myisam.test	2007-10-10 14:43:20
> +0000
> +++ b/mysql-test/suite/rpl/t/rpl_extraColmaster_myisam.test	2008-11-17 10:34:29
> +0000
> @@ -2,14 +2,7 @@
>  # Purpose: To test having extra columns on the master WL#3915
>  #############################################################
>  -- source include/master-slave.inc
> +source include/have_binlog_format_row.inc;
>  
>  let $engine_type = 'MyISAM';
> -
> -set binlog_format=row;
> --- source extra/rpl_tests/rpl_extraMaster_Col.test
> -
> -set binlog_format=statement;
> --- source extra/rpl_tests/rpl_extraMaster_Col.test
> -
> -set binlog_format=mixed;
> --- source extra/rpl_tests/rpl_extraMaster_Col.test
> +--source extra/rpl_tests/rpl_extraMaster_Col.test
> 
> === modified file 'mysql-test/suite/rpl/t/rpl_found_rows.test'
> --- a/mysql-test/suite/rpl/t/rpl_found_rows.test	2008-09-02 09:14:13 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_found_rows.test	2008-11-17 10:34:29 +0000
> @@ -110,147 +110,8 @@ SELECT * FROM logtbl WHERE sect = 3 ORDE
>  --echo ==== 1.9. Cleanup ====
>  --echo **** On Master ****
>  connection master;
> -DELETE FROM logtbl;
> +DROP TABLE t1, logtbl;
>  DROP PROCEDURE just_log;
>  DROP PROCEDURE calc_and_log;
>  DROP FUNCTION log_rows;
>  sync_slave_with_master;
[... removed rest of file...]


This is now a STATEMENT-only test? Please rename it to
rpl_stm_found_rows.test, and do "source
include/have_binlog_format_statement.inc" instead of "SET
BINLOG_FORMAT=STATEMENT" at the top of the test.

> === added file 'mysql-test/suite/rpl/t/rpl_found_rows_mixed.test'
> --- a/mysql-test/suite/rpl/t/rpl_found_rows_mixed.test	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_found_rows_mixed.test	2008-11-17 10:34:29 +0000

Please use the naming convention of most other rpl tests:
rpl_mix_found_rows.test

> @@ -0,0 +1,151 @@
> +source include/master-slave.inc;
> +source include/have_binlog_format_mixed.inc;
> +
> +# The purpose of this test case is to test that FOUND_ROWS() can be
> +# replicated properly under MIXED mode (on both master and slave).
> +
> +--echo #### 2. Using mixed mode ####

I think you can remove the number "2" now (in all the following headers
too).

> +
> +--echo ==== 2.1. Checking a procedure ====
> +
> +--echo **** On Master ****
> +connection master;
> +CREATE TABLE t1 (a INT);
> +CREATE TABLE logtbl (sect INT, test INT, count INT);
> +
> +INSERT INTO t1 VALUES (1),(2),(3);
> +INSERT INTO t1 SELECT 2*a+3 FROM t1;
> +INSERT INTO t1 SELECT 2*a+3 FROM t1;
> +INSERT INTO t1 SELECT 2*a+3 FROM t1;
> +INSERT INTO t1 SELECT 2*a+3 FROM t1;
> +INSERT INTO t1 SELECT 2*a+3 FROM t1;
> +INSERT INTO t1 SELECT 2*a+3 FROM t1;
> +
> +# We will now check some stuff that will not work in statement-based
> +# replication, but which should cause the binary log to switch to
> +# row-based logging.
> +
> +--delimiter $$
> +CREATE PROCEDURE just_log(sect INT, test INT) BEGIN
> +  INSERT INTO logtbl VALUES (sect,test,FOUND_ROWS());
> +END $$
> +--delimiter ;
> +sync_slave_with_master;
> +
> +--echo **** On Master 1 ****
> +connection master1;
> +SET BINLOG_FORMAT=MIXED;
> +SELECT SQL_CALC_FOUND_ROWS * FROM t1 WHERE a > 5 ORDER BY a LIMIT 1;
> +CALL just_log(1,1);
> +
> +--echo **** On Master ****
> +connection master;
> +SELECT SQL_CALC_FOUND_ROWS * FROM t1 WHERE a > 5 ORDER BY a LIMIT 1;
> +CALL just_log(1,2);
> +
> +--echo **** On Master 1 ****
> +
> +connection master1;
> +SELECT SQL_CALC_FOUND_ROWS * FROM t1 WHERE a < 5 ORDER BY a LIMIT 1;
> +CALL just_log(1,3);
> +sync_slave_with_master;
> +
> +--echo **** On Master ****
> +connection master;
> +SELECT SQL_CALC_FOUND_ROWS * FROM t1 WHERE a > 5 ORDER BY a LIMIT 1;
> +CALL just_log(1,4);
> +sync_slave_with_master;
> +
> +connection master;
> +SELECT * FROM logtbl WHERE sect = 1 ORDER BY sect,test;
> +--echo **** On Slave ****
> +sync_slave_with_master;
> +SELECT * FROM logtbl WHERE sect = 1 ORDER BY sect,test;
> +
> +--echo ==== 2.1. Checking a stored function ====
> +--echo **** On Master ****
> +connection master;
> +--delimiter $$
> +CREATE FUNCTION log_rows(sect INT, test INT)
> +  RETURNS INT
> +BEGIN
> +  DECLARE found_rows INT;
> +  SELECT FOUND_ROWS() INTO found_rows;
> +  INSERT INTO logtbl VALUES(sect,test,found_rows);
> +  RETURN found_rows;
> +END $$
> +--delimiter ;
> +
> +SELECT SQL_CALC_FOUND_ROWS * FROM t1 WHERE a < 5 ORDER BY a LIMIT 1;
> +SELECT log_rows(2,1), log_rows(2,2);
> +
> +CREATE TABLE t2 (a INT, b INT);
> +
> +# Trying with referencing FOUND_ROWS() directly in the trigger.
> +
> +--delimiter $$
> +CREATE TRIGGER t2_tr BEFORE INSERT ON t2 FOR EACH ROW
> +BEGIN
> +  INSERT INTO logtbl VALUES (NEW.a, NEW.b, FOUND_ROWS());
> +END $$
> +--delimiter ;
> +
> +SELECT SQL_CALC_FOUND_ROWS * FROM t1 WHERE a < 5 ORDER BY a LIMIT 1;
> +INSERT INTO t2 VALUES (2,3), (2,4);
> +
> +# Referencing FOUND_ROWS() indirectly.
> +
> +DROP TRIGGER t2_tr;
> +
> +--delimiter $$
> +CREATE TRIGGER t2_tr BEFORE INSERT ON t2 FOR EACH ROW
> +BEGIN
> +  DECLARE dummy INT;
> +  SELECT log_rows(NEW.a, NEW.b) INTO dummy;
> +END $$
> +--delimiter ;
> +
> +SELECT SQL_CALC_FOUND_ROWS * FROM t1 WHERE a > 5 ORDER BY a LIMIT 1;
> +INSERT INTO t2 VALUES (2,5), (2,6);
> +
> +# Putting FOUND_ROWS() even lower in the call chain.
> +
> +connection master;
> +DROP TRIGGER t2_tr;
> +
> +--delimiter $$
> +CREATE PROCEDURE log_me_inner(sect INT, test INT)
> +BEGIN
> +  DECLARE dummy INT;
> +  SELECT log_rows(sect, test) INTO dummy;
> +  SELECT log_rows(sect, test+1) INTO dummy;
> +END $$
> +
> +CREATE PROCEDURE log_me(sect INT, test INT)
> +BEGIN
> +  CALL log_me_inner(sect,test);
> +END $$
> +--delimiter ;
> +
> +--delimiter $$
> +CREATE TRIGGER t2_tr BEFORE INSERT ON t2 FOR EACH ROW
> +BEGIN
> +  CALL log_me(NEW.a, NEW.b);
> +END $$
> +--delimiter ;
> +
> +SELECT SQL_CALC_FOUND_ROWS * FROM t1 WHERE a > 5 ORDER BY a LIMIT 1;
> +INSERT INTO t2 VALUES (2,5), (2,6);
> +
> +SELECT * FROM logtbl WHERE sect = 2 ORDER BY sect,test;
> +sync_slave_with_master;
> +SELECT * FROM logtbl WHERE sect = 2 ORDER BY sect,test;
> +
> +connection master;
> +DROP TABLE t1, t2, logtbl;
> +DROP PROCEDURE just_log;
> +DROP PROCEDURE log_me;
> +DROP PROCEDURE log_me_inner;
> +DROP FUNCTION log_rows;
> +sync_slave_with_master;
> +
> 
> === modified file 'mysql-test/suite/rpl/t/rpl_idempotency.test'
> --- a/mysql-test/suite/rpl/t/rpl_idempotency.test	2008-02-04 07:43:22 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_idempotency.test	2008-11-17 10:34:29 +0000
> @@ -102,6 +102,8 @@ sync_slave_with_master;
>  # Other than above idempotent errors dealing with foreign keys constraint
>  #
>  
> +source include/have_binlog_format_row.inc;
> +
>  select @@global.slave_exec_mode /* must be IDEMPOTENT */;
>  
>  connection master;
> @@ -124,8 +126,6 @@ select * from ti2 order by b /* must be 
>  connection master;
>  
>  # from now on checking rbr specific idempotent errors
> -set @save_binlog_format= @@session.binlog_format;
> -set @@session.binlog_format= row;
>  delete from ti1 where b=1;
>  
>  select * from ti1 order by b /* must be (2),(3) */;
> @@ -397,7 +397,6 @@ set global slave_exec_mode='STRICT';
>  # cleanup for bug#31609 tests
>  
>  connection master;
> -set @@session.binlog_format= @save_binlog_format;
>  drop table t1,t2,ti2,ti1;
>  
>  sync_slave_with_master;
> 
> === added file 'mysql-test/suite/rpl/t/rpl_mix_insert_delayed.test'
> --- a/mysql-test/suite/rpl/t/rpl_mix_insert_delayed.test	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_mix_insert_delayed.test	2008-11-17 10:34:29 +0000
> @@ -0,0 +1,8 @@
> +--source include/have_binlog_format_mixed.inc
> +--source include/master-slave.inc
> +--source include/not_embedded.inc
> +--source include/not_windows.inc
> +
> +connection master;
> +let $binlog_format_statement=0;
> +--source extra/rpl_tests/rpl_insert_delayed.test
> 
> === modified file 'mysql-test/suite/rpl/t/rpl_rbr_to_sbr.test'
> --- a/mysql-test/suite/rpl/t/rpl_rbr_to_sbr.test	2007-06-27 12:29:10 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_rbr_to_sbr.test	2008-11-17 10:34:29 +0000
> @@ -1,13 +1,9 @@
> --- source include/have_binlog_format_mixed_or_statement.inc
> +-- source include/have_binlog_format_mixed.inc
>  -- source include/not_ndb_default.inc
>  -- source include/master-slave.inc
>  
>  # Test that the slave temporarily switches to ROW when seeing binrow
> -# events when it is in STATEMENT or MIXED mode
> -
> -SET BINLOG_FORMAT=MIXED;
> -SET GLOBAL BINLOG_FORMAT=MIXED;
> -SELECT @@GLOBAL.BINLOG_FORMAT, @@SESSION.BINLOG_FORMAT;
> +# events when it is in MIXED mode
>  
>  --echo **** On Master ****
>  CREATE TABLE t1 (a INT, b LONG);
> 
> === modified file 'mysql-test/suite/rpl/t/rpl_row_max_relay_size.test'
> --- a/mysql-test/suite/rpl/t/rpl_row_max_relay_size.test	2007-06-27 12:28:02 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_row_max_relay_size.test	2008-11-17 10:34:29 +0000
> @@ -5,7 +5,6 @@
>  
>  # Requires statement logging
>  -- source include/not_ndb_default.inc
> -SET SESSION BINLOG_FORMAT=ROW;
> -SET GLOBAL BINLOG_FORMAT=ROW;
> +-- source include/have_binlog_format_row.inc
>  -- source extra/rpl_tests/rpl_max_relay_size.test
>  
> 
> === modified file 'mysql-test/suite/rpl/t/rpl_slave_skip.test'
> --- a/mysql-test/suite/rpl/t/rpl_slave_skip.test	2008-03-28 12:16:41 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_slave_skip.test	2008-11-17 10:34:29 +0000
> @@ -1,6 +1,9 @@
>  source include/master-slave.inc;
>  source include/have_innodb.inc;
>  
> +# To ensure that the slave thread is running in MIXED mode.
> +source include/have_binlog_format_mixed.inc;
> +
>  --echo **** On Slave ****
>  connection slave;
>  source include/have_innodb.inc;
> 
> === modified file 'mysql-test/suite/rpl/t/rpl_stm_insert_delayed.test'
> --- a/mysql-test/suite/rpl/t/rpl_stm_insert_delayed.test	2007-06-27 12:28:02 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_stm_insert_delayed.test	2008-11-17 10:34:29 +0000
> @@ -1,20 +1,8 @@
> -# we run first in statement-based then in mixed binlogging
> -
> ---source include/have_binlog_format_mixed_or_statement.inc
> +--source include/have_binlog_format_statement.inc
>  --source include/master-slave.inc
>  --source include/not_embedded.inc
>  --source include/not_windows.inc
>  
>  connection master;
> -set @old_global_binlog_format = @@global.binlog_format;
> -
>  let $binlog_format_statement=1;
> -set @@global.binlog_format = statement;
> ---source extra/rpl_tests/rpl_insert_delayed.test
> -
> -let $binlog_format_statement=0;
> -set @@global.binlog_format = mixed;
>  --source extra/rpl_tests/rpl_insert_delayed.test
> -
> -connection master;
> -set @@global.binlog_format = @old_global_binlog_format;
> 
> === modified file 'mysql-test/suite/rpl/t/rpl_switch_stm_row_mixed.test'
> --- a/mysql-test/suite/rpl/t/rpl_switch_stm_row_mixed.test	2008-07-03 08:27:25 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_switch_stm_row_mixed.test	2008-11-17 10:34:29 +0000
> @@ -7,10 +7,14 @@
>  #     e.g a query invokes UUID(), thereafter to serve as the definition
>  #     of MIXED binlog format
>  #   - correctness of execution
> +#
> +# Since this test generates row-based events in the binary log, the
> +# slave SQL thread cannot be in STATEMENT mode to execute this test.
>  
>  
>  -- source include/not_ndb_default.inc
>  -- source include/master-slave.inc
> +-- source include/have_binlog_format_mixed_or_row.inc
>  
>  connection master;
>  --disable_warnings
> 
> === modified file 'mysql-test/suite/rpl/t/rpl_temporary_errors.test'
> --- a/mysql-test/suite/rpl/t/rpl_temporary_errors.test	2008-01-14 09:54:23 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_temporary_errors.test	2008-11-17 10:34:29 +0000
> @@ -1,8 +1,8 @@
>  source include/master-slave.inc;
> +source include/have_binlog_format_row.inc;
>  
>  --echo **** On Master ****
>  connection master;
> -SET SESSION BINLOG_FORMAT=ROW;
>  CREATE TABLE t1 (a INT PRIMARY KEY, b INT);
>  INSERT INTO t1 VALUES (1,1), (2,2), (3,3), (4,4);
>  --echo **** On Slave ****
> 
> === modified file 'mysql-test/suite/rpl/t/rpl_truncate_2myisam.test'
> --- a/mysql-test/suite/rpl/t/rpl_truncate_2myisam.test	2007-06-27 12:28:02 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_truncate_2myisam.test	2008-11-17 10:34:29 +0000
> @@ -1,4 +1,5 @@
>  --source include/not_ndb_default.inc
> +--source include/have_binlog_format_row.inc
>  let $engine=MyISAM;
>  --source extra/rpl_tests/rpl_truncate.test
>  
> 
> === modified file 'mysql-test/suite/rpl/t/rpl_truncate_3innodb.test'
> --- a/mysql-test/suite/rpl/t/rpl_truncate_3innodb.test	2007-06-27 12:28:02 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_truncate_3innodb.test	2008-11-17 10:34:29 +0000
> @@ -1,6 +1,6 @@
> -
>  --source include/have_innodb.inc
>  --source include/not_ndb_default.inc
> +--source include/have_binlog_format_row.inc
>  
>  let $engine=InnoDB;
>  --source extra/rpl_tests/rpl_truncate.test
> 
> === modified file 'mysql-test/suite/rpl/t/rpl_udf.test'
> --- a/mysql-test/suite/rpl/t/rpl_udf.test	2007-06-27 12:28:02 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_udf.test	2008-11-17 10:34:29 +0000
> @@ -5,18 +5,9 @@
>  # statement based format. This tests work completed in WL#3629.   #
>  ###################################################################
>  
> +# Embedded server doesn't support binlogging
>  --source include/not_embedded.inc

Same as above, I think you can remove this since all rpl tests are
skipped in embedded mode.

>  --source include/master-slave.inc
>  
>  let $engine_type= MyISAM;
> -
> -set binlog_format=row;
> -
> -# Embedded server doesn't support binlogging
>  --source include/rpl_udf.inc
> -
> -set binlog_format=statement;
> -
> -# Embedded server doesn't support binlogging
> ---source include/rpl_udf.inc
> -
> 
> === added file 'mysql-test/suite/rpl_ndb/combinations'
> --- a/mysql-test/suite/rpl_ndb/combinations	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/rpl_ndb/combinations	2008-11-17 10:34:29 +0000
> @@ -0,0 +1,8 @@
> +[stmt]
> +--binlog-format=statement
> +
> +[mix]
> +--binlog-format=mixed
> +
> +[row]
> +--binlog-format=row
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_2innodb.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_2innodb.test	2008-02-11 14:26:38 +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_2innodb.test	2008-11-17 10:34:29 +0000
> @@ -6,9 +6,14 @@
>  # cluster slave. Due to limitations I have created wrappers
>  # to be able to use the same code for all these different
>  # test and to have control over the tests.
> +#
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +#
>  ##############################################################
>  -- source include/have_ndb.inc
>  -- source include/master-slave.inc
> +-- source include/have_binlog_format_mixed_or_row.inc
>  
>  -- connection slave
>  -- source include/have_innodb.inc
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_2myisam.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_2myisam.test	2008-02-11 14:26:38 +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_2myisam.test	2008-11-17 10:34:29 +0000
> @@ -6,9 +6,14 @@
>  # cluster slave. Due to limitations I have created wrappers
>  # to be able to use the same code for all these different
>  # test and to have control over the tests.
> +#
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +#
>  ##############################################################
>  -- source include/have_ndb.inc
>  -- source include/master-slave.inc
> +-- source include/have_binlog_format_mixed_or_row.inc
>  
>  -- connection slave
>  set @@global.slave_exec_mode= 'IDEMPOTENT';
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_basic.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_basic.test	2008-03-10 15:24:12 +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_basic.test	2008-11-17 10:34:29 +0000
> @@ -1,7 +1,9 @@
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +
>  --source include/have_ndb.inc
>  --source include/ndb_master-slave.inc
> -
> -
> +--source include/have_binlog_format_mixed_or_row.inc
>  
>  #
>  # Bug #11087
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_blob.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_blob.test	2007-09-05 16:44:47 +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_blob.test	2008-11-17 10:34:29 +0000
> @@ -1,9 +1,12 @@
>  --source include/have_ndb.inc
>  --source include/ndb_master-slave.inc
> +--source include/have_binlog_format_mixed_or_row.inc
>  
>  #
>  # basic test of blob replication for NDB
>  #
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
>  
>  # easy test
>  
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_blob2.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_blob2.test	2007-09-05 16:44:47 +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_blob2.test	2008-11-17 10:34:29 +0000
> @@ -3,8 +3,13 @@
>  # Using wrapper to share test   #
>  # code between engine tests     #
>  #################################
> +#
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +#
>  -- source include/have_ndb.inc
>  -- source include/ndb_master-slave.inc
>  let $engine_type=NDBCLUSTER;
>  -- source extra/rpl_tests/rpl_row_blob.test
> +-- source include/have_binlog_format_mixed_or_row.inc
>  
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_commit_afterflush.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_commit_afterflush.test	2007-09-05 16:44:47
> +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_commit_afterflush.test	2008-11-17 10:34:29
> +0000
> @@ -4,7 +4,12 @@
>  # Different engines                 #
>  # By JBM 2004-02-15                 #
>  #####################################
> +
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +
>  -- source include/have_ndb.inc
>  -- source include/ndb_master-slave.inc
>  let $engine_type=NDB;
>  -- source extra/rpl_tests/rpl_commit_after_flush.test
> +-- source include/have_binlog_format_mixed_or_row.inc
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_ctype_ucs2_def.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_ctype_ucs2_def.test	2007-07-04 21:41:15
> +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_ctype_ucs2_def.test	2008-11-17 10:34:29
> +0000
> @@ -2,6 +2,11 @@
>  --source include/have_ndb.inc
>  --source include/ndb_master-slave.inc
>  
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +
> +--source include/have_binlog_format_mixed_or_row.inc
> +
>  #
>  # MySQL Bug#15276: MySQL ignores collation-server
>  #
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_delete_nowhere.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_delete_nowhere.test	2007-09-05 16:44:47
> +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_delete_nowhere.test	2008-11-17 10:34:29
> +0000
> @@ -4,5 +4,11 @@
>  #########################################
>  --source include/have_ndb.inc
>  -- source include/ndb_master-slave.inc
> +
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +
> +--source include/have_binlog_format_mixed_or_row.inc
> +
>  let $engine_type=NDB;
>  -- source extra/rpl_tests/rpl_delete_no_where.test
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_do_db.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_do_db.test	2007-09-05 16:44:47 +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_do_db.test	2008-11-17 10:34:29 +0000
> @@ -8,6 +8,11 @@
>  --source include/have_ndb.inc
>  --source include/ndb_master-slave.inc
>  
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +
> +--source include/have_binlog_format_mixed_or_row.inc
> +
>  --disable_warnings
>  DROP DATABASE IF EXISTS replica;
>  --enable_warnings
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_do_table.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_do_table.test	2007-09-05 16:44:47 +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_do_table.test	2008-11-17 10:34:29 +0000
> @@ -8,6 +8,11 @@
>  --source include/have_ndb.inc
>  --source include/ndb_master-slave.inc
>  
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +
> +--source include/have_binlog_format_mixed_or_row.inc
> +
>  --disable_warnings
>  DROP TABLE IF EXISTS t1, t2;
>  --enable_warnings
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_func003.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_func003.test	2007-09-05 16:44:47 +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_func003.test	2008-11-17 10:34:29 +0000
> @@ -8,6 +8,13 @@
>  ###################################
>  -- source include/have_ndb.inc
>  -- source include/ndb_master-slave.inc
> +
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +
> +-- source include/have_binlog_format_mixed_or_row.inc
> +
>  let $engine_type=NDB;
>  -- source extra/rpl_tests/rpl_row_func003.test
>  -- source include/master-slave-end.inc
> +
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_innodb_trans.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_innodb_trans.test	2007-09-05 16:44:47 +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_innodb_trans.test	2008-11-17 10:34:29 +0000
> @@ -4,6 +4,11 @@
>  -- source include/have_innodb.inc
>  -- source include/ndb_master-slave.inc
>  
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +
> +-- source include/have_binlog_format_mixed_or_row.inc
> +
>  create table t1 (a int, unique(a)) engine=ndbcluster;
>  create table t2 (a int, unique(a)) engine=innodb;
>  
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_insert_ignore.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_insert_ignore.test	2007-09-05 16:44:47
> +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_insert_ignore.test	2008-11-17 10:34:29
> +0000
> @@ -3,6 +3,12 @@
>  #####################################
>  -- source include/have_ndb.inc
>  -- source include/ndb_master-slave.inc
> +
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +
> +-- source include/have_binlog_format_mixed_or_row.inc
> +
>  let $engine_type=NDB;
>  let $engine_type2=myisam;
>  -- source extra/rpl_tests/rpl_insert_ignore.test
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_multi_update3.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_multi_update3.test	2007-09-05 16:44:47
> +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_multi_update3.test	2008-11-17 10:34:29
> +0000
> @@ -4,5 +4,11 @@
>  ############################################################
>  -- source include/have_ndb.inc
>  -- source include/ndb_master-slave.inc
> +
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +
> +-- source include/have_binlog_format_mixed_or_row.inc
> +
>  let $engine_type=NDB;
>  -- source extra/rpl_tests/rpl_multi_update3.test
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_rep_ignore.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_rep_ignore.test	2007-09-05 16:44:47 +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_rep_ignore.test	2008-11-17 10:34:29 +0000
> @@ -9,6 +9,11 @@
>  --source include/have_ndb.inc
>  --source include/ndb_master-slave.inc
>  
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +
> +-- source include/have_binlog_format_mixed_or_row.inc
> +
>  --disable_warnings
>  DROP DATABASE IF EXISTS replica;
>  --enable_warnings
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_row_001.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_row_001.test	2007-09-05 16:44:47 +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_row_001.test	2008-11-17 10:34:29 +0000
> @@ -3,5 +3,11 @@
>  ########################################################
>  --source include/have_ndb.inc
>  -- source include/ndb_master-slave.inc
> +
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +
> +-- source include/have_binlog_format_mixed_or_row.inc
> +
>  let $engine_type=NDB;
>  -- source extra/rpl_tests/rpl_row_001.test
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_sp003.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_sp003.test	2007-09-05 16:44:47 +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_sp003.test	2008-11-17 10:34:29 +0000
> @@ -6,5 +6,11 @@
>  #################################
>  -- source include/have_ndb.inc
>  -- source include/ndb_master-slave.inc
> +
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +
> +-- source include/have_binlog_format_mixed_or_row.inc
> +
>  let $engine_type=NDBCLUSTER;
>  -- source extra/rpl_tests/rpl_row_sp003.test
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_sp006.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_sp006.test	2007-09-05 16:44:47 +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_sp006.test	2008-11-17 10:34:29 +0000
> @@ -6,5 +6,11 @@
>  #################################
>  -- source include/have_ndb.inc
>  -- source include/ndb_master-slave.inc
> +
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +
> +-- source include/have_binlog_format_mixed_or_row.inc
> +
>  let $engine_type=NDBCLUSTER;
>  -- source extra/rpl_tests/rpl_row_sp006.test
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_transaction.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_transaction.test	2008-03-14 16:45:14 +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_transaction.test	2008-11-17 10:34:29 +0000
> @@ -8,6 +8,11 @@ source include/ndb_master-slave.inc;
>  source include/have_ndb.inc;
>  source include/have_debug.inc;
>  
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +
> +-- source include/have_binlog_format_mixed_or_row.inc
> +
>  CREATE TABLE tmyisam (a int) ENGINE = MYISAM;
>  CREATE TABLE tinnodb (a int) ENGINE = INNODB;
>  CREATE TABLE tndb (a int) ENGINE = NDB;
> 
> === modified file 'mysql-test/suite/rpl_ndb/t/rpl_ndb_trig004.test'
> --- a/mysql-test/suite/rpl_ndb/t/rpl_ndb_trig004.test	2007-09-05 16:44:47 +0000
> +++ b/mysql-test/suite/rpl_ndb/t/rpl_ndb_trig004.test	2008-11-17 10:34:29 +0000
> @@ -9,6 +9,12 @@
>  # Includes
>  -- source include/have_ndb.inc
>  -- source include/ndb_master-slave.inc
> +
> +# Since the master generates row-based events, the slave may not be in
> +# STATEMENT mode to accept the events.
> +
> +-- source include/have_binlog_format_mixed_or_row.inc
> +
>  let $engine_type=NDB;
>  -- source extra/rpl_tests/rpl_trig004.test
>  
> 
> === modified file 'sql/log_event.cc'
> --- a/sql/log_event.cc	2008-10-07 13:21:17 +0000
> +++ b/sql/log_event.cc	2008-11-17 10:34:29 +0000
> @@ -7039,16 +7039,12 @@ int Rows_log_event::do_apply_event(Relay
>      mysql_reset_thd_for_next_command(thd);
>  
>      /*
> -      Check if the slave is set to use SBR.  If so, it should switch
> -      to using RBR until the end of the "statement", i.e., next
> -      STMT_END_F or next error.
> -    */
> -    if (!thd->current_stmt_binlog_row_based &&
> -        mysql_bin_log.is_open() && (thd->options & OPTION_BIN_LOG))
> -    {
> -      thd->set_current_stmt_binlog_row_based();
> -    }
> -
> +      This is a row injection, so we flag the "statement" as
> +      such. Note that this code is called both when the slave does row
> +      injections and when the BINLOG statement is used to do row
> +      injections.
> +     */
> +    thd->lex->set_stmt_row_inject();
>  
>      /*
>        There are a few flags that are replicated with each row event.
> 
> === modified file 'sql/share/errmsg.txt'
> --- a/sql/share/errmsg.txt	2008-08-06 14:20:41 +0000
> +++ b/sql/share/errmsg.txt	2008-11-17 10:34:29 +0000
> @@ -6076,8 +6076,7 @@ ER_SLAVE_INCIDENT
>  ER_NO_PARTITION_FOR_GIVEN_VALUE_SILENT
>          eng "Table has no partition for some existing values"
>  ER_BINLOG_UNSAFE_STATEMENT
> -        eng "Statement is not safe to log in statement format."
> -        swe "Detta �inte s�rt att logga i statement-format."
> +        eng "Unsafe statement binlogged as statement since %s."
>  ER_SLAVE_FATAL_ERROR
>          eng "Fatal error: %s"
>  ER_SLAVE_RELAY_LOG_READ_FAILURE
> 
> === modified file 'sql/sql_base.cc'
> --- a/sql/sql_base.cc	2008-10-08 08:46:25 +0000
> +++ b/sql/sql_base.cc	2008-11-17 10:34:29 +0000
> @@ -30,6 +30,13 @@
>  
>  #define FLAGSTR(S,F) ((S) & (F) ? #F " " : "")
>  
> +static const char *prelocked_mode_name[] = {
> +  "NON_PRELOCKED",
> +  "PRELOCKED",
> +  "PRELOCKED_UNDER_LOCK_TABLES",
> +};

This is only used for one DBUG_PRINT? I suggest moving the array
declaration to where it is used:

  #ifdef DEBUG
  {
    static const char *prelocked_mode_name = { ... };
    DBUG_PRINT(...);
  }
  #endif

> +
> +
>  /**
>    This internal handler is used to trap internally
>    errors that can occur when executing open table
> @@ -5038,33 +5045,57 @@ static void mark_real_tables_as_free_for
>  /**
>     Decide on logging format to use for the statement.
>  
>     Compute the capabilities vector for the involved storage engines
>     and mask out the flags for the binary log. Right now, the binlog
>     flags only include the capabilities of the storage engines, so this
>     is safe.

This comment wasn't added by your patch, but I don't understand it at
all (What is the "capabilities vector"? What is "safe", in what sense is
it "safe", and why do we need to explain that it is safe?)

Could you update this?

>  
> -   We now have three alternatives that prevent the statement from
> -   being loggable:
> +   Decision table for logging format
> +   ---------------------------------
> +
> +   Row capable       N NNNNNNNNN YYYYYYYYY YYYYYYYYY
> +   Statement capable N YYYYYYYYY NNNNNNNNN YYYYYYYYY
> +
> +   Statement         * SSSUUUIII SSSUUUIII SSSUUUIII
> +
> +   Mode              * SMRSMRSMR SMRSMRSMR SMRSMRSMR
> +
> +   Format (Row/Stmt) - SS-SS---- -RR-RR-RR SRRSRR-RR
> +   Warning/Error     7 --4554666 3--3--2-- ---1--2--
> +
> +   Legend
> +   ------
> +
> +   Statement: (S)afe, (U)nsafe, or row (I)njection
> +   Mode:      (S)TATEMENT, (M)IXED, or (R)OW
>  
> -   1. If there are no capabilities left (all flags are clear) it is
> -      not possible to log the statement at all, so we roll back the
> -      statement and report an error.
> +   Warnings/Errors
> +   ---------------
>  
> -   2. Statement mode is set, but the capabilities indicate that
> -      statement format is not possible.
> +   1. Warning: Unsafe statement logged as statement due to
> +      binlog_format = STATEMENT
>  
> -   3. Row mode is set, but the capabilities indicate that row
> -      format is not possible.
> +   2. Error: Cannot execute row injection since binlog_format =
> +      STATEMENT
>  
> -   4. Statement is unsafe, but the capabilities indicate that row
> -      format is not possible.
> +   3. Error: Cannot modify table that uses a storage engine limited to
> +      row-logging when binlog_format = STATEMENT
>  
> -   If we are in MIXED mode, we then decide what logging format to use:
> +   4. Error: Cannot modify table that uses a storage engine limited to
> +      statement-logging when binlog_format = ROW
>  
> -   1. If the statement is unsafe, row-based logging is used.
> +   5. Warning: Unsafe statement logged as statement since storage
> +      engine is limited to statement-logging.
>  
> -   2. If statement-based logging is not possible, row-based logging is
> -      used.
> +   6. Error: Cannot inject row in table that uses a storage engine
> +      limited to statement-logging
>  
> -   3. Otherwise, statement-based logging is used.
> +   7. Error: Unable to binlog statement since both row-incapable
> +      engines and statement-incapable engines are involved.
>  
> -   @param thd    Client thread
> -   @param tables Tables involved in the query
> +   We now have three alternatives that prevent the statement from

Hmm, three alternatives? There are five error messages above. I suggest
reformulating to "In each of the above error cases, the statement is
prevented from being logged".

> +   being loggable. In these cases we report an error and roll back the
> +   statement.
> +
> +   @see THD::binlog_query
> +
> +   @param[in] thd    Client thread
> +   @param[in] tables Tables involved in the query
>   */
>  
>  int decide_logging_format(THD *thd, TABLE_LIST *tables)
> @@ -5075,6 +5106,8 @@ int decide_logging_format(THD *thd, TABL
>      handler::Table_flags flags_all_set= ~handler::Table_flags();
>      my_bool multi_engine= FALSE;
>      void* prev_ht= NULL;
> +    DBUG_PRINT("debug", ("thd->prelocked_mode: %s",
> +                         prelocked_mode_name[thd->prelocked_mode]));
>      for (TABLE_LIST *table= tables; table; table= table->next_global)
>      {
>        if (table->placeholder())
> @@ -5107,27 +5140,63 @@ int decide_logging_format(THD *thd, TABL
>      DBUG_PRINT("info", ("multi_engine: %s",
>                          multi_engine ? "TRUE" : "FALSE"));
>  
> +    thd->set_flags_all_set(flags_all_set);
> +

(*) It is a little bit ugly to save flags_all_set here. It's just an
internal state used in a computation. Why not determine the warning in
this function, and save a pointer to the warning text in the THD object
instead of the intersection of all flags? That would ensure all logic is
in this method, and binlog_query only does the reporting (so less
interface changes in case the logic needs to change later).

>      int error= 0;
>      if (flags_all_set == 0)

I think this should be:

    if (flags_all_set &
        (HA_BINLOG_STMT_CAPABLE | HA_BINLOG_ROW_CAPABLE) == 0)'

Otherwise no error is printed if some other flag is set for all tables.

>      {
> +      /*
> +        7. Error: Unable to binlog statement since both row-incapable
> +           engines and statement-incapable engines are involved.
> +      */
>        my_error((error= ER_BINLOG_LOGGING_IMPOSSIBLE), MYF(0),
> -               "Statement cannot be logged to the binary log in"
> -               " row-based nor statement-based format");
> +               "Unable to binlog statement since both row-incapable"
> +               " engines and statement-incapable engines are involved");
>      }
> -    else if (thd->variables.binlog_format == BINLOG_FORMAT_STMT &&
> -             (flags_all_set & HA_BINLOG_STMT_CAPABLE) == 0)
> +    else if (thd->lex->is_stmt_row_inject() &&
> +             (flags_all_set & HA_BINLOG_ROW_CAPABLE) == 0)
>      {
> +      /*
> +         6. Error: Cannot inject row in table that uses a storage
> +            engine limited to statement-logging
> +      */
>        my_error((error= ER_BINLOG_LOGGING_IMPOSSIBLE), MYF(0),
> -                "Statement-based format required for this statement,"
> -                " but not allowed by this combination of engines");
> +               "Cannot inject row in table that uses a storage engine"
> +               " limited to statement-logging");
> +
> +    }
> +    else if (thd->lex->is_stmt_row_inject() &&
> +             thd->variables.binlog_format == BINLOG_FORMAT_STMT)
> +    {
> +      /*
> +        2. Error: Cannot execute row injection since binlog_format =
> +           STATEMENT
> +      */
> +      my_error((error= ER_BINLOG_LOGGING_IMPOSSIBLE), MYF(0),
> +               "Cannot execute row injection since binlog_format = STATEMENT");
> +
>      }
> -    else if ((thd->variables.binlog_format == BINLOG_FORMAT_ROW ||
> -              thd->lex->is_stmt_unsafe()) &&
> +    else if (thd->variables.binlog_format == BINLOG_FORMAT_ROW &&
>               (flags_all_set & HA_BINLOG_ROW_CAPABLE) == 0)
>      {
> +      /*
> +        4. Error: Cannot modify table that uses a storage engine
> +           limited to statement-logging when binlog_format = ROW
> +      */
> +      my_error((error= ER_BINLOG_LOGGING_IMPOSSIBLE), MYF(0),
> +               "Cannot modify table that uses a storage engine limited"
> +               " to statement-logging when binlog_format = ROW");
> +    }
> +    else if (thd->variables.binlog_format == BINLOG_FORMAT_STMT &&
> +             (flags_all_set & HA_BINLOG_STMT_CAPABLE) == 0)
> +    {
> +      /*
> +        3. Error: Cannot modify table that uses a storage engine
> +           limited to row-logging when binlog_format = STATEMENT
> +      */
>        my_error((error= ER_BINLOG_LOGGING_IMPOSSIBLE), MYF(0),
> -                "Row-based format required for this statement,"
> -                " but not allowed by this combination of engines");
> +               "Cannot modify table that uses a storage engine"
> +               " limited to row-logging when binlog_format = STATEMENT");

This will read part of the message from errmsg.txt and part of it will
be hard-coded. Won't it look funny when part of the message is
translated and half is English? Why not use 7 different error codes for
the 7 messages (5 errors and 2 warnings)?

>      }
>  
>      /*
> @@ -5157,6 +5226,7 @@ int decide_logging_format(THD *thd, TABL
>  
>        1. If the statement is unsafe
>        2. If statement format cannot be used
> +      3. If we are executing a row injection
>  
>        Observe that point to cannot be decided before the tables
>        involved in a statement has been checked, i.e., we cannot put
>        this code in reset_current_stmt_binlog_row_based(), it has to be
>        here.

I can't parse the sentence starting "Observe..." and it doesn't seem
very important. I suggest removing it.

>      */
>      if (thd->lex->is_stmt_unsafe() ||
> -        (flags_all_set & HA_BINLOG_STMT_CAPABLE) == 0)
> +        (flags_all_set & HA_BINLOG_STMT_CAPABLE) == 0 ||
> +        thd->lex->is_stmt_row_inject())
>      {
>        thd->set_current_stmt_binlog_row_based_if_mixed();
>      }
> 
> === modified file 'sql/sql_class.cc'
> --- a/sql/sql_class.cc	2008-10-08 15:25:24 +0000
> +++ b/sql/sql_class.cc	2008-11-17 10:34:29 +0000
> @@ -3581,28 +3581,30 @@ show_query_type(THD::enum_binlog_query_t
>  #endif
>  
>  
> -/*
> -  Member function that will log query, either row-based or
> -  statement-based depending on the value of the 'current_stmt_binlog_row_based'
> -  the value of the 'qtype' flag.
> +/**
> +  Member function that will log query.
> +
> +  The query will be logged either row-based or statement-based
> +  depending on the value of the @c current_stmt_binlog_row_based the
> +  value of the @c qtype flag.

There is some parse error in this sentence: "the current_blah the value
of the qtype flag". Can you fix?

>  
>    This function should be called after the all calls to ha_*_row()
>    functions have been issued, but before tables are unlocked and
>    closed.
>  
> -  OBSERVE
> -    There shall be no writes to any system table after calling
> -    binlog_query(), so these writes has to be moved to before the call
> -    of binlog_query() for correct functioning.
> -
> -    This is necessesary not only for RBR, but the master might crash
> -    after binlogging the query but before changing the system tables.
> -    This means that the slave and the master are not in the same state
> -    (after the master has restarted), so therefore we have to
> -    eliminate this problem.
> +  @see decide_logging_format
> +
> +  @attention There shall be no writes to any system table after
> +  calling @c binlog_query(), so these writes has to be moved to before
> +  the call of @c binlog_query() for correct functioning.
> +
> +  @par This is necessesary not only for RBR, but the master might
> +  crash after binlogging the query but before changing the system
> +  tables.  This means that the slave and the master are not in the
> +  same state (after the master has restarted), so therefore we have to
> +  eliminate this problem.

The last paragraph is very difficult to understand (too many "this", not
clear what they refer to). If you know what it means, can you please
clarify?

>  
> -  RETURN VALUE
> -    Error code, or 0 if no error.
> +  @return Error code, or 0 if no error.
>  */
>  int THD::binlog_query(THD::enum_binlog_query_type qtype, char const *query_arg,
>                        ulong query_len, bool is_trans, bool suppress_use,
> @@ -3627,23 +3629,50 @@ int THD::binlog_query(THD::enum_binlog_q
>        DBUG_RETURN(error);
>  
>    /*
> -    If we are in statement mode and trying to log an unsafe statement,
> -    we should print a warning.
> +    There are two warnings that are printed here instead of in
> +    decide_logging_format(). We choose this since the warnings should
> +    be printed only if the statement is actually logged and when
> +    executing decide_logging_format() we only know that the binary log
> +    is open and some information about the statement. At that point,
> +    we cannot know for sure that this function will be called.
>    */
> -  if (lex->is_stmt_unsafe() &&
> -      variables.binlog_format == BINLOG_FORMAT_STMT)
> +  if (lex->is_stmt_unsafe())
>    {
> -    DBUG_ASSERT(this->query != NULL);
> -    push_warning(this, MYSQL_ERROR::WARN_LEVEL_WARN,
> -                 ER_BINLOG_UNSAFE_STATEMENT,
> -                 ER(ER_BINLOG_UNSAFE_STATEMENT));
> -    if (!(binlog_flags & BINLOG_FLAG_UNSAFE_STMT_PRINTED))
> +    const char *rationale= NULL;
> +    if (variables.binlog_format == BINLOG_FORMAT_STMT)
> +    {
> +      /*
> +        1. Warning: Unsafe statement logged as statement since
> +        binlog_format = STATEMENT
> +      */
> +      rationale= "binlog_format = STATEMENT";
> +    }
> +    else if ((flags_all_set() & HA_BINLOG_ROW_CAPABLE) == 0)
> +    {
> +      /*
> +        5. Warning: Unsafe statement logged as statement since storage
> +        engine is limited to statement-logging.
> +      */
> +      rationale= "storage engine is limited to statement-logging";
> +    }
> +
> +    if (rationale)

If you accept my suggestion at (*) above (compute the warning message in
decide_logging_format), then the code from "if (lex->is_stmt_unsafe())"
to "if (rationale)" would change to "if (thd->binlog_statement_warning)"
(and the following block would be indented one step less).

>      {
>        char warn_buf[MYSQL_ERRMSG_SIZE];
> -      my_snprintf(warn_buf, MYSQL_ERRMSG_SIZE, "%s Statement: %s",
> -                  ER(ER_BINLOG_UNSAFE_STATEMENT), this->query);
> -      sql_print_warning(warn_buf);
> -      binlog_flags|= BINLOG_FLAG_UNSAFE_STMT_PRINTED;
> +      size_t len;
> +      len= my_snprintf(warn_buf, MYSQL_ERRMSG_SIZE,
> +                       ER(ER_BINLOG_UNSAFE_STATEMENT), rationale);
> +      push_warning(this, MYSQL_ERROR::WARN_LEVEL_WARN,
> +                   ER_BINLOG_UNSAFE_STATEMENT, warn_buf);
> +
> +      if (!(binlog_flags & BINLOG_FLAG_UNSAFE_STMT_PRINTED))
> +      {
> +        DBUG_ASSERT(this->query != NULL);
> +        my_snprintf(warn_buf + len, MYSQL_ERRMSG_SIZE,
> +                    " Statement: %s", this->query);
> +        sql_print_warning(warn_buf);
> +        binlog_flags|= BINLOG_FLAG_UNSAFE_STMT_PRINTED;
> +      }
>      }
>    }
>  
> 
> === modified file 'sql/sql_class.h'
> --- a/sql/sql_class.h	2008-10-09 10:39:42 +0000
> +++ b/sql/sql_class.h	2008-11-17 10:34:29 +0000
> @@ -2210,7 +2210,25 @@ public:
>    */
>    void pop_internal_handler();
>  
> +  /**
> +     Set intersection of all table flags.
> +   */
> +  void set_flags_all_set(handler::Table_flags flags) {
> +    m_flags_all_set= flags;
> +  }
> +
> +
> +  /**
> +     Get intersection of all table flags computed in
> +     @c decide_logging_format().
> +   */
> +  handler::Table_flags flags_all_set() const {
> +    return m_flags_all_set;
> +  }
> +
>  private:
> +  handler::Table_flags m_flags_all_set;
> +

If you accept my suggestion at (*) above (compute the warning message in
decide_logging_format), then this would change to char
*binlog_statement_warning or something like that.

>    /** The current internal error handler for this thread, or NULL. */
>    Internal_error_handler *m_internal_handler;
>    /**
> 
> === modified file 'sql/sql_lex.h'
> --- a/sql/sql_lex.h	2008-10-07 21:52:49 +0000
> +++ b/sql/sql_lex.h	2008-11-17 10:34:29 +0000
> @@ -1036,6 +1036,13 @@ public:
>    }
>  
>    /**
> +     Is this statement actually a row injection?
> +   */
> +  inline bool is_stmt_row_inject() const {
> +    return binlog_stmt_flags & (1U << BINLOG_STMT_FLAG_ROW_INJECTION);
> +  }
> +
> +  /**
>       Flag the current (top-level) statement as unsafe.
>  
>       The flag will be reset after the statement has finished.
> @@ -1049,6 +1056,14 @@ public:
>      binlog_stmt_flags&= ~(1U << BINLOG_STMT_FLAG_UNSAFE);
>    }
>  
> +  inline void set_stmt_row_inject() {
> +    binlog_stmt_flags|= (1U << BINLOG_STMT_FLAG_ROW_INJECTION);
> +  }
> +
> +  inline void clear_stmt_row_inject() {
> +    binlog_stmt_flags|= (1U << BINLOG_STMT_FLAG_ROW_INJECTION);
> +  }

Why a bit field? We can't have injection and unsafe at the same time
anyways. I suggest:
 - add the enumeration value BINLOG_STMT_SAFE to enum_binlog_stmt_flag
 - rename enum_binlog_stmt_flag -> enum_binlog_stmt_type
 - replace "int binlog_stmt_flags" by "enum_binlog_stmt_type
binlog_stmt_type".

Also, I suggest renaming [set|clear]_stmt_row_inject to
[set|clear]_stmt_row_injection (add "ion" to the end), so that it's the
same nomenclature everywhere.

> +
>    /**
>      true if the parsed tree contains references to stored procedures
>      or functions, false otherwise
> @@ -1059,6 +1074,7 @@ public:
>  private:
>    enum enum_binlog_stmt_flag {
>      BINLOG_STMT_FLAG_UNSAFE,
> +    BINLOG_STMT_FLAG_ROW_INJECTION,
>      BINLOG_STMT_FLAG_COUNT
>    };
>  
> 
> === modified file 'sql/sql_prepare.cc'
> --- a/sql/sql_prepare.cc	2008-07-15 01:43:12 +0000
> +++ b/sql/sql_prepare.cc	2008-11-17 10:34:29 +0000
> @@ -1166,6 +1166,8 @@ static bool mysql_test_insert(Prepared_s
>        counter++;
>        if (values->elements != value_count)
>        {
> +        DBUG_PRINT("debug", ("values->elements (%d) != value_count (%d)",
> +                             values->elements, value_count));

Hmm, was this committed by mistake? If not, how is it related to BUG#39934?

>          my_error(ER_WRONG_VALUE_COUNT_ON_ROW, MYF(0), counter);
>          goto error;
>        }
> 


-- 
Sven Sandberg, Software Engineer
MySQL AB, www.mysql.com

Thread
bzr commit into mysql-5.1 branch (mats:2771) Bug#39934Mats Kindahl17 Nov
  • Re: bzr commit into mysql-5.1 branch (mats:2771) Bug#39934Sven Sandberg2 Dec
    • Re: bzr commit into mysql-5.1 branch (mats:2771) Bug#39934Sven Sandberg4 Dec
    • Re: bzr commit into mysql-5.1 branch (mats:2771) Bug#39934Mats Kindahl12 Dec
Re: bzr commit into mysql-5.1 branch (mats:2771) Bug#39934Mats Kindahl17 Dec