Hi Luis,
I don't have comments on the code.
I think the possible problems were already mentioned by Jasonh.
Comments on the test case:
1 - Collapse the tests in one single place but keep different test cases
for mixed and statement modes.
2 - Incorporate the test case that I wrote while discussing isolation
levels with Sven.
3 - Add multi-update statements.
4 - Add statements that access both databases.
5 - Add comments/output (i.e. echo) stating what is the expected output.
6 - For the sake of completeness would be great to have the row based mode.
(This is up to you. I don't have a strong opinion on that.)
Find just one comment in-line.
Cheers.
Luis Soares wrote:
> #At file:///home/lsoares/Workspace/mysql-server/bugfix/42829/5.1-bt/ based on
> revid:matthias.leich@stripped
>
> 2840 Luis Soares 2009-04-23
> BUG#42829: binlogging enabled for all schemas regardless of
> binlog-db-db / binlog-ignore-db
>
> InnoDB will return an error if statement based replication is used
> along with transaction isolation level READ-COMMITTED (or stricter),
> even if the statement in question is filtered out according to the
> binlog-do-db rules set. In this case, an error should not be printed.
>
> This patch addresses this issue by extending the existing check in
> external_lock to take into account the filter rules before deciding to
> print an error. Furthermore, it also changes decide_logging_format to
> take into consideration whether the statement is filtered out from
> binlog before decision is made.
> @ mysql-test/suite/binlog/r/binlog_mix_do_db.result
> Result file.
> @ mysql-test/suite/binlog/r/binlog_stm_do_db.result
> Result file.
> @ mysql-test/suite/binlog/t/binlog_mix_do_db.test
> Added test case for mixed format to check that statements are
> turned into unsafe and logged in rows and that the additional
> clause added in decide logging format produces no side-effects.
> @ mysql-test/suite/binlog/t/binlog_stm_do_db.test
> Test case for statement based logging. This format triggered
> the bug reported.
> @ sql/sql_base.cc
> Changed the check on decide_logging_format to take into account
> whether statement is filtered or not (and if it is in mixed mode)
> before making the decision on log format (error checking).
> @ sql/sql_class.cc
> Added the thd_binlog_filter_ok to INNODB_COMPATIBILITY_HOOKS set.
> @ storage/innobase/handler/ha_innodb.cc
> Extended check in external_lock to take into consideration the
> filtering when deciding to throw an error.
> @ storage/innobase/handler/ha_innodb.h
> Added declaration of new hook.
>
> added:
> mysql-test/suite/binlog/r/binlog_mix_do_db.result
> mysql-test/suite/binlog/r/binlog_stm_do_db.result
> mysql-test/suite/binlog/t/binlog_mix_do_db-master.opt
> mysql-test/suite/binlog/t/binlog_mix_do_db.test
> mysql-test/suite/binlog/t/binlog_stm_do_db-master.opt
> mysql-test/suite/binlog/t/binlog_stm_do_db.test
> modified:
> sql/sql_base.cc
> sql/sql_class.cc
> storage/innobase/handler/ha_innodb.cc
> storage/innobase/handler/ha_innodb.h
> === added file 'mysql-test/suite/binlog/r/binlog_mix_do_db.result'
> --- a/mysql-test/suite/binlog/r/binlog_mix_do_db.result 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/binlog/r/binlog_mix_do_db.result 2009-04-23 11:31:32 +0000
> @@ -0,0 +1,42 @@
> +SET @old_isolation_level= @@tx_isolation;
> +SET tx_isolation= 'READ-COMMITTED';
> +CREATE DATABASE db_not_filtered;
> +use db_not_filtered;
> +CREATE TABLE t1 (x int, y int) engine=InnoDB;
> +CREATE TABLE t2 (x int, y int) engine=InnoDB;
> +CREATE DATABASE db_filtered;
> +use db_filtered;
> +CREATE TABLE t1 (x int, y int) engine=InnoDB;
> +CREATE TABLE t2 (x int, y int) engine=InnoDB;
> +INSERT INTO t2 VALUES (1,2);
> +INSERT INTO t1 SELECT * FROM t2;
> +INSERT INTO db_not_filtered.t1 VALUES (1, 2);
> +INSERT INTO db_not_filtered.t1 VALUES (UUID(), 2);
> +use db_not_filtered;
> +INSERT INTO t2 VALUES (1,2);
> +INSERT INTO t1 SELECT * FROM t2;
> +DROP DATABASE db_not_filtered;
> +DROP DATABASE db_filtered;
> +show binlog events from <binlog_start>;
> +Log_name Pos Event_type Server_id End_log_pos Info
> +master-bin.000001 # Query # # CREATE DATABASE db_not_filtered
> +master-bin.000001 # Query # # use `db_not_filtered`; CREATE TABLE t1 (x int, y int)
> engine=InnoDB
> +master-bin.000001 # Query # # use `db_not_filtered`; CREATE TABLE t2 (x int, y int)
> engine=InnoDB
> +master-bin.000001 # Query # # use `db_filtered`; BEGIN
> +master-bin.000001 # Table_map # # table_id: # (db_not_filtered.t1)
> +master-bin.000001 # Write_rows # # table_id: # flags: STMT_END_F
> +master-bin.000001 # Xid # # COMMIT /* XID */
> +master-bin.000001 # Query # # use `db_filtered`; BEGIN
> +master-bin.000001 # Table_map # # table_id: # (db_not_filtered.t1)
> +master-bin.000001 # Write_rows # # table_id: # flags: STMT_END_F
> +master-bin.000001 # Xid # # COMMIT /* XID */
> +master-bin.000001 # Query # # use `db_not_filtered`; BEGIN
> +master-bin.000001 # Table_map # # table_id: # (db_not_filtered.t2)
> +master-bin.000001 # Write_rows # # table_id: # flags: STMT_END_F
> +master-bin.000001 # Xid # # COMMIT /* XID */
> +master-bin.000001 # Query # # use `db_not_filtered`; BEGIN
> +master-bin.000001 # Table_map # # table_id: # (db_not_filtered.t1)
> +master-bin.000001 # Write_rows # # table_id: # flags: STMT_END_F
> +master-bin.000001 # Xid # # COMMIT /* XID */
> +master-bin.000001 # Query # # DROP DATABASE db_not_filtered
> +SET @@tx_isolation= @old_isolation_level;
>
> === added file 'mysql-test/suite/binlog/r/binlog_stm_do_db.result'
> --- a/mysql-test/suite/binlog/r/binlog_stm_do_db.result 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/binlog/r/binlog_stm_do_db.result 2009-04-23 11:31:32 +0000
> @@ -0,0 +1,26 @@
> +SET @old_isolation_level= @@tx_isolation;
> +SET tx_isolation= 'READ-COMMITTED';
> +CREATE DATABASE db_not_filtered;
> +use db_not_filtered;
> +CREATE TABLE t1 (x int, y int) engine=InnoDB;
> +CREATE TABLE t2 (x int, y int) engine=InnoDB;
> +CREATE DATABASE db_filtered;
> +use db_filtered;
> +CREATE TABLE t1 (x int, y int) engine=InnoDB;
> +CREATE TABLE t2 (x int, y int) engine=InnoDB;
> +INSERT INTO t2 VALUES (1,2);
> +INSERT INTO t1 SELECT * FROM t2;
> +use db_not_filtered;
> +INSERT INTO t2 VALUES (1,2);
> +ERROR HY000: Binary logging not possible. Message: Transaction level
> 'READ-COMMITTED' in InnoDB is not safe for binlog mode 'STATEMENT'
> +INSERT INTO t1 SELECT * FROM t2;
> +ERROR HY000: Binary logging not possible. Message: Transaction level
> 'READ-COMMITTED' in InnoDB is not safe for binlog mode 'STATEMENT'
> +DROP DATABASE db_not_filtered;
> +DROP DATABASE db_filtered;
> +show binlog events from <binlog_start>;
> +Log_name Pos Event_type Server_id End_log_pos Info
> +master-bin.000001 # Query # # CREATE DATABASE db_not_filtered
> +master-bin.000001 # Query # # use `db_not_filtered`; CREATE TABLE t1 (x int, y int)
> engine=InnoDB
> +master-bin.000001 # Query # # use `db_not_filtered`; CREATE TABLE t2 (x int, y int)
> engine=InnoDB
> +master-bin.000001 # Query # # DROP DATABASE db_not_filtered
> +SET @@tx_isolation= @old_isolation_level;
>
> === added file 'mysql-test/suite/binlog/t/binlog_mix_do_db-master.opt'
> --- a/mysql-test/suite/binlog/t/binlog_mix_do_db-master.opt 1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/binlog/t/binlog_mix_do_db-master.opt 2009-04-23 11:31:32
> +0000
> @@ -0,0 +1 @@
> +--binlog-do-db=db_not_filtered
>
> === added file 'mysql-test/suite/binlog/t/binlog_mix_do_db.test'
> --- a/mysql-test/suite/binlog/t/binlog_mix_do_db.test 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/binlog/t/binlog_mix_do_db.test 2009-04-23 11:31:32 +0000
> @@ -0,0 +1,53 @@
> +# BUG#42829: binlogging enabled for all schemas regardless of binlog-db-db /
> +# binlog-ignore-db
> +#
> +# DESCRIPTION
> +# ===========
> +#
> +# The test is implemented as follows:
> +# i) set tx_isolation to read-committed.
> +# ii) create two databases (one filtered other not - using binlog-do-db)
> +# iii) Create statements that are to be filtered on filtered db
> +# - At this point before the patch an error would be thrown.
> +# iv) Create statements while using db_filtered that change db_not_filtered
> +# tables. (These should be marked as unsafe and make it into the binlog)
> +# v) Insert into tables in the not filtered database and check that
> +# no error exists because these are logged as row events.
> +
> +source include/have_log_bin.inc;
> +source include/have_innodb.inc;
> +source include/have_binlog_format_mixed.inc;
> +
> +SET @old_isolation_level= @@tx_isolation;
> +SET tx_isolation= 'READ-COMMITTED';
> +
> +let $engine= InnoDB;
> +
> +CREATE DATABASE db_not_filtered;
> +use db_not_filtered;
> +eval CREATE TABLE t1 (x int, y int) engine=$engine;
> +eval CREATE TABLE t2 (x int, y int) engine=$engine;
> +
> +CREATE DATABASE db_filtered;
> +use db_filtered;
> +eval CREATE TABLE t1 (x int, y int) engine=$engine;
> +eval CREATE TABLE t2 (x int, y int) engine=$engine;
> +
> +INSERT INTO t2 VALUES (1,2);
> +INSERT INTO t1 SELECT * FROM t2;
> +
>
Please, do not disable the warning message.
> +INSERT INTO db_not_filtered.t1 VALUES (1, 2);
> +disable_warnings;
> +INSERT INTO db_not_filtered.t1 VALUES (UUID(), 2);
> +enable_warnings;
> +
> +use db_not_filtered;
> +INSERT INTO t2 VALUES (1,2);
> +INSERT INTO t1 SELECT * FROM t2;
>
Just for the sake of completeness do also:
+INSERT INTO db_filtered.t1 VALUES (1, 2);
+INSERT INTO db_filtered.t1 VALUES (UUID(), 2);
> +
> +DROP DATABASE db_not_filtered;
> +DROP DATABASE db_filtered;
> +
> +source include/show_binlog_events.inc;
> +
> +SET @@tx_isolation= @old_isolation_level;
>
> === added file 'mysql-test/suite/binlog/t/binlog_stm_do_db-master.opt'
> --- a/mysql-test/suite/binlog/t/binlog_stm_do_db-master.opt 1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/binlog/t/binlog_stm_do_db-master.opt 2009-04-23 11:31:32
> +0000
> @@ -0,0 +1 @@
> +--binlog-do-db=db_not_filtered
>
> === added file 'mysql-test/suite/binlog/t/binlog_stm_do_db.test'
> --- a/mysql-test/suite/binlog/t/binlog_stm_do_db.test 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/binlog/t/binlog_stm_do_db.test 2009-04-23 11:31:32 +0000
> @@ -0,0 +1,52 @@
> +#
> +# BUG#42829: binlogging enabled for all schemas regardless of binlog-db-db /
> +# binlog-ignore-db
> +#
> +# DESCRIPTION
> +# ===========
> +#
> +# The test is implemented as follows:
> +# i) set tx_isolation to read-committed.
> +# ii) create two databases (one filtered other not - using binlog-do-db)
> +# iii) Create statements that are to be filtered on filtered db
> +# - At this point before the patch an error would be thrown.
> +# iv) do the same thing for not the filtered database and check that
> +# events throw an error:
> +#
> +# - Error: 1598 SQLSTATE: HY000 (ER_BINLOG_LOGGING_IMPOSSIBLE)
> +#
> +
> +source include/have_log_bin.inc;
> +source include/have_innodb.inc;
> +source include/have_binlog_format_statement.inc;
> +
> +SET @old_isolation_level= @@tx_isolation;
> +SET tx_isolation= 'READ-COMMITTED';
> +
> +let $engine= InnoDB;
> +
> +CREATE DATABASE db_not_filtered;
> +use db_not_filtered;
> +eval CREATE TABLE t1 (x int, y int) engine=$engine;
> +eval CREATE TABLE t2 (x int, y int) engine=$engine;
> +
> +CREATE DATABASE db_filtered;
> +use db_filtered;
> +eval CREATE TABLE t1 (x int, y int) engine=$engine;
> +eval CREATE TABLE t2 (x int, y int) engine=$engine;
> +
> +INSERT INTO t2 VALUES (1,2);
> +INSERT INTO t1 SELECT * FROM t2;
> +
> +use db_not_filtered;
> +--error 1598
> +INSERT INTO t2 VALUES (1,2);
> +--error 1598
> +INSERT INTO t1 SELECT * FROM t2;
> +
> +DROP DATABASE db_not_filtered;
> +DROP DATABASE db_filtered;
> +
> +source include/show_binlog_events.inc;
> +
> +SET @@tx_isolation= @old_isolation_level;
>
> === modified file 'sql/sql_base.cc'
> --- a/sql/sql_base.cc 2009-03-27 12:55:14 +0000
> +++ b/sql/sql_base.cc 2009-04-23 11:31:32 +0000
> @@ -5089,7 +5089,19 @@ static void mark_real_tables_as_free_for
>
> int decide_logging_format(THD *thd, TABLE_LIST *tables)
> {
> - if (mysql_bin_log.is_open() && (thd->options & OPTION_BIN_LOG))
> + /*
> + Only deciding if:
> +
> + 1. binlog is open and option bin log is set
> +
> + 2. AND statement is not filtered out from binlog OR binlog format
> + is mixed (meaning that filter rules may present different results
> + based on the logging format choosen later - see BUG#43457 for
> + details).
> + */
> + if (mysql_bin_log.is_open() && (thd->options & OPTION_BIN_LOG)
> &&
> + (thd->variables.binlog_format == BINLOG_FORMAT_MIXED ||
> + binlog_filter->db_ok(thd->db)))
> {
> /*
> Compute the starting vectors for the computations by creating a
>
> === modified file 'sql/sql_class.cc'
> --- a/sql/sql_class.cc 2009-03-24 13:58:52 +0000
> +++ b/sql/sql_class.cc 2009-04-23 11:31:32 +0000
> @@ -2894,6 +2894,11 @@ extern "C" void thd_mark_transaction_to_
> {
> mark_transaction_to_rollback(thd, all);
> }
> +
> +extern "C" bool thd_binlog_filter_ok(const MYSQL_THD thd)
> +{
> + return binlog_filter->db_ok(thd->db);
> +}
> #endif // INNODB_COMPATIBILITY_HOOKS */
>
> /****************************************************************************
>
> === modified file 'storage/innobase/handler/ha_innodb.cc'
> --- a/storage/innobase/handler/ha_innodb.cc 2009-03-24 13:58:52 +0000
> +++ b/storage/innobase/handler/ha_innodb.cc 2009-04-23 11:31:32 +0000
> @@ -6810,8 +6810,10 @@ ha_innobase::external_lock(
> {
> ulong const binlog_format= thd_binlog_format(thd);
> ulong const tx_isolation = thd_tx_isolation(current_thd);
> + bool const is_binlog_filter_ok= thd_binlog_filter_ok(thd);
> if (tx_isolation <= ISO_READ_COMMITTED &&
> - binlog_format == BINLOG_FORMAT_STMT)
> + binlog_format == BINLOG_FORMAT_STMT &&
> + is_binlog_filter_ok)
> {
> char buf[256];
> my_snprintf(buf, sizeof(buf),
>
> === modified file 'storage/innobase/handler/ha_innodb.h'
> --- a/storage/innobase/handler/ha_innodb.h 2008-12-14 20:59:50 +0000
> +++ b/storage/innobase/handler/ha_innodb.h 2009-04-23 11:31:32 +0000
> @@ -252,4 +252,11 @@ int thd_binlog_format(const MYSQL_THD th
> @param all TRUE <=> rollback main transaction.
> */
> void thd_mark_transaction_to_rollback(MYSQL_THD thd, bool all);
> +
> +/**
> + Check if binary logging is filtered for thread's current db.
> + @param thd Thread handle
> + @retval 1 the query is not filtered, 0 otherwise.
> +*/
> +bool thd_binlog_filter_ok(const MYSQL_THD thd);
> }
>
>
> ------------------------------------------------------------------------
>
>
>
