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

[Sergei Golubchik]

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 ?

> @@ -1052,27 +1072,9 @@ end:
>      unlock_table_name(thd, table_list);
>      VOID(pthread_mutex_unlock(&LOCK_open));
>    }
> -  DBUG_RETURN(error);
>  
> -trunc_by_del:
> -  /* Probably InnoDB table */
> -  ulonglong save_options= thd->options;
> -  table_list->lock_type= TL_WRITE;
> -  thd->options&= ~(OPTION_BEGIN | OPTION_NOT_AUTOCOMMIT);
> -  ha_enable_transaction(thd, FALSE);
> -  mysql_init_select(thd->lex);
> -  bool save_binlog_row_based= thd->current_stmt_binlog_row_based;
> -  thd->clear_current_stmt_binlog_row_based();
> -  error= mysql_delete(thd, table_list, (COND*) 0, (SQL_LIST*) 0,
> -                      HA_POS_ERROR, LL(0), TRUE);
> -  ha_enable_transaction(thd, TRUE);
> -  /*
> -    Safety, in case the engine ignored ha_enable_transaction(FALSE)
> -    above. Also clears thd->transaction.*.
> -  */
> -  error= ha_autocommit_or_rollback(thd, error);
> -  ha_commit(thd);
> -  thd->options= save_options;
> -  thd->current_stmt_binlog_row_based= save_binlog_row_based;
> +  ha_autocommit_or_rollback(thd, 0);
> +  end_active_trans(thd);
> +
>    DBUG_RETURN(error);
>  }
> 
Regards / Mit vielen Grüßen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Engineer/Server Architect
/_/  /_/\_, /___/\___\_\___/  Sun Microsystems GmbH, HRB München 161028
       <___/                  Sonnenallee 1, 85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schroeder, Wolfgang Engels, Dr. Roland Boemer
Vorsitzender des Aufsichtsrates: Martin Häring