Hi Jasonh,
Thank you for your comments.
I think I've addressed all your requests.
Cheers-
He Zhenxing wrote:
> Hi Alfranio,
>
> I think you're right, then basically I have not other comments, I think
> the patch is OK to push if nothing new comes up during our meeting
> today. Thank you for the great work!
>
> Alfranio Correia wrote:
>
>> He Zhenxing wrote:
>>
>>> Hi Alfranio,
>>>
>>> Great work!
>>>
>>> I think the patch is almost OK, I have one proposal about M statement,
>>> currently we consider all statement that referenced both N-table and
>>> T-table as unsafe, I think there need a refinement, we should only
>>> consider tables that are modified within the transaction before this
>>> statement or will be modified by this statement, and ignore tables that
>>> are not modified within the transaction before and are not for update.
>>>
>>> Let's assume 't' as a trans table, and 'n' as a non-trans table, so the
>>> following INSERT .. SELECT statements will be considered an N or T
>>> statement, not an M-statement, and so should not generate unsafe errors
>>> nor switch to row format in mixed mode:
>>> BEGIN;
>>> INSERT INTO n SELECT * FROM t; /* This is an N statement, safe */
>>> COMMIT;
>>>
>>> BEGIN;
>>> INSERT INTO t SELECT * FROM n; /* This is an T statemet, safe */
>>> COMMIT;
>>>
>>> But this INSERT .. SELECT is considered as an M-statement:
>>> BEGIN;
>>> INSERT INTO t VALUES (1);
>>> INSERT INTO n SELECT * FROM t; /* This is an M statement, unsafe */
>>> COMMIT;
>>>
>>> BEGIN;
>>> INSERT INTO n VALUES (1);
>>> INSERT INTO t SELECT * FROM n; /* Because the above statement will be logged
> separately, so this is a *T* statement, safe */
>>> COMMIT;
>>>
>>> In nuts shell, if a table is not for update, and it has not been
>>> modified within the transaction before this statement, then this table
>>> will be ignored when deciding if the statement is N,T or M.
>>>
>>> Please see also comments below!
>>>
>>>
>> Hi Jasonh,
>>
>> I've handled some of the things you requested in your comments below,
>> but I will reply again as soon as I handle everything. I just want to
>> give you
>> a feedback on your proposal on how to classify a statement as unsafe.
>>
>> I put Andrei in CC and if you want we can discuss your idea tomorrow.
>>
>> First, recall that an unsafe statement will have rows written into the
>> binary log
>> and that the mixed mode should be equivalent to the row mode in the sense
>> that it should produce the same result under any circumstance.
>>
>> Said that, let's analyze the following case:
>>
>> T1, T2 are transactions executed on behalf of different connections.
>>
>> 1 - Populating t:
>>
>> CREATE TABLE t(a int, PRIMARY KEY(a));
>> T1: INSERT INTO t VALUES (1);
>> T1: INSERT INTO t VALUES (100);
>> T1: INSERT INTO t VALUES (200);
>> T1: INSERT INTO t VALUES (300);
>>
>>
>> T1: BEGIN
>> T1: INSERT INTO n SELECT * FROM t;
>> T2: INSERT INTO t VALUES (250);
>> T1: INSERT INTO t VALUES (50);
>> T1: COMMIT;
>>
>> The point is that the concurrent statements may be written into the
>> binary in way different from the execution.
>> For instance,
>>
>> BINARY LOG:
>>
>> INSERT INTO t VALUES (250);
>> INSERT INTO n SELECT * FROM t;
>> ....
>>
>> or
>>
>> BINARY LOG:
>>
>> INSERT INTO n SELECT * FROM t;
>> INSERT INTO t VALUES (250);
>> ....
>>
>> Is this possible right? If it is not, please, tell me why.
>> So, assuming that it is possible, the slave and the master may diverge
>> if we adopt your proposal.
>> By using the current implementation which classifies the INSERT INTO n
>> SELECT * FROM t
>> as unsafe, we would be propagating rows and as such the ordering would
>> not matter.
>>
>> I also would like to hear from you if there would be any counterexample
>> where the mixed mode
>> would not be safe.
>>
>> Cheers.
>>
>>
>>> Alfranio Correia wrote:
>>>
>>>
>>>> #At
> file:///home/acorreia/workspace.sun/repository.mysql/bzrwork/bug-40278/mysql-5.1-bugteam/
> based on revid:staale.smedseng@stripped
>>>>
>>>> 2911 Alfranio Correia 2009-06-06
>>>> WL#2687
>>>>
>>>> removed:
>>>> mysql-test/suite/rpl/r/rpl_row_basic_11bugs-master.opt
>>>> mysql-test/suite/rpl/r/rpl_row_basic_11bugs-slave.opt
>>>> added:
>>>> mysql-test/suite/rpl/include/rpl_mixed_engines.inc
>>>> mysql-test/suite/rpl/r/rpl_mixing_mixed_engines.result
>>>> mysql-test/suite/rpl/r/rpl_mixing_row_engines.result
>>>> mysql-test/suite/rpl/r/rpl_mixing_stmt_engines.result
>>>> mysql-test/suite/rpl/t/rpl_mixing_mixed_engines.test
>>>> mysql-test/suite/rpl/t/rpl_mixing_row_engines.test
>>>> mysql-test/suite/rpl/t/rpl_mixing_stmt_engines.test
>>>>
--------
>>>>
>>>>
>>> I think the convention is to put the mode name (row,stm,mixed) right
>>> after the 'rpl_' prefix in the test names, so I'd suggest to name these
>>> as:
>>> rpl_row_mixing_engines.test
>>> rpl_stm_mixing_engines.test
>>> rpl_mixed_mixing_engines.test
>>>
--------
done.
>>>
>>>
>>>> modified:
>>>> mysql-test/extra/rpl_tests/rpl_flsh_tbls.test
>>>> mysql-test/extra/rpl_tests/rpl_insert_delayed.test
>>>> mysql-test/extra/rpl_tests/rpl_insert_ignore.test
>>>> mysql-test/extra/rpl_tests/rpl_log.test
>>>> mysql-test/r/commit_1innodb.result
>>>> mysql-test/suite/binlog/r/binlog_database.result
>>>> mysql-test/suite/binlog/r/binlog_multi_engine.result
>>>> mysql-test/suite/binlog/r/binlog_row_binlog.result
>>>> mysql-test/suite/binlog/r/binlog_row_ctype_ucs.result
>>>> mysql-test/suite/binlog/r/binlog_row_insert_select.result
>>>> mysql-test/suite/binlog/r/binlog_row_mix_innodb_myisam.result
>>>> mysql-test/suite/binlog/r/binlog_stm_binlog.result
>>>> mysql-test/suite/binlog/r/binlog_stm_mix_innodb_myisam.result
>>>> mysql-test/suite/binlog/r/binlog_stm_row.result
>>>> mysql-test/suite/binlog/t/binlog_incident.test
>>>> mysql-test/suite/ndb/r/ndb_binlog_format.result
>>>> mysql-test/suite/rpl/r/rpl_blackhole.result
>>>> mysql-test/suite/rpl/r/rpl_extraCol_myisam.result
>>>> mysql-test/suite/rpl/r/rpl_innodb.result
>>>> mysql-test/suite/rpl/r/rpl_loaddata_fatal.result
>>>> mysql-test/suite/rpl/r/rpl_log_pos.result
>>>> mysql-test/suite/rpl/r/rpl_rbr_to_sbr.result
>>>> mysql-test/suite/rpl/r/rpl_row_basic_11bugs.result
>>>> mysql-test/suite/rpl/r/rpl_row_conflicts.result
>>>> mysql-test/suite/rpl/r/rpl_row_create_table.result
>>>> mysql-test/suite/rpl/r/rpl_row_flsh_tbls.result
>>>> mysql-test/suite/rpl/r/rpl_row_insert_delayed.result
>>>> mysql-test/suite/rpl/r/rpl_row_log.result
>>>> mysql-test/suite/rpl/r/rpl_row_log_innodb.result
>>>> mysql-test/suite/rpl/r/rpl_sp.result
>>>> mysql-test/suite/rpl/r/rpl_stm_flsh_tbls.result
>>>> mysql-test/suite/rpl/r/rpl_stm_insert_delayed.result
>>>> mysql-test/suite/rpl/r/rpl_stm_log.result
>>>> mysql-test/suite/rpl/t/rpl_blackhole.test
>>>> mysql-test/suite/rpl/t/rpl_empty_master_crash.test
>>>> mysql-test/suite/rpl/t/rpl_innodb.test
>>>> mysql-test/suite/rpl/t/rpl_invoked_features.test
>>>> mysql-test/suite/rpl/t/rpl_loaddata_fatal.test
>>>> mysql-test/suite/rpl/t/rpl_log_pos.test
>>>> mysql-test/suite/rpl/t/rpl_read_only.test
>>>> mysql-test/suite/rpl/t/rpl_row_basic_11bugs.test
>>>> mysql-test/suite/rpl/t/rpl_row_flsh_tbls.test
>>>> mysql-test/suite/rpl/t/rpl_row_until.test
>>>> mysql-test/suite/rpl/t/rpl_stm_flsh_tbls.test
>>>> mysql-test/suite/rpl/t/rpl_trigger.test
>>>>
> mysql-test/suite/rpl_ndb/r/rpl_ndb_mixed_engines_transactions.result
>>>> sql/ha_ndbcluster_binlog.cc
>>>> sql/handler.cc
>>>> sql/handler.h
>>>> sql/log.cc
>>>> sql/log.h
>>>> sql/log_event.cc
>>>> sql/log_event.h
>>>> sql/log_event_old.cc
>>>> sql/rpl_injector.cc
>>>> sql/set_var.cc
>>>> sql/sp.cc
>>>> sql/sp_head.cc
>>>> sql/sql_acl.cc
>>>> sql/sql_base.cc
>>>> sql/sql_class.cc
>>>> sql/sql_class.h
>>>> sql/sql_delete.cc
>>>> sql/sql_insert.cc
>>>> sql/sql_load.cc
>>>> sql/sql_parse.cc
>>>> sql/sql_table.cc
>>>> sql/sql_update.cc
>>>> sql/sql_view.cc
>>>> === modified file 'mysql-test/extra/rpl_tests/rpl_flsh_tbls.test'
>>>> --- a/mysql-test/extra/rpl_tests/rpl_flsh_tbls.test 2008-12-12 11:34:18
> +0000
>>>> +++ b/mysql-test/extra/rpl_tests/rpl_flsh_tbls.test 2009-06-06 17:51:51
> +0000
>>>> @@ -20,19 +20,13 @@ rename table t1 to t5, t2 to t1;
>>>> # first don't write it to the binlog, to test the NO_WRITE_TO_BINLOG
> keyword.
>>>> flush no_write_to_binlog tables;
>>>> # Check that it's not in the binlog.
>>>> ---replace_result $SERVER_VERSION SERVER_VERSION
>>>> ---replace_column 2 # 5 #
>>>> ---replace_regex /table_id: [0-9]+/table_id: #/
>>>> -eval SHOW BINLOG EVENTS FROM $rename_event_pos ;
>>>> +source include/show_binlog_events2.inc;
>>>>
--------
>>>>
>>>>
>>> I'd suggest to use:
>>> let $pos= query_get_value("SHOW MASTER STATUS", Position, 1);
>>> to get the desired position.
>>>
>>> [snip]
>>>
--------
Done.
--------
>>>
>>>
>>>> @@ -430,7 +404,7 @@ select
>>>> @a not like "%#%error_code=%error_code=%";
>>>> @a like "%#%error_code=0%ROLLBACK\n/*!*/;%ROLLBACK /* added by
> mysqlbinlog */;%" OR
>>>> @a like "%#%error_code=0%ROLLBACK\r\n/*!*/;%ROLLBACK /* added by
> mysqlbinlog */;%" @a not like "%#%error_code=%error_code=%"
>>>> -1 1
>>>> +0 1
>>>>
>>>>
>>> This looks like a test problem, below is some codes in the test case
>>> before the above output:
>>>
>>> if (`select @@binlog_format = 'ROW'`)
>
>>> {
>
>>> --exec $MYSQL_BINLOG --start-position=524 $MYSQLD_DATADIR/master-bin.000001
> > $MYSQLTEST_VARDIR/tmp/mix_innodb_myisam_binlog.output
>>> }
>
>>>
>
>>> if (`select @@binlog_format = 'STATEMENT' || @@binlog_format = 'MIXED'`)
>
>>> {
>
>>> --exec $MYSQL_BINLOG --start-position=555 $MYSQLD_DATADIR/master-bin.000001
> > $MYSQLTEST_VARDIR/tmp/mix_innodb_myisam_binlog.output
>>> }
>>>
>>> this part of the test used absolute binlog positions, and that can be
>>> problematic, probably the binlog positions should be changed
>>> accordingly, please double check that the test case still works as
>>> expected.
>>>
>>> [snip]
>>>
--------
Done.
>>>
>>>
>>>> === added file 'mysql-test/suite/rpl/include/rpl_mixed_engines.inc'
>>>> --- a/mysql-test/suite/rpl/include/rpl_mixed_engines.inc 1970-01-01
> 00:00:00 +0000
>>>> +++ b/mysql-test/suite/rpl/include/rpl_mixed_engines.inc 2009-06-06
> 17:51:51 +0000
>>>> @@ -0,0 +1,905 @@
>>>>
> +###################################################################################
>>>> +# This test checks if transactions that mixes transactional and
> non-transactional
>>>> +# tables are correctly handled. Thus to check the behavior provided by
> the current
>>>> +# code we devide the test as follows:
>>>> +#
>>>> +# 1 - CREATING TABLES through SELECT * FROM
>>>> +# 2 - MIXING TRANSACTIONAL and NON-TRANSACTIONAL TABLES
>>>> +# 3 - CONCURRENCY:
>>>> +# 2.1 - NON-TRANSACT TABLES - SET AUTOCOMMIT = 0 | COMMIT
>>>> +# 2.2 - NON-TRANSACT TABLES - SET AUTOCOMMIT = 1 | START - COMMIT
>>>> +# 4 - SAVE POINTS
>>>>
> +###################################################################################
>>>> +
>>>> +--echo
> ###################################################################################
>>>> +--echo # CONFIGURATION
>>>> +--echo
> ###################################################################################
>>>> +connection master;
>>>> +
>>>> +CREATE TABLE t1 (a text, b int PRIMARY KEY, c text) ENGINE = MyISAM;
>>>> +CREATE TABLE t2 (a text, b int PRIMARY KEY, c text) ENGINE = Innodb;
>>>> +CREATE TABLE t3 (a text, b int PRIMARY KEY, c text) ENGINE = Innodb;
>>>> +CREATE TABLE t4 (a text, b int PRIMARY KEY, c text) ENGINE = Innodb;
>>>> +CREATE TABLE t5 (a text, b int PRIMARY KEY, c text) ENGINE = MyISAM;
>>>> +CREATE TABLE t6 (a text, b int PRIMARY KEY, c text) ENGINE = Innodb;
>>>> +CREATE TABLE t7 (a text, b int PRIMARY KEY, c text) ENGINE = Innodb;
>>>> +CREATE TABLE t8 (a text, b int PRIMARY KEY, c text) ENGINE = MyISAM;
>>>> +CREATE TABLE t9 (a text, b int PRIMARY KEY, c text) ENGINE = MyISAM;
>>>>
--------
>>>>
>>>>
>>> I'd suggest to name all trans-tables t1,t2... and non-trans tables n1,
>>> n2..., so that's easier to understand the test.
>>>
--------
Done.
>>>
>>>
>>>> +
>>>> +INSERT INTO t1 VALUES("text 1", 1, "text 1"), ("text 2", 2, "text 2"),
> ("text 3", 3, "text 3");
>>>> +INSERT INTO t2 VALUES("text 1", 1, "text 1"), ("text 2", 2, "text 2"),
> ("text 3", 3, "text 3");
>>>> +INSERT INTO t3 VALUES("text 1", 1, "text 1"), ("text 2", 2, "text 2"),
> ("text 3", 3, "text 3");
>>>> +INSERT INTO t4 VALUES("text 1", 1, "text 1"), ("text 2", 2, "text 2"),
> ("text 3", 3, "text 3");
>>>> +INSERT INTO t5 VALUES("text 1", 1, "text 1"), ("text 2", 2, "text 2"),
> ("text 3", 3, "text 3");
>>>> +
>>>> +DELIMITER |;
>>>> +
>>>> +CREATE FUNCTION f1 () RETURNS VARCHAR(64)
>>>> +BEGIN
>>>> + RETURN "Testing...";
>>>> +END|
>>>> +
>>>> +CREATE FUNCTION f2 () RETURNS VARCHAR(64)
>>>> +BEGIN
>>>> + RETURN f1();
>>>> +END|
>>>> +
>>>> +CREATE PROCEDURE pc6 (IN x INT, IN y VARCHAR(64))
>>>> +BEGIN
>>>> + INSERT INTO t6 VALUES (y,x,x);
>>>> +END|
>>>> +
>>>> +CREATE TRIGGER tr6_i BEFORE INSERT ON t6 FOR EACH ROW
>>>> +BEGIN
>>>> + INSERT INTO t7 VALUES (NEW.a, NEW.b, NEW.c);
>>>> + INSERT INTO t8 VALUES (NEW.a, NEW.b, NEW.c);
>>>> + INSERT INTO t9 VALUES (NEW.a, NEW.b, NEW.c);
>>>> +END|
>>>> +
>>>> +DELIMITER ;|
>>>> +
>>>> +INSERT INTO t6 VALUES("text 100", 100, "text 100"), ("text 200", 200,
> "text 200"), ("text 300", 300, "text 300");
>>>> +INSERT INTO t7 VALUES("text 1", 1, "text 1"), ("text 2", 2, "text 2"),
> ("text 3", 3, "text 3");
>>>> +INSERT INTO t8 VALUES("text 1", 1, "text 1"), ("text 2", 2, "text 2"),
> ("text 3", 3, "text 3");
>>>> +INSERT INTO t9 VALUES("text 1", 1, "text 1"), ("text 2", 2, "text 2"),
> ("text 3", 3, "text 3");
>>>> +
>>>> +--echo
> ###################################################################################
>>>> +--echo # 1 - CREATING TABLES
>>>> +--echo
> ###################################################################################
>>>> +connection master;
>>>> +
>>>> +CREATE TABLE t10 SELECT * FROM t2;
>>>> +DROP TABLE t10;
>>>> +CREATE TABLE t11 SELECT * FROM t2;
>>>> +DROP TABLE t11;
>>>> +
>>>> +
>>>> +--echo
> ###################################################################################
>>>> +--echo # 2 - MIXING TRANSACTIONAL and NON-TRANSACTIONAL
> TABLES
>>>> +--echo
> ###################################################################################
>>>> +connection master;
>>>> +
>>>> +# Caching (C): (stmt-cache (s), trx-cache (t), ignored (i))
>>>> +# Flushing (F): (upon committing/rollingback a statement (d), upon
>>>> +# committing/rolling back a transaction (c/r), ignored
> (i))
>>>> +
>>>> +#
>>>> +# STMT ROW MIXED
>>>> +#
>>>> +
>>>> +let $binlog_start= query_get_value("SHOW MASTER STATUS", Position, 1);
>>>> +--echo # C - T C - T C - T
>>>> +--echo #1.a) B T T C F - C F - C F - C
>>>> +--echo #
>>>> +BEGIN;
>>>> +INSERT INTO t2 VALUES ("new text 5", 5, "new text 5");
>>>>
--------
>>>>
>>>>
>>> Please also add show_binlog_events or other methods after statements
>>> within transaction to check that statement cache are correctly flushed..
>>>
>>> [snip]
>>>
>>>
--------
I think you can check this by verifying if the events are outside a
transaction.
Introducing what you have requested you just clutter the test case.
>>>
>>>
>>>> === modified file 'mysql-test/suite/rpl/r/rpl_extraCol_myisam.result'
>>>> --- a/mysql-test/suite/rpl/r/rpl_extraCol_myisam.result 2008-11-04
> 07:43:21 +0000
>>>> +++ b/mysql-test/suite/rpl/r/rpl_extraCol_myisam.result 2009-06-06
> 17:51:51 +0000
>>>> @@ -434,7 +434,7 @@ Replicate_Ignore_Table #
>>>> Replicate_Wild_Do_Table
>>>> Replicate_Wild_Ignore_Table
>>>> Last_Errno 1364
>>>> -Last_Error Could not execute Write_rows event on table test.t9; Field
> 'e' doesn't have a default value, Error_code: 1364; handler error HA_ERR_ROWS_EVENT_APPLY;
> the event's master log master-bin.000001, end_log_pos 330
>>>> +Last_Error Could not execute Write_rows event on table test.t9; Field
> 'e' doesn't have a default value, Error_code: 1364; handler error HA_ERR_ROWS_EVENT_APPLY;
> the event's master log master-bin.000001, end_log_pos 262
>>>>
--------
>>>>
>>>>
>>> Please mask out the log position.
>>>
>>>
--------
Done.
>>>
>>>
>>>> Skip_Counter 0
>>>> Exec_Master_Log_Pos #
>>>> Relay_Log_Space #
>>>> @@ -452,7 +452,7 @@ Master_SSL_Verify_Server_Cert No
>>>> Last_IO_Errno #
>>>> Last_IO_Error #
>>>> Last_SQL_Errno 1364
>>>> -Last_SQL_Error Could not execute Write_rows event on table test.t9;
> Field 'e' doesn't have a default value, Error_code: 1364; handler error
> HA_ERR_ROWS_EVENT_APPLY; the event's master log master-bin.000001, end_log_pos 330
>>>> +Last_SQL_Error Could not execute Write_rows event on table test.t9;
> Field 'e' doesn't have a default value, Error_code: 1364; handler error
> HA_ERR_ROWS_EVENT_APPLY; the event's master log master-bin.000001, end_log_pos 262
>>>> SET GLOBAL SQL_SLAVE_SKIP_COUNTER=2;
>>>> START SLAVE;
>>>> *** Create t10 on slave ***
>>>>
>>>>
>>> [snip]
>>>
>>>
>>>
>>>> === modified file 'mysql-test/suite/rpl/r/rpl_rbr_to_sbr.result'
>>>> --- a/mysql-test/suite/rpl/r/rpl_rbr_to_sbr.result 2008-07-16 09:17:10
> +0000
>>>> +++ b/mysql-test/suite/rpl/r/rpl_rbr_to_sbr.result 2009-06-06 17:51:51
> +0000
>>>> @@ -19,10 +19,8 @@ Log_name Pos Event_type Server_id End_lo
>>>> master-bin.000001 # Format_desc 1 # Server ver: VERSION, Binlog ver: 4
>>>> master-bin.000001 # Query 1 # use `test`; CREATE TABLE t1 (a INT, b
> LONG)
>>>> master-bin.000001 # Query 1 # use `test`; INSERT INTO t1 VALUES (1,1),
> (2,2)
>>>> -master-bin.000001 # Query 1 # use `test`; BEGIN
>>>> master-bin.000001 # Table_map 1 # table_id: # (test.t1)
>>>> master-bin.000001 # Write_rows 1 # table_id: # flags: STMT_END_F
>>>> -master-bin.000001 # Query 1 # use `test`; COMMIT
>>>> **** On Slave ****
>>>> SHOW SLAVE STATUS;
>>>> Slave_IO_State #
>>>> @@ -31,7 +29,7 @@ Master_User root
>>>> Master_Port MASTER_PORT
>>>> Connect_Retry 1
>>>> Master_Log_File master-bin.000001
>>>> -Read_Master_Log_Pos 594
>>>> +Read_Master_Log_Pos 457
>>>>
>>>>
--------
>>> Mask out the position
>>>
--------
Done
>>>
>>>
>>>> Relay_Log_File #
>>>> Relay_Log_Pos #
>>>> Relay_Master_Log_File master-bin.000001
>>>> @@ -46,7 +44,7 @@ Replicate_Wild_Ignore_Table
>>>> Last_Errno 0
>>>> Last_Error
>>>> Skip_Counter 0
>>>> -Exec_Master_Log_Pos 594
>>>> +Exec_Master_Log_Pos 457
>>>> Relay_Log_Space #
>>>> Until_Condition None
>>>> Until_Log_File
>>>> @@ -68,9 +66,7 @@ Log_name Pos Event_type Server_id End_lo
>>>> slave-bin.000001 # Format_desc 2 # Server ver: VERSION, Binlog ver: 4
>>>> slave-bin.000001 # Query 1 # use `test`; CREATE TABLE t1 (a INT, b
> LONG)
>>>> slave-bin.000001 # Query 1 # use `test`; INSERT INTO t1 VALUES (1,1),
> (2,2)
>>>> -slave-bin.000001 # Query 1 # use `test`; BEGIN
>>>> slave-bin.000001 # Table_map 1 # table_id: # (test.t1)
>>>> slave-bin.000001 # Write_rows 1 # table_id: # flags: STMT_END_F
>>>> -slave-bin.000001 # Query 1 # use `test`; COMMIT
>>>> DROP TABLE IF EXISTS t1;
>>>> SET @@global.binlog_format= @old_binlog_format;
>>>>
>>>>
>>> [snip]
>>>
>>>
>>>
>>>> === modified file 'mysql-test/suite/rpl/r/rpl_row_conflicts.result'
>>>> --- a/mysql-test/suite/rpl/r/rpl_row_conflicts.result 2009-01-09 14:12:31
> +0000
>>>> +++ b/mysql-test/suite/rpl/r/rpl_row_conflicts.result 2009-06-06 17:51:51
> +0000
>>>> @@ -24,7 +24,7 @@ a
>>>> 1
>>>> [on slave]
>>>> ---- Wait until slave stops with an error ----
>>>> -Last_SQL_Error = Could not execute Write_rows event on table test.t1;
> Duplicate entry '1' for key 'PRIMARY', Error_code: 1062; handler error
> HA_ERR_FOUND_DUPP_KEY; the event's master log master-bin.000001, end_log_pos 346 (expected
> "duplicate key" error)
>>>> +Last_SQL_Error = Could not execute Write_rows event on table test.t1;
> Duplicate entry '1' for key 'PRIMARY', Error_code: 1062; handler error
> HA_ERR_FOUND_DUPP_KEY; the event's master log master-bin.000001, end_log_pos 278 (expected
> "duplicate key" error)
>>>>
>>>>
>>>>
>>> Mask out the position
>>>
>>>
This does not exist in mysql-pe.
>>>
>>>
>>>> SELECT * FROM t1;
>>>> a
>>>> 1
>>>> @@ -50,7 +50,7 @@ SELECT * FROM t1;
>>>> a
>>>> [on slave]
>>>> ---- Wait until slave stops with an error ----
>>>> -Last_SQL_Error = Could not execute Delete_rows event on table test.t1;
> Can't find record in 't1', Error_code: 1032; handler error HA_ERR_KEY_NOT_FOUND; the
> event's master log master-bin.000001, end_log_pos 982 (expected "can't find record"
> error)
>>>> +Last_SQL_Error = Could not execute Delete_rows event on table test.t1;
> Can't find record in 't1', Error_code: 1032; handler error HA_ERR_KEY_NOT_FOUND; the
> event's master log master-bin.000001, end_log_pos 503 (expected "can't find record"
> error)
>>>> SELECT * FROM t1;
>>>> a
>>>> ---- Resolve the conflict on the slave and restart SQL thread ----
>>>>
>>>>
>>> [snip]
>>>
>>>
>>>
>>>> === modified file 'mysql-test/suite/rpl/t/rpl_blackhole.test'
>>>> --- a/mysql-test/suite/rpl/t/rpl_blackhole.test 2009-01-30 13:44:49
> +0000
>>>> +++ b/mysql-test/suite/rpl/t/rpl_blackhole.test 2009-06-06 17:51:51
> +0000
>>>> @@ -45,9 +45,20 @@ source extra/rpl_tests/rpl_blackhole.tes
>>>> let $statement = INSERT INTO t1 SELECT * FROM t2;
>>>> source extra/rpl_tests/rpl_blackhole.test;
>>>>
>>>> +#
>>>> +# The MASTER has MyISAM as the engine for both tables. The SLAVE has
> Blackhole
>>>> +# on t1 (transactional engine) and MyISAM on t2 (non-transactional
> engine).
>>>> +#
>>>> +# In MIXED MODE, the command below is logged as statement on the MASTER.
> On the
>>>> +# other hand, on the SLAVE, it is tagged as unsafe and then executed in
> ROW
>>>> +# MODE. In a nutshell, statements that mixes transactional and
> non-transactional
>>>> +# engines are tagged as unsafe. This behavior generates a problem as no
> rows are
>>>> +# returned and makes the # SLAVE diverge from the MASTER.
>>>> +#
>>>> # Test INSERT-SELECT from Blackhole, no primary key
>>>> -let $statement = INSERT INTO t2 SELECT * FROM t1;
>>>> -source extra/rpl_tests/rpl_blackhole.test;
>>>> +#let $statement = INSERT INTO t2 SELECT * FROM t1;
>>>> +#source extra/rpl_tests/rpl_blackhole.test;
>>>> +#
>>>>
>>>>
--------
>>>>
>>>>
>>> If following the proposal at the beginning, then the above statement
>>> will not be considered unsafe, the the test will pass in mixed mode too.
>>>
--------
I disregarded this as we are going to keep the current behavior.
>>>
>>>
>>>> connection master;
>>>> ALTER TABLE t1 ADD PRIMARY KEY pk_t1 (a,b);
>>>>
>>>> === modified file 'mysql-test/suite/rpl/t/rpl_empty_master_crash.test'
>>>> --- a/mysql-test/suite/rpl/t/rpl_empty_master_crash.test 2007-06-27
> 12:29:10 +0000
>>>> +++ b/mysql-test/suite/rpl/t/rpl_empty_master_crash.test 2009-06-06
> 17:51:51 +0000
>>>> @@ -1,6 +1,6 @@
>>>> source include/master-slave.inc;
>>>>
>>>> -source include/show_slave_status.inc;
>>>> +source include/show_slave_status2.inc;
>>>>
>>>> #
>>>> # Load table should not succeed on the master as this is not a slave
>>>>
>>>> === modified file 'mysql-test/suite/rpl/t/rpl_innodb.test'
>>>> --- a/mysql-test/suite/rpl/t/rpl_innodb.test 2008-10-29 13:25:03 +0000
>>>> +++ b/mysql-test/suite/rpl/t/rpl_innodb.test 2009-06-06 17:51:51 +0000
>>>> @@ -111,7 +111,6 @@ connection slave;
>>>> SHOW CREATE TABLE mysqltest1.tmp;
>>>> --error ER_NO_SUCH_TABLE
>>>> SHOW CREATE TABLE mysqltest1.tmp2;
>>>> -# t1 has two rows here: the transaction not rolled back since t1 uses
> MyISAM
>>>>
>>>>
>>> Please check this part of the test, I think it need to be rewritten (or
>>> removed).
>>>
>>>
--------
>>>
>>>
>>>> SELECT COUNT(*) FROM mysqltest1.t1;
>>>> FLUSH LOGS;
>>>>
>>>>
>>>>
>>> [snip]
>>>
>>>
--------
Done. I've updated the comments because in ROW based replication the
behavior is different.
However, I think it is good to leave this part of the code just to
highlight the difference between the
replication modes.
>>>
>>>
>>>> === modified file 'mysql-test/suite/rpl/t/rpl_row_until.test'
>>>> --- a/mysql-test/suite/rpl/t/rpl_row_until.test 2009-02-19 20:29:12
> +0000
>>>> +++ b/mysql-test/suite/rpl/t/rpl_row_until.test 2009-06-06 17:51:51
> +0000
>>>> @@ -14,18 +14,21 @@ INSERT INTO t1 VALUES (1),(2),(3),(4);
>>>> DROP TABLE t1;
>>>> # Save master log postion for query DROP TABLE t1
>>>> save_master_pos;
>>>> -let $master_pos_drop_t1= query_get_value(SHOW BINLOG EVENTS, Pos, 7);
>>>> +let $master_pos_drop_t1= query_get_value(SHOW BINLOG EVENTS, Pos, 5);
>>>> +#show binlog events;
>>>>
>>>>
>>>>
--------
>>> I'd suggest to use:
>>> let $master_pos_drop_t1= query_get_value(SHOW MASTER STATUS, Position, 1);
>>>
>>> right before the DROP statement.
>>>
>>>
--------
Done.
>>>
>>>
>>>> CREATE TABLE t2(n INT NOT NULL AUTO_INCREMENT PRIMARY KEY);
>>>> # Save master log postion for query CREATE TABLE t2
>>>> save_master_pos;
>>>> -let $master_pos_create_t2= query_get_value(SHOW BINLOG EVENTS, Pos, 8);
>>>> +let $master_pos_create_t2= query_get_value(SHOW BINLOG EVENTS, Pos, 6);
>>>> +#show binlog events;
>>>>
>>>> INSERT INTO t2 VALUES (1),(2);
>>>> save_master_pos;
>>>> # Save master log postion for query INSERT INTO t2 VALUES (1),(2);
>>>> -let $master_pos_insert1_t2= query_get_value(SHOW BINLOG EVENTS,
> End_log_pos, 12);
>>>> +let $master_pos_insert1_t2= query_get_value(SHOW BINLOG EVENTS,
> End_log_pos, 8);
>>>> sync_slave_with_master;
>>>> +#show binlog events;
>>>>
>>>> # Save relay log postion for query INSERT INTO t2 VALUES (1),(2);
>>>> let $relay_pos_insert1_t2= query_get_value(show slave status,
> Relay_Log_Pos, 1);
>>>> @@ -34,8 +37,9 @@ connection master;
>>>> INSERT INTO t2 VALUES (3),(4);
>>>> DROP TABLE t2;
>>>> # Save master log postion for query INSERT INTO t2 VALUES (1),(2);
>>>> -let $master_pos_drop_t2= query_get_value(SHOW BINLOG EVENTS,
> End_log_pos, 17);
>>>> +let $master_pos_drop_t2= query_get_value(SHOW BINLOG EVENTS,
> End_log_pos, 11);
>>>> sync_slave_with_master;
>>>> +#show binlog events;
>>>>
>>>> --source include/stop_slave.inc
>>>> # Reset slave.
>>>> @@ -45,6 +49,7 @@ eval CHANGE MASTER TO MASTER_USER='root'
>>>> --enable_query_log
>>>>
>>>> # Try to replicate all queries until drop of t1
>>>> +
>>>> connection slave;
>>>> echo START SLAVE UNTIL MASTER_LOG_FILE='master-bin.000001',
> MASTER_LOG_POS=master_pos_drop_t1;
>>>> --disable_query_log
>>>>
>>>>
>>> [snip]
>>>
>>>
>>>
>>>> === modified file 'sql/ha_ndbcluster_binlog.cc'
>>>> --- a/sql/ha_ndbcluster_binlog.cc 2008-04-09 16:42:05 +0000
>>>> +++ b/sql/ha_ndbcluster_binlog.cc 2009-06-06 17:51:51 +0000
>>>> @@ -257,6 +257,7 @@ static void run_query(THD *thd, char *bu
>>>> thd->query= buf;
>>>> thd->variables.pseudo_thread_id= thread_id;
>>>> thd->transaction.stmt.modified_non_trans_table= FALSE;
>>>> + thd->transaction.stmt.modified_trans_table= FALSE;
>>>> if (disable_binlog)
>>>> thd->options&= ~OPTION_BIN_LOG;
>>>>
>>>> @@ -1857,7 +1858,8 @@ static void ndb_binlog_query(THD *thd, C
>>>> thd->db= schema->db;
>>>> thd->binlog_query(THD::STMT_QUERY_TYPE, schema->query,
>>>> schema->query_length, FALSE,
>>>> - schema->name[0] == 0 || thd->db[0] == 0);
>>>> + schema->name[0] == 0 || thd->db[0] == 0,
>>>> + FALSE, THD::KILLED_NO_VALUE);
>>>>
--------
>>>>
>>>>
>>> Can we add a new argument 'direct' to ndb_binlog_query and pass it down
>>> to THD::binlog_query? So that we can get ride of the DDL hack below.
>>>
--------
Sure. I will try this.
>>>
>>>
>>>> thd->server_id= thd_server_id_save;
>>>> thd->db= thd_db_save;
>>>> }
>>>>
>>>>
>>> [snip]
>>>
>>>
>>>
>>>> /*
>>>> In most cases this is only called if 'is_open()' is true; in fact
> this is
>>>> @@ -3936,7 +4008,6 @@ bool MYSQL_BIN_LOG::write(Log_event *eve
>>>> */
>>>> if (likely(is_open()))
>>>> {
>>>> - IO_CACHE *file= &log_file;
>>>> #ifdef HAVE_REPLICATION
>>>> /*
>>>> In the future we need to add to the following if tests like
>>>> @@ -3946,48 +4017,50 @@ bool MYSQL_BIN_LOG::write(Log_event *eve
>>>> const char *local_db= event_info->get_db();
>>>> if ((thd && !(thd->options & OPTION_BIN_LOG)) ||
>>>> (!binlog_filter->db_ok(local_db)))
>>>> - {
>>>> - VOID(pthread_mutex_unlock(&LOCK_log));
>>>> DBUG_RETURN(0);
>>>> - }
>>>> #endif /* HAVE_REPLICATION */
>>>> + IO_CACHE *file= NULL;
>>>>
>>>> -#if defined(USING_TRANSACTIONS)
>>>> /*
>>>> - Should we write to the binlog cache or to the binlog on disk?
>>>> - Write to the binlog cache if:
>>>> - - it is already not empty (meaning we're in a transaction; note
> that the
>>>> - present event could be about a non-transactional table, but still
> we need
>>>> - to write to the binlog cache in that case to handle updates to
> mixed
>>>> - trans/non-trans table types the best possible in binlogging)
>>>> - - or if the event asks for it (cache_stmt == TRUE).
>>>> + This is an ugly hacking that must be removed as soon as we fix a
> bug in
>>>> + NDB. In a nutshell, this means that we should use the binary log
> directly
>>>> + if we are processing a DDL.
>>>> */
>>>> - if (opt_using_transactions && thd)
>>>> +
>>>> + bool ddl=
>>>> + (!(thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN))
> &&
>>>> + !thd->transaction.stmt.modified_non_trans_table &&
>>>> + !thd->transaction.stmt.modified_trans_table &&
>>>> + !thd->transaction.all.modified_non_trans_table &&
>>>> + !thd->transaction.all.modified_trans_table &&
>>>> + !event_info->is_trans_event());
>>>> +
>>>>
--------
>>>>
>>>>
>>> Hmmm, this looks really really really ugly, why cannot we add change the
>>> interface of ndb_binlog_query to add a new argument 'direct', and pass
>>> it down to this function?
>>>
--------
I will try this.
>>>
>>>
>>>> + direct= direct || ddl;
>>>> +
>>>> + if (direct)
>>>> + {
>>>> + file= &log_file;
>>>> + pthread_mutex_lock(&LOCK_log);
>>>> + }
>>>> + else
>>>> {
>>>> - if (thd->binlog_setup_trx_data())
>>>> + thd->binlog_setup_trx_data();
>>>> goto err;
>>>>
>>>>
>>> Must be a mistake, should not remove the 'if'.
>>>
>>> [snip]
>>>
Sure. Done.
Cheers.
