From: Date: November 23 2007 12:14am Subject: bk commit into 5.1 tree (kostja:1.2607) BUG#12713 List-Archive: http://lists.mysql.com/commits/38320 X-Bug: 12713 Message-Id: <20071122231427.55C2221001@vajra.local> Below is the list of changes that have just been committed into a local 5.1 repository of kostja. When kostja does a push these changes will be propagated to the main repository and, within 24 hours after the push, to the public repository. For information on how to access the public repository see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html ChangeSet@stripped, 2007-11-23 02:14:20+03:00, kostja@bodhi.(none) +20 -0 A fix and a test case for Bug#12713 "Error in a stored function called from a SELECT doesn't cause ROLLBACK of statem" Initial test coverage. Full test coverage is last remaining issue that is left. mysql-test/r/dml_2innodb.result@stripped, 2007-11-23 02:14:17+03:00, kostja@bodhi.(none) +336 -0 New BitKeeper file ``mysql-test/r/dml_2innodb.result'' mysql-test/r/dml_2innodb.result@stripped, 2007-11-23 02:14:17+03:00, kostja@bodhi.(none) +0 -0 mysql-test/r/sp_trans.result@stripped, 2007-11-23 02:14:15+03:00, kostja@bodhi.(none) +191 -0 Add tests for Handler_commit and Handler_prepare counts: verify that two-phase commit optimisation works. mysql-test/t/dml_2innodb.test@stripped, 2007-11-23 02:14:17+03:00, kostja@bodhi.(none) +5 -0 New BitKeeper file ``mysql-test/t/dml_2innodb.test'' mysql-test/t/dml_2innodb.test@stripped, 2007-11-23 02:14:17+03:00, kostja@bodhi.(none) +0 -0 mysql-test/t/sp_trans.test@stripped, 2007-11-23 02:14:15+03:00, kostja@bodhi.(none) +124 -0 Update results. sql/handler.cc@stripped, 2007-11-23 02:14:16+03:00, kostja@bodhi.(none) +141 -59 Implement optimisation of read-only transactions. If a transaction consists only of DML statements that do not change data, we do not perform a two-phase commit for it (run one phase commit only). sql/handler.h@stripped, 2007-11-23 02:14:16+03:00, kostja@bodhi.(none) +65 -6 Implement optimisation of read-only transactions. If a transaction consists only of DML statements that do not change data, we do not perform a two-phase commit for it (run one phase commit only). sql/rpl_injector.cc@stripped, 2007-11-23 02:14:16+03:00, kostja@bodhi.(none) +1 -0 Always commit statement transaction before committing the global one. sql/set_var.cc@stripped, 2007-11-23 02:14:16+03:00, kostja@bodhi.(none) +2 -2 Use end_cactive_trans consistently. Only update server status if operation succeeded. sql/sql_acl.cc@stripped, 2007-11-23 02:14:16+03:00, kostja@bodhi.(none) +1 -1 Fix an unrelated information_schema crash (already fixed in the main tree) to make dml.inc test pass. sql/sql_base.cc@stripped, 2007-11-23 02:14:16+03:00, kostja@bodhi.(none) +13 -21 Commit transaction at the end of the statement. Always. sql/sql_class.cc@stripped, 2007-11-23 02:14:16+03:00, kostja@bodhi.(none) +1 -1 Fix select_dumpvar::send_data to properly return operation status. A test case from dml.inc would lead to an assertion failure in the diagnostics area (double assignment). Not test otherwise by the test suite. sql/sql_class.h@stripped, 2007-11-23 02:14:16+03:00, kostja@bodhi.(none) +41 -3 Implement a new layout of storage engine transaction info in which it is easy to access all members related to the handlerton only based on ht->slot. sql/sql_cursor.cc@stripped, 2007-11-23 02:14:16+03:00, kostja@bodhi.(none) +3 -2 Update to the new layout of thd->transaction. sql/sql_delete.cc@stripped, 2007-11-23 02:14:16+03:00, kostja@bodhi.(none) +3 -21 Remove wrong and now redundant calls to ha_autocommit_or_rollback. The transaction is committed in one place, at the end of the statement. Remove calls to mysql_unlock_tables, since some engines count locks and commit statement transaction in unlock_tables(), which essentially equates mysql_unlock_tables to ha_autocommit_or_rollback. Previously it was necessary to unlock tables soon because we wanted to avoid sending of 'ok' packet to the client under locked tables. This is no longer necessary, since OK packet is also sent from one place at the end of transaction. sql/sql_insert.cc@stripped, 2007-11-23 02:14:16+03:00, kostja@bodhi.(none) +12 -31 Remove wrong and now redundant calls to ha_autocommit_or_rollback. The transaction is committed in one place, at the end of the statement. Remove calls to mysql_unlock_tables, since some engines count locks and commit statement transaction in unlock_tables(), which essentially equates mysql_unlock_tables to ha_autocommit_or_rollback. Previously it was necessary to unlock tables soon because we wanted to avoid sending of 'ok' packet to the client under locked tables. This is no longer necessary, since OK packet is also sent from one place at the end of transaction. sql/sql_load.cc@stripped, 2007-11-23 02:14:16+03:00, kostja@bodhi.(none) +0 -7 Remove wrong and now redundant calls to ha_autocommit_or_rollback. The transaction is committed in one place, at the end of the statement. Remove calls to mysql_unlock_tables, since some engines count locks and commit statement transaction in unlock_tables(), which essentially equates mysql_unlock_tables to ha_autocommit_or_rollback. Previously it was necessary to unlock tables soon because we wanted to avoid sending of 'ok' packet to the client under locked tables. This is no longer necessary, since OK packet is also sent from one place at the end of transaction. sql/sql_parse.cc@stripped, 2007-11-23 02:14:16+03:00, kostja@bodhi.(none) +42 -27 Implement optimisation of read-only transactions: bypass 2-phase commit for them. Always commit statement transaction before commiting the global one. Fix an unrelated crash in check_table_access, when called from information_schema (already fixed in the main tree). sql/sql_partition.cc@stripped, 2007-11-23 02:14:17+03:00, kostja@bodhi.(none) +2 -2 Always commit statement transaction before committing the global one. sql/sql_table.cc@stripped, 2007-11-23 02:14:17+03:00, kostja@bodhi.(none) +7 -7 Use ha_autocommit_or_rollback and end_active_trans everywhere. sql/sql_update.cc@stripped, 2007-11-23 02:14:17+03:00, kostja@bodhi.(none) +5 -34 Remove wrong and now redundant calls to ha_autocommit_or_rollback. The transaction is committed in one place, at the end of the statement. Remove calls to mysql_unlock_tables, since some engines count locks and commit statement transaction in unlock_tables(), which essentially equates mysql_unlock_tables to ha_autocommit_or_rollback. Previously it was necessary to unlock tables soon because we wanted to avoid sending of 'ok' packet to the client under locked tables. This is no longer necessary, since OK packet is also sent from one place at the end of transaction. diff -Nrup a/mysql-test/r/dml_2innodb.result b/mysql-test/r/dml_2innodb.result --- /dev/null Wed Dec 31 16:00:00 196900 +++ b/mysql-test/r/dml_2innodb.result 2007-11-23 02:14:17 +03:00 @@ -0,0 +1,336 @@ +set sql_mode=no_engine_substitution; +set storage_engine = InnoDB; +set autocommit=1; +drop table if exists t1; +drop table if exists t2; +drop table if exists t3; +drop function if exists f2; +drop procedure if exists bug12713_call; +drop procedure if exists bug12713_dump_spvars; +create table t1 (a int); +create table t2 (a int unique); +create table t3 (a int); +set sql_mode=default; +insert into t1 (a) values (1), (2); +insert into t3 (a) values (1), (2); +create function f2(x int) returns int +begin +insert into t2 (a) values (x); +insert into t2 (a) values (x); +return x; +end// +set autocommit=0; +flush status; +insert into t2 (a) values (1001); +insert into t1 (a) values (f2(1)); +ERROR 23000: Duplicate entry '1' for key 'a' +select * from t2; +a +1001 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1002); +insert into t3 (a) select f2(2) from t1; +ERROR 23000: Duplicate entry '2' for key 'a' +select * from t2; +a +1002 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1003); +update t1 set a= a + f2(3); +ERROR 23000: Duplicate entry '3' for key 'a' +select * from t2; +a +1003 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1004); +update t1, t3 set t1.a = 0, t3.a = 0 where (f2(4) = 4) and (t1.a = t3.a); +ERROR 23000: Duplicate entry '4' for key 'a' +select * from t2; +a +1004 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1005); +delete from t1 where (a = f2(5)); +ERROR 23000: Duplicate entry '5' for key 'a' +select * from t2; +a +1005 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1006); +delete from t1, t3 using t1, t3 where (f2(6) = 6) ; +ERROR 23000: Duplicate entry '6' for key 'a' +select * from t2; +a +1006 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1007); +replace t1 values (f2(7)); +ERROR 23000: Duplicate entry '7' for key 'a' +select * from t2; +a +1007 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1008); +replace into t3 (a) select f2(8) from t1; +ERROR 23000: Duplicate entry '8' for key 'a' +select * from t2; +a +1008 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1009); +select f2(9) from t1 ; +ERROR 23000: Duplicate entry '9' for key 'a' +select * from t2; +a +1009 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1010); +show databases where (f2(10) = 10); +ERROR 23000: Duplicate entry '10' for key 'a' +select * from t2; +a +1010 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1011); +show tables where (f2(11) = 11); +ERROR 23000: Duplicate entry '11' for key 'a' +select * from t2; +a +1011 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1012); +show triggers where (f2(12) = 12); +ERROR 23000: Duplicate entry '12' for key 'a' +select * from t2; +a +1012 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1013); +show table status where (f2(13) = 13); +ERROR 23000: Duplicate entry '13' for key 'a' +select * from t2; +a +1013 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1014); +show open tables where (f2(14) = 14); +ERROR 23000: Duplicate entry '14' for key 'a' +select * from t2; +a +1014 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1015); +show columns in mysql.proc where (f2(15) = 15); +ERROR 23000: Duplicate entry '15' for key 'a' +select * from t2; +a +1015 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1016); +show status where (f2(16) = 16); +ERROR 23000: Duplicate entry '16' for key 'a' +select * from t2; +a +1016 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1017); +show variables where (f2(17) = 17); +ERROR 23000: Duplicate entry '17' for key 'a' +select * from t2; +a +1017 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1018); +show charset where (f2(18) = 18); +ERROR 23000: Duplicate entry '18' for key 'a' +select * from t2; +a +1018 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1019); +show collation where (f2(19) = 19); +ERROR 23000: Duplicate entry '19' for key 'a' +select * from t2; +a +1019 +rollback; +select * from t2; +a +commit; +# We need at least one procedure to make sure the WHERE clause is +# evaluated +create procedure dummy() begin end; +insert into t2 (a) values (1020); +show procedure status where (f2(20) = 20); +ERROR 23000: Duplicate entry '20' for key 'a' +select * from t2; +a +1020 +rollback; +select * from t2; +a +commit; +drop procedure dummy; +insert into t2 (a) values (1021); +show function status where (f2(21) = 21); +ERROR 23000: Duplicate entry '21' for key 'a' +select * from t2; +a +1021 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1022); +prepare stmt from "insert into t1 (a) values (f2(22))"; +execute stmt; +ERROR 23000: Duplicate entry '22' for key 'a' +select * from t2; +a +1022 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1023); +do (f2(23)); +Warnings: +Error 1062 Duplicate entry '23' for key 'a' +select * from t2; +a +23 +1023 +rollback; +select * from t2; +a +commit; +create procedure bug12713_call () +begin +insert into t2 (a) values (24); +insert into t2 (a) values (24); +end// +insert into t2 (a) values (1024); +call bug12713_call(); +ERROR 23000: Duplicate entry '24' for key 'a' +select * from t2; +a +24 +1024 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1025); +select f2(25) into outfile "../tmp/dml.out" from t1; +ERROR 23000: Duplicate entry '25' for key 'a' +select * from t2; +a +1025 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1026); +load data infile "../std_data_ln/words.dat" into table t1 (a) set a:=f2(26); +ERROR 23000: Duplicate entry '26' for key 'a' +select * from t2; +a +1026 +rollback; +select * from t2; +a +commit; +insert into t2 (a) values (1027); +select f2(27) into @foo; +ERROR 23000: Duplicate entry '27' for key 'a' +select * from t2; +a +1027 +rollback; +select * from t2; +a +commit; +create procedure bug12713_dump_spvars () +begin +declare foo int; +declare continue handler for sqlexception +begin +select "Exception trapped"; +end; +select f2(28) into foo; +select * from t2; +end// +insert into t2 (a) values (1028); +call bug12713_dump_spvars (); +Exception trapped +Exception trapped +a +1028 +rollback; +select * from t2; +a +commit; +show status like 'Handler_prepare'; +Variable_name Value +Handler_prepare 0 +set autocommit=default; +drop table t1; +drop table t2; +drop table t3; +drop function f2; +drop procedure bug12713_call; +drop procedure bug12713_dump_spvars; diff -Nrup a/mysql-test/r/sp_trans.result b/mysql-test/r/sp_trans.result --- a/mysql-test/r/sp_trans.result 2007-06-26 15:15:02 +04:00 +++ b/mysql-test/r/sp_trans.result 2007-11-23 02:14:15 +03:00 @@ -556,3 +556,194 @@ f1 bug13575(f1) 3 ccc drop function bug13575| drop table t3| +# +# Bug#12713 Error in a stored function called from a SELECT doesn't +# cause ROLLBACK of statem +# +# Verify that two-phase commit is not issued for read-only +# transactions. +# +# Verify that two-phase commit is issued for read-write transactions, +# even if the change is done inside a stored function called from +# SELECT or SHOW statement. +# +set autocommit=0| +drop table if exists t1| +drop table if exists t2| +drop procedure if exists p_verify_status_increment| +create table t1 (a int unique) engine=innodb| +create table t2 (a int unique) engine=myisam| +# +# An auxiliary procedure to track Handler_prepare and Handler_commit +# statistics. +# +create procedure +p_verify_status_increment(commit_inc int, prepare_inc int) +begin +declare old_commit_count int default @commit_count; +declare old_prepare_count int default @prepare_count; +if @commit_count is null then +set @commit_count= 0; +end if; +if @prepare_count is null then +set @prepare_count= 0; +end if; +select variable_value +from information_schema.session_status +where variable_name='Handler_commit' + into @commit_count; +select variable_value +from information_schema.session_status +where variable_name='Handler_prepare' + into @prepare_count; +select ''; -- empty line +if old_commit_count + commit_inc <> @commit_count then +select concat("Expected commit increment: ", commit_inc, +" actual: ", @commit_count - old_commit_count) +as 'ERROR'; +elseif old_prepare_count + prepare_inc <> @prepare_count then +select concat("Expected prepare increment: ", prepare_inc, +" actual: ", @prepare_count - old_prepare_count) +as 'ERROR'; +else +select concat("Commit increment: ", commit_inc, +"; Prepare increment: ", prepare_inc) +as 'SUCCESS'; +end if; +select ''; -- empty line +end| +# Reset Handler_commit and Handler_prepare counters +flush status| +# +# 1. Read-only statement: SELECT +# +select * from t1| +a +call p_verify_status_increment(1, 0)| + + +SUCCESS +Commit increment: 1; Prepare increment: 0 + + +commit| +call p_verify_status_increment(1, 0)| + + +SUCCESS +Commit increment: 1; Prepare increment: 0 + + +# 2. Read-write statement: INSERT, insert 1 row. +# +insert into t1 (a) values (1)| +call p_verify_status_increment(2, 1)| + + +SUCCESS +Commit increment: 2; Prepare increment: 1 + + +commit| +call p_verify_status_increment(2, 1)| + + +SUCCESS +Commit increment: 2; Prepare increment: 1 + + +# 3. Read-write statement: UPDATE, update 1 row. +# +update t1 set a=2| +call p_verify_status_increment(2, 1)| + + +SUCCESS +Commit increment: 2; Prepare increment: 1 + + +commit| +call p_verify_status_increment(2, 1)| + + +SUCCESS +Commit increment: 2; Prepare increment: 1 + + +# 4. Read-write statement: UPDATE, update 0 rows. +# +update t1 set a=1| +call p_verify_status_increment(2, 1)| + + +SUCCESS +Commit increment: 2; Prepare increment: 1 + + +commit| +call p_verify_status_increment(2, 1)| + + +SUCCESS +Commit increment: 2; Prepare increment: 1 + + +# 5. Read-write statement: UPDATE, update 0 rows. +# +update t1 set a=3 where a=2| +call p_verify_status_increment(2, 0)| + + +SUCCESS +Commit increment: 2; Prepare increment: 0 + + +commit| +call p_verify_status_increment(2, 0)| + + +SUCCESS +Commit increment: 2; Prepare increment: 0 + + +# 6. Read-write statement: DELETE, delete 0 rows. +# +delete from t1 where a=2| +call p_verify_status_increment(2, 0)| + + +SUCCESS +Commit increment: 2; Prepare increment: 0 + + +commit| +call p_verify_status_increment(2, 1)| + + +ERROR +Expected prepare increment: 1 actual: 0 + + +# 7. Read-write statement: DELETE, delete 1 rows. +# +delete from t1 where a=1| +call p_verify_status_increment(2, 1)| + + +SUCCESS +Commit increment: 2; Prepare increment: 1 + + +commit| +call p_verify_status_increment(2, 1)| + + +SUCCESS +Commit increment: 2; Prepare increment: 1 + + +# +# Cleanup +# +drop table t1| +drop procedure p_verify_status_increment| diff -Nrup a/mysql-test/t/dml_2innodb.test b/mysql-test/t/dml_2innodb.test --- /dev/null Wed Dec 31 16:00:00 196900 +++ b/mysql-test/t/dml_2innodb.test 2007-11-23 02:14:17 +03:00 @@ -0,0 +1,5 @@ +-- source include/have_innodb.inc + +let $engine_type = InnoDB; + +-- source include/dml.inc diff -Nrup a/mysql-test/t/sp_trans.test b/mysql-test/t/sp_trans.test --- a/mysql-test/t/sp_trans.test 2007-06-15 20:35:09 +04:00 +++ b/mysql-test/t/sp_trans.test 2007-11-23 02:14:15 +03:00 @@ -592,6 +592,130 @@ select distinct f1, bug13575(f1) from t3 drop function bug13575| drop table t3| +--echo # +--echo # Bug#12713 Error in a stored function called from a SELECT doesn't +--echo # cause ROLLBACK of statem +--echo # +--echo # Verify that two-phase commit is not issued for read-only +--echo # transactions. +--echo # +--echo # Verify that two-phase commit is issued for read-write transactions, +--echo # even if the change is done inside a stored function called from +--echo # SELECT or SHOW statement. +--echo # +set autocommit=0| +--disable_warnings +drop table if exists t1| +drop table if exists t2| +drop procedure if exists p_verify_status_increment| +--enable_warnings + +create table t1 (a int unique) engine=innodb| +create table t2 (a int unique) engine=myisam| +--echo # +--echo # An auxiliary procedure to track Handler_prepare and Handler_commit +--echo # statistics. +--echo # +create procedure +p_verify_status_increment(commit_inc int, prepare_inc int) +begin + declare old_commit_count int default @commit_count; + declare old_prepare_count int default @prepare_count; + + if @commit_count is null then + set @commit_count= 0; + end if; + if @prepare_count is null then + set @prepare_count= 0; + end if; + + select variable_value + from information_schema.session_status + where variable_name='Handler_commit' + into @commit_count; + + select variable_value + from information_schema.session_status + where variable_name='Handler_prepare' + into @prepare_count; + + select ''; -- empty line + if old_commit_count + commit_inc <> @commit_count then + select concat("Expected commit increment: ", commit_inc, + " actual: ", @commit_count - old_commit_count) + as 'ERROR'; + elseif old_prepare_count + prepare_inc <> @prepare_count then + select concat("Expected prepare increment: ", prepare_inc, + " actual: ", @prepare_count - old_prepare_count) + as 'ERROR'; + else + select concat("Commit increment: ", commit_inc, + "; Prepare increment: ", prepare_inc) + as 'SUCCESS'; + end if; + select ''; -- empty line +end| +--echo # Reset Handler_commit and Handler_prepare counters +flush status| +--echo # +--echo # 1. Read-only statement: SELECT +--echo # +select * from t1| +call p_verify_status_increment(1, 0)| +commit| +call p_verify_status_increment(1, 0)| + +--echo # 2. Read-write statement: INSERT, insert 1 row. +--echo # +insert into t1 (a) values (1)| +call p_verify_status_increment(2, 1)| +commit| +call p_verify_status_increment(2, 1)| + +--echo # 3. Read-write statement: UPDATE, update 1 row. +--echo # +update t1 set a=2| +call p_verify_status_increment(2, 1)| +commit| +call p_verify_status_increment(2, 1)| + +--echo # 4. Read-write statement: UPDATE, update 0 rows. +--echo # +update t1 set a=1| +call p_verify_status_increment(2, 1)| +commit| +call p_verify_status_increment(2, 1)| + +--echo # 5. Read-write statement: UPDATE, update 0 rows. +--echo # +update t1 set a=3 where a=2| +# XXX: why? +call p_verify_status_increment(2, 0)| +commit| +call p_verify_status_increment(2, 0)| + +--echo # 6. Read-write statement: DELETE, delete 0 rows. +--echo # +delete from t1 where a=2| +# XXX: why? +call p_verify_status_increment(2, 0)| +commit| +call p_verify_status_increment(2, 1)| + +--echo # 7. Read-write statement: DELETE, delete 1 rows. +--echo # +delete from t1 where a=1| +# XXX: why? +call p_verify_status_increment(2, 1)| +commit| +call p_verify_status_increment(2, 1)| + +--echo # +--echo # Cleanup +--echo # +drop table t1| +drop procedure p_verify_status_increment| + # # BUG#NNNN: New bug synopsis diff -Nrup a/sql/handler.cc b/sql/handler.cc --- a/sql/handler.cc 2007-10-30 22:35:11 +03:00 +++ b/sql/handler.cc 2007-11-23 02:14:16 +03:00 @@ -596,7 +596,7 @@ void ha_close_connection(THD* thd) void trans_register_ha(THD *thd, bool all, handlerton *ht_arg) { THD_TRANS *trans; - handlerton **ht; + Ha_trx_info *ha_info; DBUG_ENTER("trans_register_ha"); DBUG_PRINT("enter",("%s", all ? "all" : "stmt")); @@ -608,12 +608,13 @@ void trans_register_ha(THD *thd, bool al else trans= &thd->transaction.stmt; - for (ht=trans->ht; *ht; ht++) - if (*ht == ht_arg) - DBUG_VOID_RETURN; /* already registered, return */ + ha_info= thd->ha_data[ht_arg->slot].ha_info + static_cast(all); + + if (ha_info->ht) + DBUG_VOID_RETURN; /* already registered, return */ + + ha_info->register_ha(trans, ht_arg); - trans->ht[trans->nht++]=ht_arg; - DBUG_ASSERT(*ht == ht_arg); trans->no_2pc|=(ht_arg->prepare==0); if (thd->transaction.xid_state.xid.is_null()) thd->transaction.xid_state.xid.set(thd->query_id); @@ -629,18 +630,19 @@ int ha_prepare(THD *thd) { int error=0, all=1; THD_TRANS *trans=all ? &thd->transaction.all : &thd->transaction.stmt; - handlerton **ht=trans->ht; + Ha_trx_info *ha_info= trans->ha_list; DBUG_ENTER("ha_prepare"); #ifdef USING_TRANSACTIONS - if (trans->nht) + if (ha_info) { - for (; *ht; ht++) + for (; ha_info; ha_info= ha_info->next) { int err; + handlerton *ht= ha_info->ht; status_var_increment(thd->status_var.ha_prepare_count); - if ((*ht)->prepare) + if (ht->prepare) { - if ((err= (*(*ht)->prepare)(*ht, thd, all))) + if ((err= ht->prepare(ht, thd, all))) { my_error(ER_ERROR_DURING_COMMIT, MYF(0), err); ha_rollback_trans(thd, all); @@ -652,7 +654,7 @@ int ha_prepare(THD *thd) { push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN, ER_ILLEGAL_HA, ER(ER_ILLEGAL_HA), - ha_resolve_storage_engine_name(*ht)); + ha_resolve_storage_engine_name(ht)); } } } @@ -669,12 +671,23 @@ int ha_prepare(THD *thd) int ha_commit_trans(THD *thd, bool all) { int error= 0, cookie= 0; + /* + 'all' means that this is either an explicit commit issued by + user, or an implicit commit issued by a DDL. + */ THD_TRANS *trans= all ? &thd->transaction.all : &thd->transaction.stmt; - bool is_real_trans= all || thd->transaction.all.nht == 0; - handlerton **ht= trans->ht; + bool is_real_trans= all || thd->transaction.all.ha_list == 0; + Ha_trx_info *ha_info= trans->ha_list; my_xid xid= thd->transaction.xid_state.xid.get_my_xid(); DBUG_ENTER("ha_commit_trans"); + /* + We must not commit a global transaction if statement + transaction is pending. + */ + DBUG_ASSERT(thd->transaction.stmt.ha_list == NULL || + trans == &thd->transaction.stmt); + if (thd->in_sub_stmt) { /* @@ -696,7 +709,7 @@ int ha_commit_trans(THD *thd, bool all) DBUG_RETURN(2); } #ifdef USING_TRANSACTIONS - if (trans->nht) + if (ha_info) { if (is_real_trans && wait_if_global_read_lock(thd, 0, 0)) { @@ -722,12 +735,36 @@ int ha_commit_trans(THD *thd, bool all) if (is_real_trans) /* not a statement commit */ thd->stmt_map.close_transient_cursors(); - if (!trans->no_2pc && trans->nht > 1) + if (!trans->no_2pc && ha_info->next) { - for (; *ht && !error; ht++) + for (; ha_info && !error; ha_info= ha_info->next) { int err; - if ((err= (*(*ht)->prepare)(*ht, thd, all))) + handlerton *ht= ha_info->ht; + + if (! all) + { + Ha_trx_info *ha_info_all= &thd->ha_data[ht->slot].ha_info[1]; + DBUG_ASSERT(ha_info_all != ha_info); + /* + Merge read-only/read-write information about statement + transaction to its global counterpart. + */ + ha_info_all->coalesce_trx_with(ha_info); + } + /** + Perform a two-phase commit only if: + - we're doing a DDL or + - we're issuing a (sequence of) DML that has actually updated + rows. + In other cases, like read-only SELECT statements, + we would like to save on two-phase commits and therefore + avoid a potential disk sync of the binary log entailed by it. + */ + if (ha_info->is_trx_read_only()) + continue; + + if ((err= ht->prepare(ht, thd, all))) { my_error(ER_ERROR_DURING_COMMIT, MYF(0), err); error= 1; @@ -765,24 +802,26 @@ int ha_commit_one_phase(THD *thd, bool a { int error=0; THD_TRANS *trans=all ? &thd->transaction.all : &thd->transaction.stmt; - bool is_real_trans=all || thd->transaction.all.nht == 0; - handlerton **ht=trans->ht; + bool is_real_trans=all || thd->transaction.all.ha_list == 0; + Ha_trx_info *ha_info= trans->ha_list, *ha_info_next; DBUG_ENTER("ha_commit_one_phase"); #ifdef USING_TRANSACTIONS - if (trans->nht) + if (ha_info) { - for (ht=trans->ht; *ht; ht++) + for (; ha_info; ha_info= ha_info_next) { int err; - if ((err= (*(*ht)->commit)(*ht, thd, all))) + handlerton *ht= ha_info->ht; + if ((err= ht->commit(ht, thd, all))) { my_error(ER_ERROR_DURING_COMMIT, MYF(0), err); error=1; } status_var_increment(thd->status_var.ha_commit_count); - *ht= 0; + ha_info_next= ha_info->next; + ha_info->reset(); /* keep it conveniently zero-filled */ } - trans->nht=0; + trans->ha_list= 0; trans->no_2pc=0; if (is_real_trans) thd->transaction.xid_state.xid.null(); @@ -805,8 +844,17 @@ int ha_rollback_trans(THD *thd, bool all { int error=0; THD_TRANS *trans=all ? &thd->transaction.all : &thd->transaction.stmt; - bool is_real_trans=all || thd->transaction.all.nht == 0; + Ha_trx_info *ha_info= trans->ha_list, *ha_info_next; + bool is_real_trans=all || thd->transaction.all.ha_list == 0; DBUG_ENTER("ha_rollback_trans"); + + /* + We must not commit a global transaction if statement + transaction is pending. + */ + DBUG_ASSERT(thd->transaction.stmt.ha_list == NULL || + trans == &thd->transaction.stmt); + if (thd->in_sub_stmt) { /* @@ -821,24 +869,26 @@ int ha_rollback_trans(THD *thd, bool all DBUG_RETURN(1); } #ifdef USING_TRANSACTIONS - if (trans->nht) + if (ha_info) { /* Close all cursors that can not survive ROLLBACK */ if (is_real_trans) /* not a statement commit */ thd->stmt_map.close_transient_cursors(); - for (handlerton **ht=trans->ht; *ht; ht++) + for (; ha_info; ha_info= ha_info_next) { int err; - if ((err= (*(*ht)->rollback)(*ht, thd, all))) + handlerton *ht= ha_info->ht; + if ((err= ht->rollback(ht, thd, all))) { // cannot happen my_error(ER_ERROR_DURING_ROLLBACK, MYF(0), err); error=1; } status_var_increment(thd->status_var.ha_rollback_count); - *ht= 0; + ha_info_next= ha_info->next; + ha_info->reset(); /* keep it conveniently zero-filled */ } - trans->nht=0; + trans->ha_list= 0; trans->no_2pc=0; if (is_real_trans) thd->transaction.xid_state.xid.null(); @@ -881,17 +931,19 @@ int ha_autocommit_or_rollback(THD *thd, { DBUG_ENTER("ha_autocommit_or_rollback"); #ifdef USING_TRANSACTIONS - if (thd->transaction.stmt.nht) + if (thd->transaction.stmt.ha_list) { if (!error) { if (ha_commit_stmt(thd)) error=1; } - else if (thd->transaction_rollback_request && !thd->in_sub_stmt) - (void) ha_rollback(thd); - else + else + { (void) ha_rollback_stmt(thd); + if (thd->transaction_rollback_request && !thd->in_sub_stmt) + (void) ha_rollback(thd); + } thd->variables.tx_isolation=thd->session_tx_isolation; } @@ -1236,43 +1288,49 @@ int ha_rollback_to_savepoint(THD *thd, S int error=0; THD_TRANS *trans= (thd->in_sub_stmt ? &thd->transaction.stmt : &thd->transaction.all); - handlerton **ht=trans->ht, **end_ht; + Ha_trx_info *ha_info, *ha_info_next; + DBUG_ENTER("ha_rollback_to_savepoint"); - trans->nht=sv->nht; trans->no_2pc=0; - end_ht=ht+sv->nht; /* rolling back to savepoint in all storage engines that were part of the transaction when the savepoint was set */ - for (; ht < end_ht; ht++) + for (ha_info= sv->ha_list; ha_info; ha_info= ha_info->next) { int err; - DBUG_ASSERT((*ht)->savepoint_set != 0); - if ((err= (*(*ht)->savepoint_rollback)(*ht, thd, (uchar *)(sv+1)+(*ht)->savepoint_offset))) + handlerton *ht= ha_info->ht; + DBUG_ASSERT(ht); + DBUG_ASSERT(ht->savepoint_set != 0); + if ((err= ht->savepoint_rollback(ht, thd, + (uchar *)(sv+1)+ht->savepoint_offset))) { // cannot happen my_error(ER_ERROR_DURING_ROLLBACK, MYF(0), err); error=1; } status_var_increment(thd->status_var.ha_savepoint_rollback_count); - trans->no_2pc|=(*ht)->prepare == 0; + trans->no_2pc|= ht->prepare == 0; } /* rolling back the transaction in all storage engines that were not part of the transaction when the savepoint was set */ - for (; *ht ; ht++) + for (ha_info= trans->ha_list; ha_info != sv->ha_list; + ha_info= ha_info_next) { int err; - if ((err= (*(*ht)->rollback)(*ht, thd, !thd->in_sub_stmt))) + handlerton *ht= ha_info->ht; + if ((err= ht->rollback(ht, thd, !thd->in_sub_stmt))) { // cannot happen my_error(ER_ERROR_DURING_ROLLBACK, MYF(0), err); error=1; } status_var_increment(thd->status_var.ha_rollback_count); - *ht=0; // keep it conveniently zero-filled + ha_info_next= ha_info->next; + ha_info->reset(); /* keep it conveniently zero-filled */ } + trans->ha_list= sv->ha_list; DBUG_RETURN(error); } @@ -1286,26 +1344,32 @@ int ha_savepoint(THD *thd, SAVEPOINT *sv int error=0; THD_TRANS *trans= (thd->in_sub_stmt ? &thd->transaction.stmt : &thd->transaction.all); - handlerton **ht=trans->ht; + Ha_trx_info *ha_info= trans->ha_list; DBUG_ENTER("ha_savepoint"); #ifdef USING_TRANSACTIONS - for (; *ht; ht++) + for (; ha_info; ha_info= ha_info->next) { int err; - if (! (*ht)->savepoint_set) + handlerton *ht= ha_info->ht; + DBUG_ASSERT(ht); + if (! ht->savepoint_set) { my_error(ER_CHECK_NOT_IMPLEMENTED, MYF(0), "SAVEPOINT"); error=1; break; } - if ((err= (*(*ht)->savepoint_set)(*ht, thd, (uchar *)(sv+1)+(*ht)->savepoint_offset))) + if ((err= ht->savepoint_set(ht, thd, (uchar *)(sv+1)+ht->savepoint_offset))) { // cannot happen my_error(ER_GET_ERRNO, MYF(0), err); error=1; } status_var_increment(thd->status_var.ha_savepoint_count); } - sv->nht=trans->nht; + /* + Remember the list of registered storage engines. All new + engines are prepended to the beginning of the list. + */ + sv->ha_list= trans->ha_list; #endif /* USING_TRANSACTIONS */ DBUG_RETURN(error); } @@ -1313,20 +1377,19 @@ int ha_savepoint(THD *thd, SAVEPOINT *sv int ha_release_savepoint(THD *thd, SAVEPOINT *sv) { int error=0; - THD_TRANS *trans= (thd->in_sub_stmt ? &thd->transaction.stmt : - &thd->transaction.all); - handlerton **ht=trans->ht, **end_ht; + Ha_trx_info *ha_info= sv->ha_list; DBUG_ENTER("ha_release_savepoint"); - end_ht=ht+sv->nht; - for (; ht < end_ht; ht++) + for (; ha_info; ha_info= ha_info->next) { int err; - if (!(*ht)->savepoint_release) + handlerton *ht= ha_info->ht; + /* Savepoint life time is enclosed into transaction life time. */ + DBUG_ASSERT(ht); + if (!ht->savepoint_release) continue; - if ((err= (*(*ht)->savepoint_release)(*ht, thd, - (uchar *)(sv+1)+ - (*ht)->savepoint_offset))) + if ((err= ht->savepoint_release(ht, thd, + (uchar *)(sv+1) + ht->savepoint_offset))) { // cannot happen my_error(ER_GET_ERRNO, MYF(0), err); error=1; @@ -1506,6 +1569,17 @@ int ha_delete_table(THD *thd, handlerton DBUG_RETURN(error); } + +/** Mark the transaction read-write for all registered engines. */ + +void +THD_TRANS::set_trx_read_write() +{ + Ha_trx_info *ha_info= ha_list; + for (; ha_info; ha_info= ha_info->next) + ha_info->set_trx_read_write(); +} + /**************************************************************************** ** General handler functions ****************************************************************************/ @@ -3677,6 +3751,8 @@ int handler::ha_reset() int handler::ha_write_row(uchar *buf) { int error; + if (has_transactions()) + ha_thd()->ha_data[ht->slot].ha_info[0].set_trx_read_write(); if (unlikely(error= write_row(buf))) return error; if (unlikely(error= binlog_log_row(table, 0, buf))) @@ -3695,6 +3771,8 @@ int handler::ha_update_row(const uchar * */ DBUG_ASSERT(new_data == table->record[0]); + if (has_transactions()) + ha_thd()->ha_data[ht->slot].ha_info[0].set_trx_read_write(); if (unlikely(error= update_row(old_data, new_data))) return error; if (unlikely(error= binlog_log_row(table, old_data, new_data))) @@ -3705,6 +3783,10 @@ int handler::ha_update_row(const uchar * int handler::ha_delete_row(const uchar *buf) { int error; + + if (has_transactions()) + ha_thd()->ha_data[ht->slot].ha_info[0].set_trx_read_write(); + if (unlikely(error= delete_row(buf))) return error; if (unlikely(error= binlog_log_row(table, buf, 0))) diff -Nrup a/sql/handler.h b/sql/handler.h --- a/sql/handler.h 2007-09-07 17:41:46 +04:00 +++ b/sql/handler.h 2007-11-23 02:14:16 +03:00 @@ -721,14 +721,14 @@ struct handlerton #define HTON_SUPPORT_LOG_TABLES (1 << 7) //Engine supports log tables #define HTON_NO_PARTITION (1 << 8) //You can not partition these tables -typedef struct st_thd_trans +class Ha_trx_info; + +struct THD_TRANS { - /* number of entries in the ht[] */ - uint nht; /* true is not all entries in the ht[] support 2pc */ bool no_2pc; - /* storage engines that registered themselves for this transaction */ - handlerton *ht[MAX_HA]; + /* storage engines that registered in this transaction */ + Ha_trx_info *ha_list; /* The purpose of this flag is to keep track of non-transactional tables that were modified in scope of: @@ -758,7 +758,66 @@ typedef struct st_thd_trans saved value. */ bool modified_non_trans_table; -} THD_TRANS; + + void reset() { no_2pc= FALSE; modified_non_trans_table= FALSE; } + /** Mark all registered storage engine transactions read-write. */ + void set_trx_read_write(); +}; + + +/** + Either statement- or transaction- local thread-specific + storage engine data. + + If a storage engine participates in a statement/transaction, + an instance of this class is present in + thd->transaction.{stmt|all}.ha_list. The addition to ha_list + is done by trans_register_ha. + + When it's time to commit or rollback, each element of ha_list + is used to access storage engine prepare/commit/rollback + methods, and also to evaluate whether a full two phase commit + is necessary. +*/ + +struct Ha_trx_info +{ + /** + Register this storage engine in the given transaction context. + */ + void register_ha(THD_TRANS *trans, handlerton *ht_arg) + { next= trans->ha_list; trans->ha_list= this; ht= ht_arg; flags= 0; } + + /** Clear, prepare for reuse. */ + void reset() { next= NULL; ht= NULL; flags= 0; } + + Ha_trx_info() { reset(); } + + void set_trx_read_write() { flags|= (int) TRX_READ_WRITE; } + bool is_trx_read_only() const { return !(flags & (int) TRX_READ_WRITE); } + /** Mark the argument read-write if this transaction is read-write. */ + void coalesce_trx_with(const Ha_trx_info *stmt) + { + if (! stmt->is_trx_read_only()) + set_trx_read_write(); + } + +public: + /** public only to simplify access. Please use methods to assign. */ + /** Auxiliary, used for ha_list management */ + Ha_trx_info *next; + /** + Although a given Ha_trx_info instance is currently always used + for the same storage engine, 'ht' is not-NULL only when the + corresponding storage is a part of a transaction. + */ + handlerton *ht; +private: + enum { TRX_READ_ONLY= 0, TRX_READ_WRITE= 1 }; + uchar flags; +}; + + enum enum_tx_isolation { ISO_READ_UNCOMMITTED, ISO_READ_COMMITTED, ISO_REPEATABLE_READ, ISO_SERIALIZABLE}; diff -Nrup a/sql/rpl_injector.cc b/sql/rpl_injector.cc --- a/sql/rpl_injector.cc 2007-04-12 18:13:46 +04:00 +++ b/sql/rpl_injector.cc 2007-11-23 02:14:16 +03:00 @@ -62,6 +62,7 @@ int injector::transaction::commit() { DBUG_ENTER("injector::transaction::commit()"); m_thd->binlog_flush_pending_rows_event(true); + ha_autocommit_or_rollback(m_thd, 0); end_trans(m_thd, COMMIT); DBUG_RETURN(0); } diff -Nrup a/sql/set_var.cc b/sql/set_var.cc --- a/sql/set_var.cc 2007-10-30 20:08:11 +03:00 +++ b/sql/set_var.cc 2007-11-23 02:14:16 +03:00 @@ -2636,12 +2636,12 @@ static bool set_option_autocommit(THD *t { if ((org_options & OPTION_NOT_AUTOCOMMIT)) { + if (end_active_trans(thd)) + return 1; /* We changed to auto_commit mode */ thd->options&= ~(ulonglong) (OPTION_BEGIN | OPTION_KEEP_LOG); thd->transaction.all.modified_non_trans_table= FALSE; thd->server_status|= SERVER_STATUS_AUTOCOMMIT; - if (ha_commit(thd)) - return 1; } else { diff -Nrup a/sql/sql_acl.cc b/sql/sql_acl.cc --- a/sql/sql_acl.cc 2007-11-04 13:35:17 +03:00 +++ b/sql/sql_acl.cc 2007-11-23 02:14:16 +03:00 @@ -3770,7 +3770,7 @@ bool check_grant(THD *thd, ulong want_ac of other queries). For simple queries first_not_own_table is 0. */ for (i= 0, table= tables; - table != first_not_own_table && i < number; + table && table != first_not_own_table && i < number; table= table->next_global, i++) { /* Remove SHOW_VIEW_ACL, because it will be checked during making view */ diff -Nrup a/sql/sql_base.cc b/sql/sql_base.cc --- a/sql/sql_base.cc 2007-11-04 13:35:17 +03:00 +++ b/sql/sql_base.cc 2007-11-23 02:14:16 +03:00 @@ -1220,18 +1220,20 @@ void close_thread_tables(THD *thd) Mark all temporary tables used by this statement as free for reuse. */ mark_temp_tables_as_free_for_reuse(thd); + /* + Let us commit transaction for statement. Since in 5.0 we only have + one statement transaction and don't allow several nested statement + transactions this call will do nothing if we are inside of stored + function or trigger (i.e. statement transaction is already active and + does not belong to statement for which we do close_thread_tables()). + TODO: This should be fixed in later releases. + */ + thd->main_da.can_overwrite_status= TRUE; + ha_autocommit_or_rollback(thd, thd->is_error()); + thd->main_da.can_overwrite_status= FALSE; if (thd->locked_tables || prelocked_mode) { - /* - Let us commit transaction for statement. Since in 5.0 we only have - one statement transaction and don't allow several nested statement - transactions this call will do nothing if we are inside of stored - function or trigger (i.e. statement transaction is already active and - does not belong to statement for which we do close_thread_tables()). - TODO: This should be fixed in later releases. - */ - ha_commit_stmt(thd); /* Ensure we are calling ha_reset() for all used tables */ mark_used_tables_as_free_for_reuse(thd, thd->open_tables); @@ -1255,6 +1257,8 @@ void close_thread_tables(THD *thd) /* Fallthrough */ } + thd->transaction.stmt.reset(); + if (thd->lock) { /* @@ -1270,18 +1274,6 @@ void close_thread_tables(THD *thd) mysql_unlock_tables(thd, thd->lock); thd->lock=0; } - /* - assume handlers auto-commit (if some doesn't - transaction handling - in MySQL should be redesigned to support it; it's a big change, - and it's not worth it - better to commit explicitly only writing - transactions, read-only ones should better take care of themselves. - saves some work in 2pc too) - see also sql_parse.cc - dispatch_command() - */ - if (!(thd->state_flags & Open_tables_state::BACKUPS_AVAIL)) - bzero(&thd->transaction.stmt, sizeof(thd->transaction.stmt)); - if (!thd->active_transaction()) - thd->transaction.xid_state.xid.null(); if (thd->open_tables) close_open_tables(thd); diff -Nrup a/sql/sql_class.cc b/sql/sql_class.cc --- a/sql/sql_class.cc 2007-11-04 13:35:17 +03:00 +++ b/sql/sql_class.cc 2007-11-23 02:14:16 +03:00 @@ -2449,7 +2449,7 @@ bool select_dumpvar::send_data(Listupdate(); } } - DBUG_RETURN(0); + DBUG_RETURN(thd->is_error()); } bool select_dumpvar::send_eof() diff -Nrup a/sql/sql_class.h b/sql/sql_class.h --- a/sql/sql_class.h 2007-11-04 13:35:17 +03:00 +++ b/sql/sql_class.h 2007-11-23 02:14:16 +03:00 @@ -685,7 +685,8 @@ private: struct st_savepoint { struct st_savepoint *prev; char *name; - uint length, nht; + uint length; + Ha_trx_info *ha_list; }; enum xa_states {XA_NOTR=0, XA_ACTIVE, XA_IDLE, XA_PREPARED}; @@ -1090,6 +1091,32 @@ private: }; +/* + Storage engine specific thread local data. +*/ + +struct Ha_data +{ + /** + Storage engine specific thread local data. + Lifetime: one user connection. + */ + void *ha_ptr; + /** + 0: Life time: one statement within a transaction. If @@autocommit is + on, also represents the entire transaction. + @sa trans_register_ha() + + 1: Life time: one transaction within a connection. + If the storage engine does not participate in a transaction, + this should not be used. + @sa trans_register_ha() + */ + Ha_trx_info ha_info[2]; + + Ha_data() :ha_ptr(NULL) {} +}; + /** @class THD For each client connection we create a separate thread with THD serving as @@ -1223,7 +1250,7 @@ public: uint in_sub_stmt; /* container for handler's private per-connection data */ - void *ha_data[MAX_HA]; + Ha_data ha_data[MAX_HA]; #ifndef MYSQL_CLIENT int binlog_setup_trx_data(); @@ -2658,7 +2685,7 @@ public: bool send_data(List &items); bool initialize_tables (JOIN *join); void send_error(uint errcode,const char *err); - int do_updates (bool from_send_error); + int do_updates(); bool send_eof(); }; @@ -2701,6 +2728,17 @@ public: #define CF_STATUS_COMMAND 4 #define CF_SHOW_TABLE_COMMAND 8 #define CF_WRITE_LOGS_COMMAND 16 +/** + Should be set if this is a DML statement. Is used to optimize + read-only transactions: if a transaction consists only of CF_DML + statements and does not change any engine data, two-phase commit + is optimised to skip the first phase. We try to determine + that the transaction modifies by tracking whether handler::write_row/ + handler::update_row/handler::delete_row calls, so if your statement + may change data by means of some other method, you should perhaps + not set this flag. +*/ +#define CF_DML 32 /* Functions in sql_class.cc */ diff -Nrup a/sql/sql_cursor.cc b/sql/sql_cursor.cc --- a/sql/sql_cursor.cc 2006-12-31 03:06:36 +03:00 +++ b/sql/sql_cursor.cc 2007-11-23 02:14:16 +03:00 @@ -315,9 +315,10 @@ Sensitive_cursor::post_open(THD *thd) close_at_commit= FALSE; /* reset in case we're reusing the cursor */ info= &ht_info[0]; - for (handlerton **pht= thd->transaction.stmt.ht; *pht; pht++) + for (Ha_trx_info *ha_trx_info= thd->transaction.stmt.ha_list; + ha_trx_info; ha_trx_info= ha_trx_info->next) { - handlerton *ht= *pht; + handlerton *ht= ha_trx_info->ht; close_at_commit|= test(ht->flags & HTON_CLOSE_CURSORS_AT_COMMIT); if (ht->create_cursor_read_view) { diff -Nrup a/sql/sql_delete.cc b/sql/sql_delete.cc --- a/sql/sql_delete.cc 2007-11-02 02:36:10 +03:00 +++ b/sql/sql_delete.cc 2007-11-23 02:14:16 +03:00 @@ -379,17 +379,6 @@ cleanup: } DBUG_ASSERT(transactional_table || !deleted || thd->transaction.stmt.modified_non_trans_table); free_underlaid_joins(thd, select_lex); - if (transactional_table) - { - if (ha_autocommit_or_rollback(thd,error >= 0)) - error=1; - } - - if (thd->lock) - { - mysql_unlock_tables(thd, thd->lock); - thd->lock=0; - } if (error < 0 || (thd->lex->ignore && !thd->is_fatal_error)) { thd->row_count_func= deleted; @@ -740,11 +729,9 @@ void multi_delete::send_error(uint errco The same if all tables are transactional, regardless of where we are. In all other cases do attempt deletes ... */ - if ((table_being_deleted == delete_tables && - table_being_deleted->table->file->has_transactions()) || - !normal_tables) - ha_rollback_stmt(thd); - else if (do_delete) + if (do_delete && normal_tables && + (table_being_deleted != delete_tables || + !table_being_deleted->table->file->has_transactions())) { /* We have to execute the recorded do_deletes() and write info into the @@ -887,11 +874,6 @@ bool multi_delete::send_eof() thd->transaction.all.modified_non_trans_table= TRUE; } DBUG_ASSERT(!normal_tables || !deleted || thd->transaction.stmt.modified_non_trans_table); - - /* Commit or rollback the current SQL statement */ - if (transactional_tables) - if (ha_autocommit_or_rollback(thd,local_error > 0)) - local_error=1; if (!local_error) { diff -Nrup a/sql/sql_insert.cc b/sql/sql_insert.cc --- a/sql/sql_insert.cc 2007-11-04 13:35:18 +03:00 +++ b/sql/sql_insert.cc 2007-11-23 02:14:16 +03:00 @@ -893,12 +893,9 @@ bool mysql_insert(THD *thd,TABLE_LIST *t } DBUG_ASSERT(transactional_table || !changed || thd->transaction.stmt.modified_non_trans_table); - if (transactional_table) - error=ha_autocommit_or_rollback(thd,error); - + if (thd->lock) { - mysql_unlock_tables(thd, thd->lock); /* Invalidate the table in the query cache if something changed after unlocking when changes become fisible. @@ -909,7 +906,6 @@ bool mysql_insert(THD *thd,TABLE_LIST *t { query_cache_invalidate3(thd, table_list, 1); } - thd->lock=0; } } thd->proc_info="end"; @@ -3104,10 +3100,7 @@ bool select_insert::send_eof() if (changed= (info.copied || info.deleted || info.updated)) { - /* - We must invalidate the table in the query cache before binlog writing - and ha_autocommit_or_rollback. - */ + /* We must invalidate the table in the query cache before binlog writing. */ query_cache_invalidate3(thd, table, 1); if (thd->transaction.stmt.modified_non_trans_table) thd->transaction.all.modified_non_trans_table= TRUE; @@ -3119,7 +3112,7 @@ bool select_insert::send_eof() Write to binlog before commiting transaction. No statement will be written by the binlog_query() below in RBR mode. All the events are in the transaction cache and will be written when - ha_autocommit_or_rollback() is issued below. + ha_autocommit_or_rollback() is issued. */ if (mysql_bin_log.is_open()) { @@ -3129,18 +3122,6 @@ bool select_insert::send_eof() thd->query, thd->query_length, trans_table, FALSE); } - /* - We will call ha_autocommit_or_rollback() also for - non-transactional tables under row-based replication: there might - be events in the binary logs transaction, and we need to write - them to the binary log. - */ - if (trans_table || thd->current_stmt_binlog_row_based) - { - int error2= ha_autocommit_or_rollback(thd, error); - if (error2 && !error) - error= error2; - } table->file->ha_release_auto_increment(); if (error) @@ -3217,7 +3198,6 @@ void select_insert::abort() { table->file->ha_release_auto_increment(); } - ha_rollback_stmt(thd); DBUG_VOID_RETURN; } @@ -3659,7 +3639,10 @@ bool select_create::send_eof() nevertheless. */ if (!table->s->tmp_table) - ha_commit(thd); // Can fail, but we proceed anyway + { + ha_autocommit_or_rollback(thd, 0); + end_active_trans(thd); // Can fail, but we proceed anyway + } table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY); table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE); @@ -3683,12 +3666,9 @@ void select_create::abort() by removing the table, even for non-transactional tables. */ tmp_disable_binlog(thd); - select_insert::abort(); - reenable_binlog(thd); - /* - We roll back the statement, including truncating the transaction - cache of the binary log, if the statement failed. + In select_insert::abort() we roll back the statement, including + truncating the transaction cache of the binary log. We roll back the statement prior to deleting the table and prior to releasing the lock on the table, since there might be potential @@ -3699,8 +3679,9 @@ void select_create::abort() of the table succeeded or not, since we need to reset the binary log state. */ - if (thd->current_stmt_binlog_row_based) - ha_rollback_stmt(thd); + select_insert::abort(); + reenable_binlog(thd); + if (m_plock) { diff -Nrup a/sql/sql_load.cc b/sql/sql_load.cc --- a/sql/sql_load.cc 2007-07-30 20:02:19 +04:00 +++ b/sql/sql_load.cc 2007-11-23 02:14:16 +03:00 @@ -498,8 +498,6 @@ bool mysql_load(THD *thd,sql_exchange *e } } #endif /*!EMBEDDED_LIBRARY*/ - if (transactional_table) - error=ha_autocommit_or_rollback(thd,error); /* ok to client sent only after binlog write and engine commit */ send_ok(thd, info.copied + info.deleted, 0L, name); @@ -507,11 +505,6 @@ err: DBUG_ASSERT(transactional_table || !(info.copied || info.deleted) || thd->transaction.stmt.modified_non_trans_table); table->file->ha_release_auto_increment(); - if (thd->lock) - { - mysql_unlock_tables(thd, thd->lock); - thd->lock=0; - } table->auto_increment_field_not_null= FALSE; thd->abort_on_warning= 0; DBUG_RETURN(error); diff -Nrup a/sql/sql_parse.cc b/sql/sql_parse.cc --- a/sql/sql_parse.cc 2007-11-04 13:35:18 +03:00 +++ b/sql/sql_parse.cc 2007-11-23 02:14:16 +03:00 @@ -204,7 +204,7 @@ void init_update_queries(void) sql_command_flags[SQLCOM_ALTER_TABLE]= CF_CHANGES_DATA | CF_WRITE_LOGS_COMMAND; sql_command_flags[SQLCOM_TRUNCATE]= CF_CHANGES_DATA | CF_WRITE_LOGS_COMMAND; sql_command_flags[SQLCOM_DROP_TABLE]= CF_CHANGES_DATA; - sql_command_flags[SQLCOM_LOAD]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_LOAD]= CF_CHANGES_DATA | CF_DML; sql_command_flags[SQLCOM_CREATE_DB]= CF_CHANGES_DATA; sql_command_flags[SQLCOM_DROP_DB]= CF_CHANGES_DATA; sql_command_flags[SQLCOM_RENAME_TABLE]= CF_CHANGES_DATA; @@ -217,14 +217,17 @@ void init_update_queries(void) sql_command_flags[SQLCOM_ALTER_EVENT]= CF_CHANGES_DATA; sql_command_flags[SQLCOM_DROP_EVENT]= CF_CHANGES_DATA; - sql_command_flags[SQLCOM_UPDATE]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT; - sql_command_flags[SQLCOM_UPDATE_MULTI]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT; - sql_command_flags[SQLCOM_INSERT]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT; - sql_command_flags[SQLCOM_INSERT_SELECT]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT; - sql_command_flags[SQLCOM_DELETE]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT; - sql_command_flags[SQLCOM_DELETE_MULTI]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT; - sql_command_flags[SQLCOM_REPLACE]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT; - sql_command_flags[SQLCOM_REPLACE_SELECT]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT; + sql_command_flags[SQLCOM_SELECT]= CF_DML; + sql_command_flags[SQLCOM_HA_READ]= CF_DML; + sql_command_flags[SQLCOM_UPDATE]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT | CF_DML; + sql_command_flags[SQLCOM_UPDATE_MULTI]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT | CF_DML; + sql_command_flags[SQLCOM_INSERT]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT | CF_DML; + sql_command_flags[SQLCOM_INSERT_SELECT]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT | CF_DML; + sql_command_flags[SQLCOM_DELETE]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT | CF_DML; + sql_command_flags[SQLCOM_DELETE_MULTI]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT | CF_DML; + sql_command_flags[SQLCOM_REPLACE]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT | CF_DML; + sql_command_flags[SQLCOM_REPLACE_SELECT]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT | CF_DML; + sql_command_flags[SQLCOM_DO]= CF_DML; sql_command_flags[SQLCOM_SHOW_STATUS_PROC]= CF_STATUS_COMMAND; sql_command_flags[SQLCOM_SHOW_STATUS]= CF_STATUS_COMMAND; @@ -1324,21 +1327,12 @@ bool dispatch_command(enum enum_server_c break; } - thd->proc_info= "closing tables"; - /* Free tables */ - close_thread_tables(thd); + /* If commit fails, we should be able to reset the OK status. */ + thd->main_da.can_overwrite_status= TRUE; + ha_autocommit_or_rollback(thd, thd->is_error()); + thd->main_da.can_overwrite_status= FALSE; - /* - assume handlers auto-commit (if some doesn't - transaction handling - in MySQL should be redesigned to support it; it's a big change, - and it's not worth it - better to commit explicitly only writing - transactions, read-only ones should better take care of themselves. - saves some work in 2pc too) - see also sql_base.cc - close_thread_tables() - */ - bzero(&thd->transaction.stmt, sizeof(thd->transaction.stmt)); - if (!thd->active_transaction()) - thd->transaction.xid_state.xid.null(); + thd->transaction.stmt.reset(); /* report error issued during command execution */ if (thd->killed_errno() && ! thd->main_da.is_set()) @@ -1347,6 +1341,11 @@ bool dispatch_command(enum enum_server_c net_end_statement(thd); query_cache_end_of_result(thd); + thd->proc_info= "closing tables"; + /* Free tables */ + close_thread_tables(thd); + + log_slow_statement(thd); thd->proc_info="cleaning up"; @@ -1845,7 +1844,24 @@ mysql_execute_command(THD *thd) status_var_increment(thd->status_var.com_stat[lex->sql_command]); DBUG_ASSERT(thd->transaction.stmt.modified_non_trans_table == FALSE); - + + /* + If this statement (or maybe it is a sub-statement) is not + a DML statement, we have no way to keep track that it does not + change data. Mark statement transaction as read-write in all + registered engines, pessimistically. + This is a limitation of the mechanism that is used to keep + track of changes: statements that have CF_DML flag set + may change data only by means of handler::write, + handler::delete and handler::update. + So we intercept these calls and mark the transaction + read-write if one of them is issued. But non-DML statements + may issue other handler calls, which we do not trace, and + thus we can not know if they were used. + */ + if (! (sql_command_flags[thd->lex->sql_command] & CF_DML)) + thd->transaction.stmt.set_trx_read_write(); + switch (lex->sql_command) { case SQLCOM_SHOW_EVENTS: if ((res= check_access(thd, EVENT_ACL, thd->lex->select_lex.db, 0, 0, 0, @@ -2818,10 +2834,8 @@ end_with_restore_list: /* INSERT ... SELECT should invalidate only the very first table */ TABLE_LIST *save_table= first_table->next_local; first_table->next_local= 0; - mysql_unlock_tables(thd, thd->lock); query_cache_invalidate3(thd, first_table, 1); first_table->next_local= save_table; - thd->lock=0; } delete sel_result; } @@ -3107,6 +3121,7 @@ end_with_restore_list: can free its locks if LOCK TABLES locked some tables before finding that it can't lock a table in its list */ + ha_autocommit_or_rollback(thd, 1); end_active_trans(thd); thd->options&= ~(ulong) (OPTION_TABLE_LOCK); } @@ -4931,7 +4946,7 @@ check_table_access(THD *thd, ulong want_ the given table list refers to the list for prelocking (contains tables of other queries). For simple queries first_not_own_table is 0. */ - for (; tables != first_not_own_table; tables= tables->next_global) + for (; tables != first_not_own_table && tables; tables= tables->next_global) { if (tables->security_ctx) sctx= tables->security_ctx; diff -Nrup a/sql/sql_partition.cc b/sql/sql_partition.cc --- a/sql/sql_partition.cc 2007-11-04 13:35:18 +03:00 +++ b/sql/sql_partition.cc 2007-11-23 02:14:17 +03:00 @@ -3954,8 +3954,8 @@ static int fast_end_partition(THD *thd, thd->proc_info="end"; if (!is_empty) query_cache_invalidate3(thd, table_list, 0); - error= ha_commit_stmt(thd); - if (ha_commit(thd)) + error= ha_autocommit_or_rollback(thd, 0); + if (end_active_trans(thd)) error= 1; if (!error || is_empty) { diff -Nrup a/sql/sql_table.cc b/sql/sql_table.cc --- a/sql/sql_table.cc 2007-11-04 13:35:18 +03:00 +++ b/sql/sql_table.cc 2007-11-23 02:14:17 +03:00 @@ -4859,8 +4859,8 @@ mysql_discard_or_import_tablespace(THD * query_cache_invalidate3(thd, table_list, 0); /* The ALTER TABLE is always in its own transaction */ - error = ha_commit_stmt(thd); - if (ha_commit(thd)) + error = ha_autocommit_or_rollback(thd, 0); + if (end_active_trans(thd)) error=1; if (error) goto err; @@ -6367,8 +6367,8 @@ view_err: VOID(pthread_mutex_unlock(&LOCK_open)); alter_table_manage_keys(table, table->file->indexes_are_disabled(), alter_info->keys_onoff); - error= ha_commit_stmt(thd); - if (ha_commit(thd)) + error= ha_autocommit_or_rollback(thd, 0); + if (end_active_trans(thd)) error= 1; } thd->count_cuted_fields= CHECK_FIELD_IGNORE; @@ -6456,7 +6456,7 @@ view_err: /* Need to commit before a table is unlocked (NDB requirement). */ DBUG_PRINT("info", ("Committing before unlocking table")); - if (ha_commit_stmt(thd) || ha_commit(thd)) + if (ha_autocommit_or_rollback(thd, 0) || end_active_trans(thd)) goto err1; committed= 1; } @@ -6941,9 +6941,9 @@ copy_data_between_tables(TABLE *from,TAB Ensure that the new table is saved properly to disk so that we can do a rename */ - if (ha_commit_stmt(thd)) + if (ha_autocommit_or_rollback(thd, 0)) error=1; - if (ha_commit(thd)) + if (end_active_trans(thd)) error=1; err: diff -Nrup a/sql/sql_update.cc b/sql/sql_update.cc --- a/sql/sql_update.cc 2007-10-30 20:08:13 +03:00 +++ b/sql/sql_update.cc 2007-11-23 02:14:17 +03:00 @@ -792,8 +792,7 @@ int mysql_update(THD *thd, error < 0 means really no error at all: we processed all rows until the last one without error. error > 0 means an error (e.g. unique key violation and no IGNORE or REPLACE). error == 0 is also an error (if - preparing the record or invoking before triggers fails). See - ha_autocommit_or_rollback(error>=0) and DBUG_RETURN(error>=0) below. + preparing the record or invoking before triggers fails). Sometimes we want to binlog even if we updated no rows, in case user used it to be sure master and slave are in same state. */ @@ -816,17 +815,6 @@ int mysql_update(THD *thd, } DBUG_ASSERT(transactional_table || !updated || thd->transaction.stmt.modified_non_trans_table); free_underlaid_joins(thd, select_lex); - if (transactional_table) - { - if (ha_autocommit_or_rollback(thd, error >= 0)) - error=1; - } - - if (thd->lock) - { - mysql_unlock_tables(thd, thd->lock); - thd->lock=0; - } /* If LAST_INSERT_ID(X) was used, report X */ id= thd->arg_of_last_insert_id_function ? @@ -1724,13 +1712,8 @@ void multi_update::send_error(uint errco If not attempt to do remaining updates. */ - if (trans_safe) + if (! trans_safe) { - DBUG_ASSERT(transactional_tables); - (void) ha_autocommit_or_rollback(thd, 1); - } - else - { DBUG_ASSERT(thd->transaction.stmt.modified_non_trans_table); if (do_update && table_count > 1) { @@ -1739,7 +1722,7 @@ void multi_update::send_error(uint errco todo/fixme: do_update() is never called with the arg 1. should it change the signature to become argless? */ - VOID(do_updates(0)); + VOID(do_updates()); } } if (thd->transaction.stmt.modified_non_trans_table) @@ -1758,15 +1741,10 @@ void multi_update::send_error(uint errco thd->transaction.all.modified_non_trans_table= TRUE; } DBUG_ASSERT(trans_safe || !updated || thd->transaction.stmt.modified_non_trans_table); - - if (transactional_tables) - { - (void) ha_autocommit_or_rollback(thd, 1); - } } -int multi_update::do_updates(bool from_send_error) +int multi_update::do_updates() { TABLE_LIST *cur_table; int local_error= 0; @@ -1913,7 +1891,6 @@ int multi_update::do_updates(bool from_s DBUG_RETURN(0); err: - if (!from_send_error) { thd->fatal_error(); prepare_record_for_error_message(local_error, table); @@ -1951,7 +1928,7 @@ bool multi_update::send_eof() thd->proc_info="updating reference tables"; /* Does updates for the last n - 1 tables, returns 0 if ok */ - int local_error = (table_count) ? do_updates(0) : 0; + int local_error = (table_count) ? do_updates() : 0; thd->proc_info= "end"; /* We must invalidate the query cache before binlog writing and @@ -1990,12 +1967,6 @@ bool multi_update::send_eof() } if (thd->transaction.stmt.modified_non_trans_table) thd->transaction.all.modified_non_trans_table= TRUE; - } - - if (transactional_tables) - { - if (ha_autocommit_or_rollback(thd, local_error != 0)) - local_error=1; } if (local_error > 0) // if the above log write did not fail ...