From: Date: June 6 2007 3:55pm Subject: bk commit into 5.0 tree (aelkin:1.2439) BUG#27417 List-Archive: http://lists.mysql.com/commits/28205 X-Bug: 27417 Message-Id: <200706061355.l56DtArD016962@dsl-hkibras1-ff5dc300-70.dhcp.inet.fi> Below is the list of changes that have just been committed into a local 5.0 repository of elkin. When elkin 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-06-06 16:55:00+03:00, aelkin@stripped +9 -0 Bug #27417 thd->no_trans_update.stmt lost value inside of SF-exec-stack Once had been set the flag might later got reset inside of a stored routine execution stack. The reason was in that there was no check if a new statement started at time of resetting. The artifact affects most of binlogable DML queries. Notice, that multi-update is wrapped up within bug#27716 fix. Fixed with introducing Sub_statement_state::no_trans_update_stmt flag durable within substatement life time. At the end of substatement the gained value contributes to thd->no_trans_update.stmt so that on the bottom line are: 1. Eventually thd->no_trans_update.stmt will be propagated out of the last substatement as the union of values worked out in all sub-statements - fixed bug#27417 2. Inside of any sub-statement thd->no_trans_update.stmt corresponds to the value yielded at the sub-statement to satisfy THD::really_abort_on_warnin() concern to get the status of the current substatement. The supplied test verifies limitely, mostly asserts. The ultimate testing is defered for bug_13270, bug_23333. mysql-test/r/stored_routines_side_effect.result@stripped, 2007-06-06 16:54:54+03:00, aelkin@stripped +26 -0 new result file mysql-test/r/stored_routines_side_effect.result@stripped, 2007-06-06 16:54:54+03:00, aelkin@stripped +0 -0 mysql-test/t/sp_trans.test@stripped, 2007-06-06 16:54:52+03:00, aelkin@stripped +1 -0 not-related test clean-up spotted and fixed mysql-test/t/stored_routines_side_effect.test@stripped, 2007-06-06 16:54:53+03:00, aelkin@stripped +56 -0 the test basicly to check asserts deployed with the patch. mysql-test/t/stored_routines_side_effect.test@stripped, 2007-06-06 16:54:53+03:00, aelkin@stripped +0 -0 sql/sql_class.cc@stripped, 2007-06-06 16:54:52+03:00, aelkin@stripped +3 -0 backing up and resetting .stmt flag for a new starting statement; restoring the flag as the union with the value yieded by the current sub-statement; sql/sql_class.h@stripped, 2007-06-06 16:54:53+03:00, aelkin@stripped +5 -0 adding a member Sub_statement_state sql/sql_delete.cc@stripped, 2007-06-06 16:54:53+03:00, aelkin@stripped +23 -7 correcting basic delete incl truncate branch and multi-delete queries to set and reset thd->no_trans_update.stmt; sql/sql_insert.cc@stripped, 2007-06-06 16:54:53+03:00, aelkin@stripped +14 -8 asserts regarding to the .stmt flag sql/sql_load.cc@stripped, 2007-06-06 16:54:53+03:00, aelkin@stripped +4 -6 asserts regarding to the flag; eliminating a separate issue where the flag was stored and re-stored after write_record that could change it and the change would be lost but should remain permanent. sql/sql_update.cc@stripped, 2007-06-06 16:54:53+03:00, aelkin@stripped +5 -1 correcting update (multi-update part of the issues covered by other bug fix) to set and reset the flag; assert is added. # This is a BitKeeper patch. What follows are the unified diffs for the # set of deltas contained in the patch. The rest of the patch, the part # that BitKeeper cares about, is below these diffs. # User: aelkin # Host: dsl-hkibras1-ff5dc300-70.dhcp.inet.fi # Root: /home/elkin/MySQL/TEAM/FIXES/5.0/bug27417-no_trans_update__stmt --- 1.269/sql/sql_class.cc 2007-04-20 11:35:22 +03:00 +++ 1.270/sql/sql_class.cc 2007-06-06 16:54:52 +03:00 @@ -2098,6 +2098,8 @@ void THD::reset_sub_statement_state(Sub_ backup->cuted_fields= cuted_fields; backup->client_capabilities= client_capabilities; backup->savepoints= transaction.savepoints; + backup->no_trans_update_stmt= no_trans_update.stmt; + no_trans_update.stmt= 0; if (!lex->requires_prelocking() || is_update_query(lex->sql_command)) options&= ~OPTION_BIN_LOG; @@ -2160,6 +2162,7 @@ void THD::restore_sub_statement_state(Su */ examined_row_count+= backup->examined_row_count; cuted_fields+= backup->cuted_fields; + no_trans_update.stmt= backup->no_trans_update_stmt || no_trans_update.stmt; } --- 1.328/sql/sql_class.h 2007-04-03 14:55:14 +03:00 +++ 1.329/sql/sql_class.h 2007-06-06 16:54:53 +03:00 @@ -1086,6 +1086,11 @@ public: bool last_insert_id_used; my_bool no_send_ok; SAVEPOINT *savepoints; + /* + the flag resets at the beginning of substatement and restores as + the union with the gained value at the end of the substatement + */ + bool no_trans_update_stmt; }; /** --- 1.197/sql/sql_delete.cc 2007-04-17 16:51:56 +03:00 +++ 1.198/sql/sql_delete.cc 2007-06-06 16:54:53 +03:00 @@ -91,7 +91,7 @@ bool mysql_delete(THD *thd, TABLE_LIST * } select_lex->no_error= thd->lex->ignore; - + transactional_table= table->file->has_transactions(); /* Test if the user wants to delete all rows and deletion doesn't have any side-effects (because of triggers), so we can use optimized @@ -109,8 +109,11 @@ bool mysql_delete(THD *thd, TABLE_LIST * if (!(error=table->file->delete_all_rows())) { error= -1; // ok + if (!transactional_table && deleted > 0) + thd->no_trans_update.stmt= TRUE; goto cleanup; } + deleted= 0; if (error != HA_ERR_WRONG_COMMAND) { table->file->print_error(error,MYF(0)); @@ -216,6 +219,8 @@ bool mysql_delete(THD *thd, TABLE_LIST * init_ftfuncs(thd, select_lex, 1); thd->proc_info="updating"; + thd->no_trans_update.stmt= FALSE; + if (table->triggers) { table->triggers->mark_fields_used(thd, TRG_EVENT_DELETE); @@ -280,6 +285,9 @@ bool mysql_delete(THD *thd, TABLE_LIST * else table->file->unlock_row(); // Row failed selection, release lock on it } + if (!transactional_table && deleted > 0) + thd->no_trans_update.stmt= TRUE; + if (thd->killed && !error) error= 1; // Aborted thd->proc_info="end"; @@ -314,7 +322,6 @@ cleanup: } delete select; - transactional_table= table->file->has_transactions(); /* See similar binlogging code in sql_update.cc, for comments */ if ((error < 0) || (deleted && !transactional_table)) { @@ -330,6 +337,7 @@ cleanup: if (!transactional_table) thd->no_trans_update.all= TRUE; } + DBUG_ASSERT(transactional_table || !deleted || thd->no_trans_update.stmt); free_underlaid_joins(thd, select_lex); if (transactional_table) { @@ -526,6 +534,7 @@ multi_delete::initialize_tables(JOIN *jo Unique **tempfiles_ptr; DBUG_ENTER("initialize_tables"); + thd->no_trans_update.stmt= FALSE; if ((thd->options & OPTION_SAFE_UPDATES) && error_if_full_join(join)) DBUG_RETURN(1); @@ -644,20 +653,22 @@ bool multi_delete::send_data(List if (table->triggers && table->triggers->process_triggers(thd, TRG_EVENT_DELETE, TRG_ACTION_BEFORE, FALSE)) - DBUG_RETURN(1); + DBUG_RETURN(1); table->status|= STATUS_DELETED; if (!(error=table->file->delete_row(table->record[0]))) { - deleted++; + deleted++; + if (!table->file->has_transactions()) + thd->no_trans_update.stmt= TRUE; if (table->triggers && table->triggers->process_triggers(thd, TRG_EVENT_DELETE, TRG_ACTION_AFTER, FALSE)) - DBUG_RETURN(1); + DBUG_RETURN(1); } else { - table->file->print_error(error,MYF(0)); - DBUG_RETURN(1); + table->file->print_error(error,MYF(0)); + DBUG_RETURN(1); } } else @@ -734,6 +745,7 @@ int multi_delete::do_deletes() for (; table_being_deleted; table_being_deleted= table_being_deleted->next_local, counter++) { + ha_rows last_deleted= deleted; TABLE *table = table_being_deleted->table; if (tempfiles[counter]->get(table)) { @@ -771,6 +783,8 @@ int multi_delete::do_deletes() break; } } + if (last_deleted != deleted && !table->file->has_transactions()) + thd->no_trans_update.stmt= TRUE; end_read_record(&info); if (thd->killed && !local_error) local_error= 1; @@ -824,6 +838,8 @@ bool multi_delete::send_eof() if (!transactional_tables) thd->no_trans_update.all= TRUE; } + DBUG_ASSERT(!normal_tables || !deleted || thd->no_trans_update.stmt); + /* Commit or rollback the current SQL statement */ if (transactional_tables) if (ha_autocommit_or_rollback(thd,local_error > 0)) --- 1.230/sql/sql_insert.cc 2007-04-12 12:46:07 +03:00 +++ 1.231/sql/sql_insert.cc 2007-06-06 16:54:53 +03:00 @@ -779,6 +779,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *t thd->no_trans_update.all= TRUE; } } + DBUG_ASSERT(transactional_table || !changed || thd->no_trans_update.stmt); if (transactional_table) error=ha_autocommit_or_rollback(thd,error); @@ -2691,6 +2692,7 @@ void select_insert::store_values(Listfile->has_transactions(); if (!thd->prelocked_mode) table->file->end_bulk_insert(); /* @@ -2711,21 +2714,22 @@ void select_insert::send_error(uint errc error while inserting into a MyISAM table) we must write to the binlog (and the error code will make the slave stop). */ - if ((info.copied || info.deleted || info.updated) && - !table->file->has_transactions()) + if ((changed= info.copied || info.deleted || info.updated) && + !transactional_table) { if (last_insert_id) thd->insert_id(last_insert_id); // For binary log if (mysql_bin_log.is_open()) { Query_log_event qinfo(thd, thd->query, thd->query_length, - table->file->has_transactions(), FALSE); + transactional_table, FALSE); mysql_bin_log.write(&qinfo); } if (!table->s->tmp_table) thd->no_trans_update.all= TRUE; } - if (info.copied || info.deleted || info.updated) + DBUG_ASSERT(transactional_table || !changed || thd->no_trans_update.stmt); + if (changed) { query_cache_invalidate3(thd, table, 1); } @@ -2736,7 +2740,8 @@ void select_insert::send_error(uint errc bool select_insert::send_eof() { - int error,error2; + int error, error2; + bool changed, transactional_table= table->file->has_transactions(); DBUG_ENTER("select_insert::send_eof"); error= (!thd->prelocked_mode) ? table->file->end_bulk_insert():0; @@ -2748,12 +2753,13 @@ bool select_insert::send_eof() and ha_autocommit_or_rollback */ - if (info.copied || info.deleted || info.updated) + if (changed= (info.copied || info.deleted || info.updated)) { query_cache_invalidate3(thd, table, 1); - if (!(table->file->has_transactions() || table->s->tmp_table)) + if (!(transactional_table || table->s->tmp_table)) thd->no_trans_update.all= TRUE; } + DBUG_ASSERT(transactional_table || !changed || thd->no_trans_update.stmt); if (last_insert_id) thd->insert_id(info.copied ? last_insert_id : 0); // For binary log @@ -2763,7 +2769,7 @@ bool select_insert::send_eof() if (!error) thd->clear_error(); Query_log_event qinfo(thd, thd->query, thd->query_length, - table->file->has_transactions(), FALSE); + transactional_table, FALSE); mysql_bin_log.write(&qinfo); } if ((error2=ha_autocommit_or_rollback(thd,error)) && ! error) --- 1.113/sql/sql_load.cc 2007-05-08 03:43:16 +03:00 +++ 1.114/sql/sql_load.cc 2007-06-06 16:54:53 +03:00 @@ -488,6 +488,8 @@ bool mysql_load(THD *thd,sql_exchange *e /* ok to client sent only after binlog write and engine commit */ send_ok(thd, info.copied + info.deleted, 0L, name); err: + DBUG_ASSERT(transactional_table || !(info.copied || info.deleted) || + thd->no_trans_update.stmt); if (thd->lock) { mysql_unlock_tables(thd, thd->lock); @@ -532,7 +534,7 @@ read_fixed_length(THD *thd, COPY_INFO &i Item_field *sql_field; TABLE *table= table_list->table; ulonglong id; - bool no_trans_update_stmt, err; + bool err; DBUG_ENTER("read_fixed_length"); id= 0; @@ -560,7 +562,6 @@ read_fixed_length(THD *thd, COPY_INFO &i #ifdef HAVE_purify read_info.row_end[0]=0; #endif - no_trans_update_stmt= !table->file->has_transactions(); restore_record(table, s->default_values); /* @@ -628,7 +629,6 @@ read_fixed_length(THD *thd, COPY_INFO &i table->auto_increment_field_not_null= FALSE; if (err) DBUG_RETURN(1); - thd->no_trans_update.stmt= no_trans_update_stmt; /* If auto_increment values are used, save the first one for @@ -671,12 +671,11 @@ read_sep_field(THD *thd, COPY_INFO &info TABLE *table= table_list->table; uint enclosed_length; ulonglong id; - bool no_trans_update_stmt, err; + bool err; DBUG_ENTER("read_sep_field"); enclosed_length=enclosed.length(); id= 0; - no_trans_update_stmt= !table->file->has_transactions(); for (;;it.rewind()) { @@ -817,7 +816,6 @@ read_sep_field(THD *thd, COPY_INFO &info We don't need to reset auto-increment field since we are restoring its default value at the beginning of each loop iteration. */ - thd->no_trans_update.stmt= no_trans_update_stmt; if (read_info.next_line()) // Skip to next line break; if (read_info.line_cuted) --- 1.215/sql/sql_update.cc 2007-04-12 12:46:07 +03:00 +++ 1.216/sql/sql_update.cc 2007-06-06 16:54:53 +03:00 @@ -486,7 +486,6 @@ int mysql_update(THD *thd, (byte*) table->record[0]))) { updated++; - thd->no_trans_update.stmt= !transactional_table; if (table->triggers && table->triggers->process_triggers(thd, TRG_EVENT_UPDATE, @@ -520,6 +519,10 @@ int mysql_update(THD *thd, table->file->unlock_row(); thd->row_count++; } + + if (!transactional_table && updated > 0) + thd->no_trans_update.stmt= TRUE; + if (thd->killed && !error) error= 1; // Aborted end_read_record(&info); @@ -560,6 +563,7 @@ int mysql_update(THD *thd, if (!transactional_table) thd->no_trans_update.all= TRUE; } + DBUG_ASSERT(transactional_table || !updated || thd->no_trans_update.stmt); free_underlaid_joins(thd, select_lex); if (transactional_table) { --- New file --- +++ mysql-test/r/stored_routines_side_effect.result 07/06/06 16:54:54 drop function if exists bug27417; drop table if exists t1,t2; CREATE TABLE t1 (a int NOT NULL auto_increment primary key) ENGINE=MyISAM; CREATE TABLE t2 (a int NOT NULL auto_increment, PRIMARY KEY (a)); create function bug27417(n int) RETURNS int(11) DETERMINISTIC begin insert into t1 values (null); return n; end| reset master; insert into t2 values (bug27417(1)); insert into t2 select bug27417(2); reset master; insert into t2 values (bug27417(2)); ERROR 23000: Duplicate entry '2' for key 1 show master status; File Position Binlog_Do_DB Binlog_Ignore_DB master-bin.000001 98 /* with fixes for #23333 will show there is the query */; select count(*) from t1 /* must be 3 */; count(*) 3 drop table t1,t2; end of tests --- New file --- +++ mysql-test/t/stored_routines_side_effect.test 07/06/06 16:54:53 # # Bug #27417 thd->no_trans_update.stmt lost value inside of SF-exec-stack # # Testing the assertion: if there is a side effect of modifying non-transactional # table thd->no_trans_update.stmt must be TRUE; # the assert is active with debug build # --disable_warnings drop function if exists bug27417; drop table if exists t1,t2; --enable_warnings # side effect table CREATE TABLE t1 (a int NOT NULL auto_increment primary key) ENGINE=MyISAM; # target tables CREATE TABLE t2 (a int NOT NULL auto_increment, PRIMARY KEY (a)); delimiter |; create function bug27417(n int) RETURNS int(11) DETERMINISTIC begin insert into t1 values (null); return n; end| delimiter ;| reset master; # execute insert into t2 values (bug27417(1)); insert into t2 select bug27417(2); reset master; --error ER_DUP_ENTRY insert into t2 values (bug27417(2)); show master status; /* only (!) with fixes for #23333 will show there is the query */; select count(*) from t1 /* must be 3 */; reset master; select count(*) from t2; delete from t2 where a=bug27417(3); select count(*) from t2 /* nothing got deleted */; show master status; /* the query must be in regardless of #23333 */; select count(*) from t1 /* must be 5 */; --enable_info delete t2 from t2 where t2.a=bug27417(100) /* must not affect t2 */; --disable_info select count(*) from t1 /* must be 7 */; drop table t1,t2; --echo end of tests --- 1.12/mysql-test/t/sp_trans.test 2007-03-24 19:19:58 +02:00 +++ 1.13/mysql-test/t/sp_trans.test 2007-06-06 16:54:52 +03:00 @@ -583,6 +583,7 @@ insert into t2 values (bug23333(),1)| --replace_column 2 # 5 # 6 # show binlog events from 98 /* with fixes for #23333 will show there is the query */| select count(*),@a from t1 /* must be 1,1 */| +drop function bug23333| # # BUG#NNNN: New bug synopsis