Comment 3 for bug 544303

Revision history for this message
Jay Pipes (jaypipes) wrote : Re: Problem with sync on Solaris - numerous test failures

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(&LOCK_open);
        goto err;
      }

      /*
        Data is copied. Now we:
        1) Wait until all other threads close old version of table.
        2) Close instances of table open by this thread and replace them
        with exclusive name-locks.
        3) Rename the old table to a temp name, rename the new one to the
        old name.
        4) If we are under LOCK TABLES and don't do ALTER Table ... RENAME
        we reopen new version of table.
        5) Write statement to the binary log.
        6) If we are under LOCK TABLES and do ALTER Table ... RENAME we
        remove name-locks from list of open tables and table cache.
        7) If we are not not under LOCK TABLES we rely on close_thread_tables()
        call to remove name-locks from table cache and list of open table.
      */

      session->set_proc_info("rename result table");

      snprintf(old_name, sizeof(old_name), "%s2-%lx-%"PRIx64, TMP_FILE_PREFIX, (unsigned long) current_pid, session->thread_id);

      my_casedn_str(files_charset_info, old_name);

      wait_while_table_is_used(session, table, HA_EXTRA_PREPARE_FOR_RENAME);
      session->close_data_files_and_morph_locks(db, table_name);

      error= 0;
      save_old_db_type= old_db_type;

      /*
        This leads to the storage engine (SE) not being notified for renames in
        mysql_rename_table(), because we just juggle with the FRM and nothing
        more. If we have an intermediate table, then we notify the SE that
        it should become the actual table. Later, we will recycle the old table.
        However, in case of ALTER Table RENAME there might be no intermediate
        table. This is when the old and new tables are compatible, according to
        compare_table(). Then, we need one additional call to
      */
      if (mysql_rename_table(old_db_type, db, table_name, db, old_name, FN_TO_IS_TMP))
      {
        error= 1;
        TableIdentifier identifier(new_db, tmp_name, INTERNAL_TMP_TABLE);
        quick_rm_table(*session, identifier);
      }
      else
      {
        if (mysql_rename_table(new_db_type, new_db, tmp_name, new_db, new_alias, FN_FROM_IS_TMP) != 0)
        {
          /* Try to get everything back. */
          error= 1;

          TableIdentifier alias_identifier(new_db, new_alias, STANDARD_TABLE);
          quick_rm_table(*session, alias_identifier);

          TableIdentifier tmp_identifier(new_db, tmp_name, INTERNAL_TMP_TABLE);
          quick_rm_table(*session, tmp_identifier);

          mysql_rename_table(old_db_type, db, old_name, db, table_name, FN_FROM_IS_TMP);
        }
      }

      if (error)
      {
        /*
          An error happened while we were holding exclusive name-lock on table
          being altered. To be safe under LOCK TABLES we should remove placeholders
          from list of open tables list and table cache.
        */
        session->unlink_open_table(table);
        if (name_lock)
          session->unlink_open_table(name_lock);
        pthread_mutex_unlock(&LOCK_open);
        return true;
      }

      {
        TableIdentifier old_identifier(db, old_name, INTERNAL_TMP_TABLE);
        quick_rm_table(*session, old_identifier);
      }

      pthread_mutex_unlock(&LOCK_open);

      session->set_proc_info("end");

      write_bin_log(session, session->query.c_str());
      table_list->table= NULL;
    }

    /*
     * Field::store() may have called my_error(). If this is
     * the case, we must not send an ok packet, since
     * Diagnostics_area::is_set() will fail an assert.
   */
    if (! session->is_error())
    {
      snprintf(tmp_name, sizeof(tmp_name), ER(ER_INSERT_INFO),
               (ulong) (copied + deleted), (ulong) deleted,
               (ulong) session->cuted_fields);
      session->my_ok(copied + deleted, 0, 0L, tmp_name);
      session->some_tables_deleted=0;
      return false;
    }
    else
    {
      /* my_error() was called. Return true (which means error...) */
      return true;
    }