Hi Libing,
Great work. You are in the right direction to simplify the binlog.
Please, see some comments in what follows.
STATUS
------
Patch not approved.
REQUESTS
--------
1 - Remove the disabled line in /sql/sql_priv.h
2 - Can you explain the additional expressions? I can understand the
format but I don't think it is necessary in_multi_stmt_transaction_mode?
/* So that CREATE TEMPORARY TABLE gets to binlog at commit/rollback */
if (!res && (create_info.options & HA_LEX_CREATE_TMP_TABLE) &&
thd->in_multi_stmt_transaction_mode() &&
!thd->is_current_stmt_binlog_format_row())
thd->transaction.stmt.modified_non_trans_table= TRUE;
3 - There are other entry points where you need to put
set_non_trans_table_in_trx_cache(). For instance,
flush_and_set_pending_rows_event().
4 - There are several failures after your patch:
rpl.rpl_mixed_binlog_max_cache_size
rpl.rpl_mixed_drop_create_temp_table
rpl.rpl_mixed_mixing_engines
rpl.rpl_non_direct_mixed_mixing_engines
rpl.rpl_stm_innodb
rpl.rpl_stop_slave
rpl.rpl_non_direct_row_mixing_engines
rpl.rpl_row_binlog_max_cache_size
rpl.rpl_row_drop_create_temp_table
rpl.rpl_row_mixing_engines
rpl.rpl_non_direct_stm_mixing_engines
rpl.rpl_stm_binlog_max_cache_size
rpl.rpl_stm_drop_create_temp_table
rpl.rpl_stm_mixing_engines
binlog.binlog_stm_mix_innodb_myisam
binlog.binlog_row_mix_innodb_myisam
binlog.binlog_innodb_row
main.flush_read_lock main.mysql
5 - Please, remove the function stmt_accessed_non_trans_temp_table and
any other function that is not necessary after your patch.
6 - You don't need a test for BUG#56184.
SUGGESTIONS
-----------
1 - Please, move this
+ void set_non_trans_table_in_trx_cache()
+ {
+ non_trans_table_in_trx_cache= TRUE;
+ }
+
+ bool is_non_trans_table_in_trx_cache()
+ {
+ return non_trans_table_in_trx_cache;
}
to binlog_cache_data.
Cheers.
On 11/29/2010 05:36 AM, Li-Bing.Song@stripped wrote:
> #At file:///home/anders/Work/bzrwork/wt3/mysql-trunk-bugfixing/ based on
> revid:dao-gang.qu@stripped
>
> 3350 Li-Bing.Song@stripped 2010-11-29
> Bug#56184 Rolled back transaction without non-trasactional table updated was
> binlogged
> Bug#55798 Slave SQL retry on transaction inserts extra data into
> non-transaction table
>
> Bug#56184
> The transaction modified non-transactional table will be binlogged with
> ROLLBACK if it
> rolls back on master. It includes the case that all statements which modified
> non-transactional table are binlogged outside(before) the transaction.
> Example:
> BEGIN
> INSERT INTO trans-table;
> INSERT INOT non-trans-table;
> ROLLBACK
> it will be binlogged as:
> BEGIN
> INSERT INTO non-trans-table;
> COMMIT
> BEGIN
> INSERT INTO trans-table;
> ROLLBACK;
> All statements in the second binlogged transaction modify only transactional
> tables and
> are rolled back safely on master. So the second transaction should not be
> binlogged.
>
> After 5.5, there are two caches for binary logs. A transactional cache and a
> statement cache.
> When executing a transaction, statements modified only transactional tables
> are always put in
> transactional cache. statements modified non-transactional tables can be put
> in both caches.
> It depends on different situations. In this patch, a flag is added to mark if
> there is any
> statement modified non-transactional table is in transactional cache. When
> rolling back a
> transaction on master, transactional cache should not be flushed to binary
> log, if there is
> no any statement in it modified non-transactional table, else it should be
> flushed into binary
> log followed by 'ROLLBACK' statement.
Some suggestions to improve the comments.
After 5.5, there are two caches for binary logs, a transactional cache
and a statement cache. When executing a transaction, statements that
modified only transactional tables are always put in transactional
cache. Statements that modified non-transactional tables can be put in
either transactional or non-transactional cache depending on different
situations. In this patch, a flag is added to mark if there is any
statement that modified non-transactional table in transactional cache.
When rolling back a transaction on master, transactional cache should
not be flushed to binary log, if there is no statement in it that
modified a non-transactional table. Otherwise, it should be flushed into
binary log followed by 'ROLLBACK' statement.
>
> BUG#55798
> When a temporary error(eg. Lock timeout) happens, Slave SQL thread will
> rollback the
> transaction and retry it again. But it is possible that the transaction cannot
> be
> rolled back safely. For example a non-transactional table has been modified by
> the
> transaction. It will make master and slave diversely.
s/diversely/to diverge/
>
> After this patch, SQL thread will not retry to execute a transaction which can
> not be rolled
> back safely if temporary error is encountered.
> @ mysql-test/suite/rpl/t/rpl_begin_commit_rollback.test
> Add test to verify this patch.
> @ sql/binlog.cc
> Refactor reset_cache, it is divided into reset_trx_cache() and
> reset_stmt_cache().
> Add code to fix this bug.
> @ sql/rpl_slave.cc
> OPTION_KEEP_LOG is replaced by stmt.modified_non_trans_table.
> Remove OPTION_KEEP_LOG from all code.
> @ sql/sql_parse.cc
> OPTION_KEEP_LOG is replaced by stmt.modified_non_trans_table.
> Remove OPTION_KEEP_LOG from all code.
> @ sql/sql_table.cc
> Set stmt.modified_non_trans_table after a temporary table has been dropped.
> @ sql/sys_vars.cc
> OPTION_KEEP_LOG is replaced by stmt.modified_non_trans_table.
> Remove OPTION_KEEP_LOG from all code.
> @ sql/transaction.cc
> OPTION_KEEP_LOG is replaced by stmt.modified_non_trans_table.
> Remove OPTION_KEEP_LOG from all code.
>
ok.
> modified:
> mysql-test/suite/rpl/r/rpl_begin_commit_rollback.result
> mysql-test/suite/rpl/t/rpl_begin_commit_rollback.test
> sql/binlog.cc
> sql/rpl_slave.cc
> sql/sql_parse.cc
> sql/sql_priv.h
> sql/sql_table.cc
> sql/sys_vars.cc
> sql/transaction.cc
> === modified file 'mysql-test/suite/rpl/r/rpl_begin_commit_rollback.result'
> --- a/mysql-test/suite/rpl/r/rpl_begin_commit_rollback.result 2010-09-27 13:20:24
> +0000
> +++ b/mysql-test/suite/rpl/r/rpl_begin_commit_rollback.result 2010-11-29 05:36:21
> +0000
> @@ -173,6 +173,64 @@ in savepoint mixed_cases
> # Verify INSERT statements on the Innodb table are rolled back;
> SELECT * FROM db1.t1 WHERE a IN (30, 40);
> a
> +
> +# Bug#56184 Rolled back transaction without non-trasactional table
> +# updated was binlogged
> +# ----------------------------------------------------------------
> +# Transactional cache should not be binlogged if no statement in it
> +# updates a non-trasactional table.
> +# [ on master ]
> +USE db1;
> +DROP TABLE t1, t2;
> +CREATE TABLE t1(c1 INT KEY) ENGINE=MyISAM;
> +CREATE TABLE t2(c1 INT KEY) ENGINE=InnoDB;
> +USE db1;
> +# [ on slave ]
> +# This statement is used to verify that the transaction which is rolled
> +# back is not binlogged. The duplicate keys will cause slave to stop if
> +# the transaction is binlogged.
> +INSERT INTO t2 VALUES(1),(2);
> +# [ on master ]
> +SET @@session.binlog_direct_non_transactional_updates= TRUE;
> +BEGIN;
> +INSERT INTO t2 VALUES(1);
> +# It will not be put into transaction cache.
> +INSERT INTO t1 VALUES(1);
> +INSERT INTO t2 VALUES(2);
> +ROLLBACK;
> +Warnings:
> +Warning 1196 Some non-transactional changed tables couldn't be rolled back
> +Comparing tables master:db1.t1 and slave:db1.t1
> +
> +# BUG#55798 Slave SQL retry on transaction inserts extra data into
> +# non-transaction table
> +# ----------------------------------------------------------------
> +# To verify that SQL thread does not retry a transaction which can
> +# not be rolled back safely, even though only a temporary error is
> +# encountered.
> +
> +# [ on slave ]
> +SET GLOBAL innodb_lock_wait_timeout= 1;
> +STOP SLAVE SQL_THREAD;
> +START SLAVE SQL_THREAD;
> +BEGIN;
> +# It will lock table t2 on row in which c1 is 1 until COMMIT or ROLLBACK
> +UPDATE t2 SET c1=10 WHERE c1=1;
> +# [ on master ]
> +SET @@session.binlog_direct_non_transactional_updates= FALSE;
> +BEGIN;
> +INSERT INTO t2 VALUES(3);
> +INSERT INTO t1 VALUES(4);
> +Warnings:
> +Note 1592 Unsafe statement written to the binary log using statement format since
> BINLOG_FORMAT = STATEMENT. Statement is unsafe because it accesses a non-transactional
> table after accessing a transactional table within the same transaction.
> +UPDATE t2 SET c1=11 WHERE c1=1;
> +COMMIT;
> +# [ on slave ]
> +Comparing tables master:db1.t1 and slave:db1.t1
> +ROLLBACK;
> +DELETE FROM t1 WHERE c1=4;
> +SET GLOBAL innodb_lock_wait_timeout= 50;
> +START SLAVE SQL_THREAD;
> #
> # Clean up
> #
>
> === modified file 'mysql-test/suite/rpl/t/rpl_begin_commit_rollback.test'
> --- a/mysql-test/suite/rpl/t/rpl_begin_commit_rollback.test 2010-06-30 13:09:43
> +0000
> +++ b/mysql-test/suite/rpl/t/rpl_begin_commit_rollback.test 2010-11-29 05:36:21
> +0000
> @@ -171,10 +171,90 @@ SELECT * FROM db1.t2 WHERE s LIKE '% sav
> --echo # Verify INSERT statements on the Innodb table are rolled back;
> SELECT * FROM db1.t1 WHERE a IN (30, 40);
>
> +--echo
> +--echo # Bug#56184 Rolled back transaction without non-trasactional table
> +--echo # updated was binlogged
> +--echo # ----------------------------------------------------------------
> +--echo # Transactional cache should not be binlogged if no statement in it
> +--echo # updates a non-trasactional table.
> +--echo # [ on master ]
> +connection master;
> +USE db1;
> +DROP TABLE t1, t2;
> +CREATE TABLE t1(c1 INT KEY) ENGINE=MyISAM;
> +CREATE TABLE t2(c1 INT KEY) ENGINE=InnoDB;
> +sync_slave_with_master;
> +
> +USE db1;
> +--echo # [ on slave ]
> +--echo # This statement is used to verify that the transaction which is rolled
> +--echo # back is not binlogged. The duplicate keys will cause slave to stop if
> +--echo # the transaction is binlogged.
> +INSERT INTO t2 VALUES(1),(2);
> +
> +--echo # [ on master ]
> +connection master;
> +SET @@session.binlog_direct_non_transactional_updates= TRUE;
> +BEGIN;
> +INSERT INTO t2 VALUES(1);
> +--echo # It will not be put into transaction cache.
> +INSERT INTO t1 VALUES(1);
> +INSERT INTO t2 VALUES(2);
> +ROLLBACK;
> +
> +sync_slave_with_master;
> +let $diff_table=db1.t1;
> +source include/rpl_diff_tables.inc;
> +
> +--echo
> +--echo # BUG#55798 Slave SQL retry on transaction inserts extra data into
> +--echo # non-transaction table
> +--echo # ----------------------------------------------------------------
> +--echo # To verify that SQL thread does not retry a transaction which can
> +--echo # not be rolled back safely, even though only a temporary error is
> +--echo # encountered.
> +--echo
> +--echo # [ on slave ]
> +
> +let $timeout_old= `SELECT @@GLOBAL.innodb_lock_wait_timeout`;
> +SET GLOBAL innodb_lock_wait_timeout= 1;
> +
> +STOP SLAVE SQL_THREAD;
> +source include/wait_for_slave_sql_to_stop.inc;
> +START SLAVE SQL_THREAD;
> +source include/wait_for_slave_sql_to_start.inc;
> +
> +BEGIN;
> +--echo # It will lock table t2 on row in which c1 is 1 until COMMIT or ROLLBACK
> +UPDATE t2 SET c1=10 WHERE c1=1;
> +
> +connection master;
> +--echo # [ on master ]
> +SET @@session.binlog_direct_non_transactional_updates= FALSE;
> +BEGIN;
> +INSERT INTO t2 VALUES(3);
> +INSERT INTO t1 VALUES(4);
> +UPDATE t2 SET c1=11 WHERE c1=1;
> +COMMIT;
> +
> +connection slave;
> +--echo # [ on slave ]
> +let $slave_sql_errno= 1205;
> +source include/wait_for_slave_sql_to_stop.inc;
> +let $diff_table_1=master:db1.t1;
> +let $diff_table_2=slave:db1.t1;
> +source include/diff_tables.inc;
> +ROLLBACK;
> +DELETE FROM t1 WHERE c1=4;
> +
> +eval SET GLOBAL innodb_lock_wait_timeout= $timeout_old;
> +START SLAVE SQL_THREAD;
> +source include/wait_for_slave_sql_to_start.inc;
> +
> --echo #
> --echo # Clean up
> --echo #
> connection master;
> DROP DATABASE db1;
> DROP DATABASE db2;
> -source include/master-slave-end.inc;
> +source include/master-slave-end.inc;
> \ No newline at end of file
>
> === modified file 'sql/binlog.cc'
> --- a/sql/binlog.cc 2010-11-16 12:38:17 +0000
> +++ b/sql/binlog.cc 2010-11-29 05:36:21 +0000
> @@ -121,16 +121,6 @@ public:
> return(incident);
> }
>
> - void set_changes_to_non_trans_temp_table()
> - {
> - changes_to_non_trans_temp_table_flag= TRUE;
> - }
> -
> - bool changes_to_non_trans_temp_table()
> - {
> - return (changes_to_non_trans_temp_table_flag);
> - }
> -
> void reset()
> {
> truncate(0);
> @@ -219,11 +209,27 @@ private:
>
> class binlog_cache_mngr {
> public:
> - binlog_cache_mngr() {}
> + binlog_cache_mngr() : non_trans_table_in_trx_cache(FALSE) {}
>
> - void reset_cache(binlog_cache_data* cache_data)
> + void reset_stmt_cache()
> {
> - cache_data->reset();
> + stmt_cache.reset();
> + }
> +
> + void reset_trx_cache()
> + {
> + trx_cache.reset();
> + non_trans_table_in_trx_cache= FALSE;
> + }
> +
> + void set_non_trans_table_in_trx_cache()
> + {
> + non_trans_table_in_trx_cache= TRUE;
> + }
> +
> + bool is_non_trans_table_in_trx_cache()
> + {
> + return non_trans_table_in_trx_cache;
> }
>
> binlog_cache_data* get_binlog_cache_data(bool is_transactional)
> @@ -244,6 +250,12 @@ private:
>
> binlog_cache_mngr& operator=(const binlog_cache_mngr& info);
> binlog_cache_mngr(const binlog_cache_mngr& info);
> +
> + /*
> + It will be set TRUE if any statement which updates a non-transactional
> + table is put in trx_cache.
> + */
> + bool non_trans_table_in_trx_cache;
> };
>
> /**
> @@ -406,7 +418,7 @@ binlog_flush_trx_cache(THD *thd, binlog_
> */
> error= mysql_bin_log.write(thd,&cache_mngr->trx_cache.cache_log, end_ev,
> cache_mngr->trx_cache.has_incident());
> - cache_mngr->reset_cache(&cache_mngr->trx_cache);
> + cache_mngr->reset_trx_cache();
>
> statistic_increment(binlog_cache_use,&LOCK_status);
> if (cache_log->disk_writes != 0)
> @@ -456,7 +468,7 @@ binlog_truncate_trx_cache(THD *thd, binl
> if (cache_mngr->trx_cache.has_incident())
> error= mysql_bin_log.write_incident(thd, TRUE);
>
> - cache_mngr->reset_cache(&cache_mngr->trx_cache);
> + cache_mngr->reset_trx_cache();
>
> thd->clear_binlog_table_maps();
> }
> @@ -516,7 +528,7 @@ binlog_flush_stmt_cache(THD *thd, binlog
> if ((error= mysql_bin_log.write(thd, cache_log,&qev,
> cache_mngr->stmt_cache.has_incident())))
> DBUG_RETURN(error);
> - cache_mngr->reset_cache(&cache_mngr->stmt_cache);
> + cache_mngr->reset_stmt_cache();
>
> statistic_increment(binlog_cache_use,&LOCK_status);
> if (cache_log->disk_writes != 0)
> @@ -563,7 +575,7 @@ static int binlog_commit(handlerton *hto
> /*
> we're here because cache_log was flushed in MYSQL_BIN_LOG::log_xid()
> */
> - cache_mngr->reset_cache(&cache_mngr->trx_cache);
> + cache_mngr->reset_trx_cache();
> DBUG_RETURN(0);
> }
>
> @@ -616,7 +628,7 @@ static int binlog_rollback(handlerton *h
> if (cache_mngr->stmt_cache.has_incident())
> {
> error= mysql_bin_log.write_incident(thd, TRUE);
> - cache_mngr->reset_cache(&cache_mngr->stmt_cache);
> + cache_mngr->reset_stmt_cache();
> }
> else if (!cache_mngr->stmt_cache.empty())
> {
> @@ -628,7 +640,7 @@ static int binlog_rollback(handlerton *h
> /*
> we're here because cache_log was flushed in MYSQL_BIN_LOG::log_xid()
> */
> - cache_mngr->reset_cache(&cache_mngr->trx_cache);
> + cache_mngr->reset_trx_cache();
> DBUG_RETURN(0);
> }
>
> @@ -646,57 +658,21 @@ static int binlog_rollback(handlerton *h
> a cache and need to be rolled back.
> */
> error= binlog_truncate_trx_cache(thd, cache_mngr, all);
> + DBUG_RETURN(error);
> }
> - else
> - {
> - /*
> - We flush the cache wrapped in a beging/rollback if:
> - . aborting a single or multi-statement transaction and;
> - . the format is STMT and non-trans engines were updated or;
> - . the OPTION_KEEP_LOG is activate.
> - . the OPTION_KEEP_LOG is active or;
> - . the format is STMT and a non-trans table was updated or;
> - . the format is MIXED and a temporary non-trans table was
> - updated or;
> - . the format is MIXED, non-trans table was updated and
> - aborting a single statement transaction;
> - */
> - if (ending_trans(thd, all)&&
> - ((thd->variables.option_bits& OPTION_KEEP_LOG) ||
> - (trans_has_updated_non_trans_table(thd)&&
> - thd->variables.binlog_format == BINLOG_FORMAT_STMT) ||
> - (cache_mngr->trx_cache.changes_to_non_trans_temp_table()&&
> - thd->variables.binlog_format == BINLOG_FORMAT_MIXED) ||
> - (trans_has_updated_non_trans_table(thd)&&
> - ending_single_stmt_trans(thd,all)&&
> - thd->variables.binlog_format == BINLOG_FORMAT_MIXED)))
> +
> + if (ending_trans(thd, all))
> + {
> + if (cache_mngr->is_non_trans_table_in_trx_cache())
> {
> Query_log_event qev(thd, STRING_WITH_LEN("ROLLBACK"), TRUE, FALSE, TRUE, 0);
> error= binlog_flush_trx_cache(thd, cache_mngr,&qev);
> }
> - /*
> - Truncate the cache if:
> - . aborting a single or multi-statement transaction or;
> - . the OPTION_KEEP_LOG is not activate and;
> - . the format is not STMT or no non-trans were updated.
> - . the OPTION_KEEP_LOG is not active and;
> - . the format is not STMT or no non-trans was updated and;
> - . the format is not MIXED or no temporary non-trans was
> - updated.
> - */
> - else if (ending_trans(thd, all) ||
> - (!(thd->variables.option_bits& OPTION_KEEP_LOG)&&
> - (!stmt_has_updated_non_trans_table(thd) ||
> - thd->variables.binlog_format != BINLOG_FORMAT_STMT)&&
> - (!cache_mngr->trx_cache.changes_to_non_trans_temp_table() ||
> - thd->variables.binlog_format != BINLOG_FORMAT_MIXED)))
> + else
> error= binlog_truncate_trx_cache(thd, cache_mngr, all);
> }
> - /*
> - This is part of the stmt rollback.
> - */
> - if (!all)
> - cache_mngr->trx_cache.set_prev_position(MY_OFF_T_UNDEF);
> + else
> + cache_mngr->trx_cache.set_prev_position(MY_OFF_T_UNDEF);
> DBUG_RETURN(error);
> }
>
> @@ -767,8 +743,10 @@ static int binlog_savepoint_rollback(han
> non-transactional table. Otherwise, truncate the binlog cache starting
> from the SAVEPOINT command.
> */
> - if (unlikely(trans_has_updated_non_trans_table(thd) ||
> - (thd->variables.option_bits& OPTION_KEEP_LOG)))
> + binlog_cache_mngr *const cache_mngr=
> + (binlog_cache_mngr*) thd_get_ha_data(thd, binlog_hton);
> +
> + if (unlikely(cache_mngr->is_non_trans_table_in_trx_cache()))
> {
> String log_query;
> if (log_query.append(STRING_WITH_LEN("ROLLBACK TO ")) ||
> @@ -3090,6 +3068,8 @@ bool MYSQL_BIN_LOG::write(Log_event *eve
> #endif /* HAVE_REPLICATION */
>
> IO_CACHE *file= NULL;
> + bool is_trans_cache= FALSE;
> + binlog_cache_mngr *cache_mngr= NULL;
>
> if (event_info->use_direct_logging())
> {
> @@ -3101,16 +3081,11 @@ bool MYSQL_BIN_LOG::write(Log_event *eve
> if (thd->binlog_setup_trx_data())
> goto err;
>
> - binlog_cache_mngr *const cache_mngr=
> - (binlog_cache_mngr*) thd_get_ha_data(thd, binlog_hton);
> -
> - bool is_trans_cache= use_trans_cache(thd, event_info->use_trans_cache());
> + cache_mngr= (binlog_cache_mngr*) thd_get_ha_data(thd, binlog_hton);
> + is_trans_cache= use_trans_cache(thd, event_info->use_trans_cache());
> file= cache_mngr->get_binlog_cache_log(is_trans_cache);
> cache_data= cache_mngr->get_binlog_cache_data(is_trans_cache);
>
> - if (thd->lex->stmt_accessed_non_trans_temp_table())
> - cache_data->set_changes_to_non_trans_temp_table();
> -
> thd->binlog_start_trans_and_stmt();
> }
> DBUG_PRINT("info",("event type: %d",event_info->get_type_code()));
> @@ -3185,7 +3160,8 @@ bool MYSQL_BIN_LOG::write(Log_event *eve
> goto err;
>
> error= 0;
> -
> + if (is_trans_cache&&
> thd->transaction.stmt.modified_non_trans_table)
> + cache_mngr->set_non_trans_table_in_trx_cache();
> err:
> if (event_info->use_direct_logging())
> {
>
> === modified file 'sql/rpl_slave.cc'
> --- a/sql/rpl_slave.cc 2010-11-16 12:38:17 +0000
> +++ b/sql/rpl_slave.cc 2010-11-29 05:36:21 +0000
> @@ -1008,17 +1008,7 @@ static bool sql_slave_killed(THD* thd, R
> DBUG_ASSERT(rli->slave_running == 1);// tracking buffer overrun
> if (abort_loop || thd->killed || rli->abort_slave)
> {
> - /*
> - The transaction should always be binlogged if OPTION_KEEP_LOG is set
> - (it implies that something can not be rolled back). And such case
> - should be regarded similarly as modifing a non-transactional table
> - because retrying of the transaction will lead to an error or inconsistency
> - as well.
> - Example: OPTION_KEEP_LOG is set if a temporary table is created or dropped.
> - */
> - if ((thd->transaction.all.modified_non_trans_table ||
> - (thd->variables.option_bits& OPTION_KEEP_LOG))
> -&& rli->is_in_group())
> + if (thd->transaction.all.modified_non_trans_table&&
> rli->is_in_group())
> {
> char msg_stopped[]=
> "... The slave SQL is stopped, leaving the current group "
> @@ -2796,7 +2786,8 @@ static int exec_relay_log_event(THD* thd
> if (slave_trans_retries)
> {
> int UNINIT_VAR(temp_err);
> - if (exec_res&& (temp_err= has_temporary_error(thd)))
> + if (exec_res&& (temp_err= has_temporary_error(thd))&&
> + !thd->transaction.all.modified_non_trans_table)
> {
> const char *errmsg;
> /*
>
> === modified file 'sql/sql_parse.cc'
> --- a/sql/sql_parse.cc 2010-10-27 14:46:44 +0000
> +++ b/sql/sql_parse.cc 2010-11-29 05:36:21 +0000
> @@ -2463,10 +2463,6 @@ case SQLCOM_PREPARE:
> */
> lex->unlink_first_table(&link_to_local);
>
> - /* So that CREATE TEMPORARY TABLE gets to binlog at commit/rollback */
> - if (create_info.options& HA_LEX_CREATE_TMP_TABLE)
> - thd->variables.option_bits|= OPTION_KEEP_LOG;
> -
> /*
> select_create is currently not re-execution friendly and
> needs to be created for every execution of a PS/SP.
> @@ -2492,9 +2488,6 @@ case SQLCOM_PREPARE:
> }
> else
> {
> - /* So that CREATE TEMPORARY TABLE gets to binlog at commit/rollback */
> - if (create_info.options& HA_LEX_CREATE_TMP_TABLE)
> - thd->variables.option_bits|= OPTION_KEEP_LOG;
> /* regular create */
> if (create_info.options& HA_LEX_CREATE_TABLE_LIKE)
> {
> @@ -2511,6 +2504,11 @@ case SQLCOM_PREPARE:
> if (!res)
> my_ok(thd);
> }
> + /* So that CREATE TEMPORARY TABLE gets to binlog at commit/rollback */
> + if (!res&& (create_info.options&
> HA_LEX_CREATE_TMP_TABLE)&&
> + thd->in_multi_stmt_transaction_mode()&&
> + !thd->is_current_stmt_binlog_format_row())
> + thd->transaction.stmt.modified_non_trans_table= TRUE;
>
> end_with_restore_list:
> break;
> @@ -3015,11 +3013,6 @@ end_with_restore_list:
> if (check_table_access(thd, DROP_ACL, all_tables, FALSE, UINT_MAX, FALSE))
> goto error; /* purecov: inspected */
> }
> - else
> - {
> - /* So that DROP TEMPORARY TABLE gets to binlog at commit/rollback */
> - thd->variables.option_bits|= OPTION_KEEP_LOG;
> - }
> /* DDL and binlog write order are protected by metadata locks. */
> res= mysql_rm_table(thd, first_table, lex->drop_if_exists,
> lex->drop_temporary);
> @@ -5248,14 +5241,8 @@ void THD::reset_for_next_command()
> beginning of each SQL statement.
> */
> thd->server_status&= ~SERVER_STATUS_CLEAR_SET;
> - /*
> - If in autocommit mode and not in a transaction, reset
> - OPTION_STATUS_NO_TRANS_UPDATE | OPTION_KEEP_LOG to not get warnings
> - in ha_rollback_trans() about some tables couldn't be rolled back.
> - */
> if (!thd->in_multi_stmt_transaction_mode())
> {
> - thd->variables.option_bits&= ~OPTION_KEEP_LOG;
> thd->transaction.all.modified_non_trans_table= FALSE;
> }
> DBUG_ASSERT(thd->security_ctx==&thd->main_security_ctx);
>
> === modified file 'sql/sql_priv.h'
> --- a/sql/sql_priv.h 2010-11-05 16:23:32 +0000
> +++ b/sql/sql_priv.h 2010-11-29 05:36:21 +0000
> @@ -101,7 +101,7 @@
> #define OPTION_BEGIN (1ULL<< 20) // THD, intern
> #define OPTION_TABLE_LOCK (1ULL<< 21) // THD, intern
> #define OPTION_QUICK (1ULL<< 22) // SELECT (for DELETE)
> -#define OPTION_KEEP_LOG (1ULL<< 23) // THD, user
> +//#define OPTION_KEEP_LOG (1ULL<< 23) // THD, user
>
> /* The following is used to detect a conflict with DISTINCT */
> #define SELECT_ALL (1ULL<< 24) // SELECT, user, parser
>
> === modified file 'sql/sql_table.cc'
> --- a/sql/sql_table.cc 2010-11-11 09:40:06 +0000
> +++ b/sql/sql_table.cc 2010-11-29 05:36:21 +0000
> @@ -2244,6 +2244,14 @@ int mysql_rm_table_part2(THD *thd, TABLE
> goto err;
> }
>
> + /*
> + DROP TEMPORARY TABLE doesn't terminate a transaction. Setting
> + modified_non_trans_table TRUE guarantees the transaction can be binlogged
> + scorrectly.
> + */
> + if (drop_temporary&& error == 0)
> + thd->transaction.stmt.modified_non_trans_table= TRUE;
> +
> if ((drop_temporary&& if_exists) || !error)
> {
> /*
>
> === modified file 'sql/sys_vars.cc'
> --- a/sql/sys_vars.cc 2010-11-16 09:38:43 +0000
> +++ b/sql/sys_vars.cc 2010-11-29 05:36:21 +0000
> @@ -2260,7 +2260,7 @@ static bool fix_autocommit(sys_var *self
> transaction implicitly at the end (@sa stmt_causes_implicitcommit()).
> */
> thd->variables.option_bits&=
> - ~(OPTION_BEGIN | OPTION_KEEP_LOG | OPTION_NOT_AUTOCOMMIT);
> + ~(OPTION_BEGIN | OPTION_NOT_AUTOCOMMIT);
> thd->transaction.all.modified_non_trans_table= false;
> thd->server_status|= SERVER_STATUS_AUTOCOMMIT;
> return false;
>
> === modified file 'sql/transaction.cc'
> --- a/sql/transaction.cc 2010-10-01 10:23:16 +0000
> +++ b/sql/transaction.cc 2010-11-29 05:36:21 +0000
> @@ -110,7 +110,7 @@ bool trans_begin(THD *thd, uint flags)
> res= test(ha_commit_trans(thd, TRUE));
> }
>
> - thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
> + thd->variables.option_bits&= ~OPTION_BEGIN;
> thd->transaction.all.modified_non_trans_table= FALSE;
>
> if (res)
> @@ -159,7 +159,7 @@ bool trans_commit(THD *thd)
> RUN_HOOK(transaction, after_rollback, (thd, FALSE));
> else
> RUN_HOOK(transaction, after_commit, (thd, FALSE));
> - thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
> + thd->variables.option_bits&= ~OPTION_BEGIN;
> thd->transaction.all.modified_non_trans_table= FALSE;
> thd->lex->start_transaction_opt= 0;
>
> @@ -196,7 +196,7 @@ bool trans_commit_implicit(THD *thd)
> res= test(ha_commit_trans(thd, TRUE));
> }
>
> - thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
> + thd->variables.option_bits&= ~OPTION_BEGIN;
> thd->transaction.all.modified_non_trans_table= FALSE;
>
> /*
> @@ -231,7 +231,7 @@ bool trans_rollback(THD *thd)
> thd->server_status&= ~SERVER_STATUS_IN_TRANS;
> res= ha_rollback_trans(thd, TRUE);
> RUN_HOOK(transaction, after_rollback, (thd, FALSE));
> - thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
> + thd->variables.option_bits&= ~OPTION_BEGIN;
> thd->transaction.all.modified_non_trans_table= FALSE;
> thd->lex->start_transaction_opt= 0;
>
> @@ -436,9 +436,7 @@ bool trans_rollback_to_savepoint(THD *th
>
> if (ha_rollback_to_savepoint(thd, sv))
> res= TRUE;
> - else if (((thd->variables.option_bits& OPTION_KEEP_LOG) ||
> - thd->transaction.all.modified_non_trans_table)&&
> - !thd->slave_thread)
> + else if (thd->transaction.all.modified_non_trans_table&&
> !thd->slave_thread)
> push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
> ER_WARNING_NOT_COMPLETE_ROLLBACK,
> ER(ER_WARNING_NOT_COMPLETE_ROLLBACK));
> @@ -664,7 +662,7 @@ bool trans_xa_commit(THD *thd)
> DBUG_RETURN(TRUE);
> }
>
> - thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
> + thd->variables.option_bits&= ~OPTION_BEGIN;
> thd->transaction.all.modified_non_trans_table= FALSE;
> thd->server_status&= ~SERVER_STATUS_IN_TRANS;
> xid_cache_delete(&thd->transaction.xid_state);
> @@ -719,7 +717,7 @@ bool trans_xa_rollback(THD *thd)
> if ((res= test(ha_rollback_trans(thd, TRUE))))
> my_error(ER_XAER_RMERR, MYF(0));
>
> - thd->variables.option_bits&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
> + thd->variables.option_bits&= ~OPTION_BEGIN;
> thd->transaction.all.modified_non_trans_table= FALSE;
> thd->server_status&= ~SERVER_STATUS_IN_TRANS;
> xid_cache_delete(&thd->transaction.xid_state);
>
>
>
>
>
