BTW just as useful (or possibly useful) info, these tables are very special.

If you are coming from a temporary table we just create another temporary table and switch the names at the end (and there is no recovery).

From a normal table, you create an INTERNAL_TABLE which half exists in the system. It is for all purposes a normal table except that it bypasses a number of systems along the way (like locking). During an alter table this table can become lost during a crash.

Unfortunately the code paths for TEMP tables and normal tables if different still. 

On Apr 28, 2010, at 9:41 AM, Jay Pipes wrote:

> The problem lies in this block of code in
> /drizzled/statement/alter_table.cc (lines 981-1000):
> 
> <code>
>  /* Create a temporary table with the new format */
>  /**
>    @note we make an internal temporary table unless the table is a temporary table. In last
>    case we just use it as is. Neither of these tables require locks in order to  be
>    filled.
>  */
>  TableIdentifier new_table_as_temporary(original_table_identifier.getSchemaName(),
>                                         tmp_name,
>                                         create_proto.type() != message::Table::TEMPORARY ? message::Table::INTERNAL :
>                                         message::Table::TEMPORARY);
> 
>  error= create_temporary_table(session, new_table_as_temporary,
> create_info, create_proto, alter_info);
> 
>  if (error != 0)
>  {
>    return true;
>  }
> 
>  /* Open the table so we need to copy the data to it. */
>  new_table= open_alter_table(session, table, new_table_as_temporary);
> 
> </code>
> 
> The reason it is problematic in relation to this bug is the comment
> here:
> 
> /**
>    @note we make an internal temporary table unless the table is a temporary table. In last
>    case we just use it as is. Neither of these tables require locks in order to  be
>    filled.
> */
> 
> Unfortunately, the only two ways that
> plugin::TransactionalStorageEngine::doStartTransaction() is called are
> the following two ways:
> 
> 1) If it is explicitly called by the kernel (which happens when a START TRANSACTION/BEGIN WORK; SQL statement is issued, or
> 2) When mysql_lock_tables() is called.   The kernel decides in /drizzled/lock.cc which engines are active in a statement and calls plugin::TransactionalStorageEngine::doStartTransaction() for each transactional engine if it has not already started a transaction for the engine
> 
> So, clearly, the comment that "Neither of these tables require locks in
> order to be filled" is a hint here.  Indeed, if we trace through the
> alter_table() method in /drizzled/statement/alter_table.cc, we find that
> during the copy phase (where the new table is being filled with a copy
> of the old table's data, that instead of Session::openTableLock() being
> called (which DOES call mysql_lock_tables()), instead
> Session::open_temporary_table() is called, which does NOT call
> mysql_lock_tables().
> 
> Therefore, the solution to this is to manually call
> plugin::TransactionalStorageEngine::doStartTransaction() *somewhere*
> before the copying of the data from the old table into the new table
> (when Cursor::rnd_init() is eventually callled...)
> 
> I should have a fix for this by end of day today.  Just trying to find
> the "right" place to put the call to doStartTransaction()...
> 
> -- 
> implicit startTransaction in ALTER TABLE calling rnd_init to do data copy
> https://bugs.launchpad.net/bugs/552420
> You received this bug notification because you are a member of Drizzle-
> developers, which is subscribed to Drizzle.
> 
> Status in A Lightweight SQL Database for Cloud and Web: In Progress
> Status in Drizzle cherry series: In Progress
> 
> Bug description:
> in an ALTER TABLE, your engine is (currently) expected to be able to start a transaction in rnd_init().
> 
> more info coming (as in, "remove these two lines from embedded_innodb and you'll see it crash)
> 
>