Problem with temp table identifiers on Solaris - numerous test failures

[Lee Bieber]

Over 70 test failures, see the complete log at http://hudson.drizzle.org/view/Drizzle-build/job/drizzle-build-opensolaris-64/97/console

[Brian Aker (brianaker)]

Hi!

I am wondering if the C++ IO calls are not calling sync when they
close...

Cheers,
 -brian

[Jay Pipes (jaypipes)]

No, it's a bug in temporary table creation only.

[Jay Pipes (jaypipes)]

I believe it was this change which is buggy:

=== modified file 'drizzled/statement/alter_table.cc'
--- drizzled/statement/alter_table.cc 2010-03-20 21:48:16 +0000
+++ drizzled/statement/alter_table.cc 2010-03-22 17:53:05 +0000
@@ -795,12 +795,10 @@
         }

         TableIdentifier identifier(new_db, lower_case_table_name);
-
         if (plugin::StorageEngine::doesTableExist(*session, identifier))
         {
           /* Table will be closed by Session::executeCommand() */
           my_error(ER_TABLE_EXISTS_ERROR, MYF(0), new_alias);
- goto err;
           error_on_rename= true;
         }
       }

It should have instead removed the error_on_rename= true; statement. The goto was necessary to skip the code blocks afterwards. Namely, this block of code is now executed when it wasn't before:

    /* Open the table so we need to copy the data to it. */
    new_table= open_alter_table(session, table, new_db, tmp_name);

    if (new_table == NULL)
      goto err1;

    /* Copy the data if necessary. */
    session->count_cuted_fields= CHECK_FIELD_WARN; // calc cuted fields
    session->cuted_fields= 0L;
    session->set_proc_info("copy to tmp table");
    copied= deleted= 0;

    assert(new_table);

    /* We don't want update TIMESTAMP fields during ALTER Table. */
    new_table->timestamp_field_type= TIMESTAMP_NO_AUTO_SET;
    new_table->next_number_field= new_table->found_next_number_field;
    error= copy_data_between_tables(table,
                                    new_table,
                                    alter_info->create_list,
                                    ignore,
                                    order_num,
                                    order,
                                    &copied,
                                    &deleted,
                                    alter_info->keys_onoff,
                                    alter_info->error_if_not_empty);

    /* We must not ignore bad input! */
    session->count_cuted_fields= CHECK_FIELD_ERROR_FOR_NULL;

    if (table->s->tmp_table != STANDARD_TABLE)
    {
      /* We changed a temporary table */
      if (error)
        goto err1;

      /* Close lock if this is a transactional table */
      if (session->lock)
      {
        mysql_unlock_tables(session, session->lock);
        session->lock= 0;
      }

      /* Remove link to old table and rename the new one */
      session->close_temporary_table(table);

      /* Should pass the 'new_name' as we store table name in the cache */
      TableIdentifier alter_identifier(new_db, new_name);
      if (new_table->renameAlterTemporaryTable(alter_identifier))
        goto err1;
    }
    else
    {
      if (new_table)
      {
        /*
          Close the intermediate table that will be the new table.
          Note that MERGE tables do not have their children attached here.
        */
        new_table->intern_close_table();
        free(new_table);
      }

      pthread_mutex_lock(&LOCK_open); /* ALTER TABLE */

      if (error)
      {
        TableIdentifier identifier(new_db, tmp_name, INTERNAL_TMP_TABLE);
        quick_rm_table(*session, identifier);
        pthread_mutex_unlock(&LOC...

[Jay Pipes (jaypipes)]

Disregard last comment...was an indentation issue on my end...that block is indeed skipped.

[Monty Taylor (mordred)]

Turns out this has nothing to do with sync whatsoever. It actually has to do with TableIdentifiers during temp table creation.