From: Date: December 23 2008 12:20am Subject: Re: bzr commit into mysql-5.1-bugteam branch (davi:2741) Bug#37016 List-Archive: http://lists.mysql.com/commits/62228 Message-Id: <495020A1.2000802@Sun.COM> MIME-Version: 1.0 Content-Type: text/plain; format=flowed; charset=ISO-8859-1 Content-Transfer-Encoding: 7BIT Hi Sergei, On 12/22/08 8:48 PM, Sergei Golubchik wrote: > Hi, Davi! > > On Dec 13, Davi Arnaut wrote: >> # At a local mysql-5.1-bugteam repository of davi >> >> 2741 Davi Arnaut 2008-12-13 >> Bug#37016: TRUNCATE TABLE removes some rows but not all >> >> The special TRUNCATE TABLE (DDL) transaction wasn't being properly >> rolled back if a error occurred during row by row deletion. The >> error can be caused by a foreign key restriction imposed by InnoDB >> SE and woudl cause the server to erroneously issue a implicit >> commit. >> >> The solution is to rollback the transaction if a truncation via row >> by row deletion fails, otherwise commit. If the truncation is >> emulated by a table recreation, always issue a implicit commit. >> >> This makes TRUNCATE TABLE a special case of DDL because a implicit >> commit is NOT issued regardless of the outcome of the operation -- >> a implicit rollback is issued if row by row deletion fails. >> === modified file 'sql/sql_delete.cc' >> --- a/sql/sql_delete.cc 2008-11-03 13:08:42 +0000 >> +++ b/sql/sql_delete.cc 2008-12-13 10:58:32 +0000 >> @@ -951,6 +951,26 @@ bool multi_delete::send_eof() >> ****************************************************************************/ >> >> /* >> + Row-by-row truncation if the engine does not support table recreation. >> + Probably a InnoDB table. >> +*/ >> + >> +static bool mysql_truncate_by_delete(THD *thd, TABLE_LIST *table_list) >> +{ >> + bool error, save_binlog_row_based= thd->current_stmt_binlog_row_based; >> + DBUG_ENTER("mysql_truncate_by_delete"); >> + table_list->lock_type= TL_WRITE; >> + mysql_init_select(thd->lex); >> + thd->clear_current_stmt_binlog_row_based(); >> + error= mysql_delete(thd, table_list, NULL, NULL, HA_POS_ERROR, LL(0), TRUE); >> + ha_autocommit_or_rollback(thd, error); >> + end_trans(thd, error ? ROLLBACK : COMMIT); >> + thd->current_stmt_binlog_row_based= save_binlog_row_based; >> + DBUG_RETURN(error); >> +} > > to summarize your changes: > > 1. you moved this to a separate function > * okay, as you like > > 2. removed ha_enable_transaction() > * that's what fixes the bug > > 3. removed the code that was enabling autocommit > * why ? > Because it's not necessary as the TRUNCATE transaction is explicitly ended (committed or rolled back). IMHO, there is no need for a yet another special case here... or I'm missing something? Regards, -- Davi