Re: bzr commit into mysql-5.1-bugteam branch (davi:2741) Bug#37016

[Davi Arnaut]

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