Patch changing how ALTER TABLE implementation handles table locking

and invalidation in the most general case (non-temporary table and
not simple RENAME or ENABLE/DISABLE KEYS or partitioning command).

See comment for sql/sql_table.cc for more information.

These changes are prerequisite for 5.1 version of fix for bug #23667
"CREATE TABLE LIKE is not isolated from alteration by other connections"

mysql-test/include/mix1.inc

@@ -762,4 +762,33 @@ alter table t2 modify i int default 4, rename t1;
 unlock tables;
 drop table t1;
 
+
+# 
+# Some more tests for ALTER TABLE and LOCK TABLES for transactional tables.
+#
+# Table which is altered under LOCK TABLES should stay in list of locked
+# tables and be available after alter takes place unless ALTER contains
+# RENAME clause. We should see the new definition of table, of course.
+# Before 5.1 this behavior was inconsistent across the platforms and
+# different engines. See also tests in alter_table.test
+#
+--disable_warnings
+drop table if exists t1;
+--enable_warnings
+create table t1 (i int);
+insert into t1 values ();
+lock table t1 write;
+# Example of so-called 'fast' ALTER TABLE
+alter table t1 modify i int default 1;
+insert into t1 values ();
+select * from t1;
+# And now full-blown ALTER TABLE
+alter table t1 change i c char(10) default "Two";
+insert into t1 values ();
+select * from t1;
+unlock tables;
+select * from t1;
+drop tables t1;
+
+
 --echo End of 5.1 tests

mysql-test/r/alter_table-big.result

@@ -5,14 +5,53 @@ key (n2, n3, n1),
 key (n3, n1, n2));
 create table t2 (i int);
 alter table t1 disable keys;
+insert into t1 values (RAND()*1000, RAND()*1000, RAND()*1000);
 reset master;
+set session debug="+d,sleep_alter_enable_indexes";
 alter table t1 enable keys;;
 insert into t2 values (1);
 insert into t1 values (1, 1, 1);
-show binlog events in 'master-bin.000001' from 102;
+set session debug="-d,sleep_alter_enable_indexes";
+show binlog events in 'master-bin.000001' from 106;
 Log_name	Pos	Event_type	Server_id	End_log_pos	Info
 master-bin.000001	#	Query	1	#	use `test`; insert into t2 values (1)
 master-bin.000001	#	Query	1	#	use `test`; alter table t1 enable keys
 master-bin.000001	#	Query	1	#	use `test`; insert into t1 values (1, 1, 1)
 drop tables t1, t2;
 End of 5.0 tests
+drop table if exists t1, t2, t3;
+create table t1 (i int);
+reset master;
+set session debug="+d,sleep_alter_before_main_binlog";
+alter table t1 change i c char(10) default 'Test1';;
+insert into t1 values ();
+select * from t1;
+c
+Test1
+alter table t1 change c vc varchar(100) default 'Test2';;
+rename table t1 to t2;
+drop table t2;
+create table t1 (i int);
+alter table t1 change i c char(10) default 'Test3', rename to t2;;
+insert into t2 values ();
+select * from t2;
+c
+Test3
+alter table t2 change c vc varchar(100) default 'Test2', rename to t1;;
+rename table t1 to t3;
+drop table t3;
+set session debug="-d,sleep_alter_before_main_binlog";
+show binlog events in 'master-bin.000001' from 106;
+Log_name	Pos	Event_type	Server_id	End_log_pos	Info
+master-bin.000001	#	Query	1	#	use `test`; alter table t1 change i c char(10) default 'Test1'
+master-bin.000001	#	Query	1	#	use `test`; insert into t1 values ()
+master-bin.000001	#	Query	1	#	use `test`; alter table t1 change c vc varchar(100) default 'Test2'
+master-bin.000001	#	Query	1	#	use `test`; rename table t1 to t2
+master-bin.000001	#	Query	1	#	use `test`; drop table t2
+master-bin.000001	#	Query	1	#	use `test`; create table t1 (i int)
+master-bin.000001	#	Query	1	#	use `test`; alter table t1 change i c char(10) default 'Test3', rename to t2
+master-bin.000001	#	Query	1	#	use `test`; insert into t2 values ()
+master-bin.000001	#	Query	1	#	use `test`; alter table t2 change c vc varchar(100) default 'Test2', rename to t1
+master-bin.000001	#	Query	1	#	use `test`; rename table t1 to t3
+master-bin.000001	#	Query	1	#	use `test`; drop table t3
+End of 5.1 tests

mysql-test/r/alter_table.result

@@ -977,6 +977,59 @@ SELECT * FROM t1;
 v	b
 abc	5
 DROP TABLE t1;
+End of 5.0 tests
+drop table if exists t1, t2, t3;
+create table t1 (i int);
+create table t3 (j int);
+insert into t1 values ();
+insert into t3 values ();
+lock table t1 write, t3 read;
+alter table t1 modify i int default 1;
+insert into t1 values ();
+select * from t1;
+i
+NULL
+1
+alter table t1 change i c char(10) default "Two";
+insert into t1 values ();
+select * from t1;
+c
+NULL
+1
+Two
+alter table t1 modify c char(10) default "Three", rename to t2;
+select * from t1;
+ERROR HY000: Table 't1' was not locked with LOCK TABLES
+select * from t2;
+ERROR HY000: Table 't2' was not locked with LOCK TABLES
+select * from t3;
+j
+NULL
+unlock tables;
+insert into t2 values ();
+select * from t2;
+c
+NULL
+1
+Three
+lock table t2 write, t3 read;
+alter table t2 change c vc varchar(100) default "Four", rename to t1;
+select * from t1;
+ERROR HY000: Table 't1' was not locked with LOCK TABLES
+select * from t2;
+ERROR HY000: Table 't2' was not locked with LOCK TABLES
+select * from t3;
+j
+NULL
+unlock tables;
+insert into t1 values ();
+select * from t1;
+vc
+NULL
+1
+Three
+Four
+drop tables t1, t3;
 DROP TABLE IF EXISTS `t+1`, `t+2`;
 CREATE TABLE `t+1` (c1 INT);
 ALTER TABLE  `t+1` RENAME `t+2`;

mysql-test/r/innodb_mysql.result

@@ -796,4 +796,28 @@ lock table t2 write;
 alter table t2 modify i int default 4, rename t1;
 unlock tables;
 drop table t1;
+drop table if exists t1;
+create table t1 (i int);
+insert into t1 values ();
+lock table t1 write;
+alter table t1 modify i int default 1;
+insert into t1 values ();
+select * from t1;
+i
+NULL
+1
+alter table t1 change i c char(10) default "Two";
+insert into t1 values ();
+select * from t1;
+c
+NULL
+1
+Two
+unlock tables;
+select * from t1;
+c
+NULL
+1
+Two
+drop tables t1;
 End of 5.1 tests

mysql-test/t/alter_table-big.test

-# In order to be more or less robust test for bug#25044 has to take
-# significant time (e.g. about 9 seconds on my (Dmitri's) computer)
-# so we probably want execute it only in --big-test mode.
+#
+# Tests for various concurrency-related aspects of ALTER TABLE implemetation
+#
+# This test takes rather long time so let us run it only in --big-test mode
 --source include/big_test.inc
+# We are using some debug-only features in this test
+--source include/have_debug.inc
+# Also we are using SBR to check that statements are executed
+# in proper order.
 --source include/have_binlog_format_mixed_or_statement.inc
 
 
@@ -22,27 +27,20 @@ create table t1 (n1 int, n2 int, n3 int,
                 key (n3, n1, n2));
 create table t2 (i int);
 
-# Populating 't1' table with keys disabled, so ALTER TABLE .. ENABLE KEYS
-# will run for some time
+# Starting from 5.1 we have runtime settable @@debug variable,
+# which can be used for introducing delays at certain points of
+# statement execution, so we don't need many rows in 't1' to make
+# this test repeatable.
 alter table t1 disable keys;
---disable_query_log
-insert into t1 values (RAND()*1000,RAND()*1000,RAND()*1000);
-let $1=19;
-while ($1)
-{
- eval insert into t1 select RAND()*1000,RAND()*1000,RAND()*1000 from t1;
- dec $1;
-}
---enable_query_log
+insert into t1 values (RAND()*1000, RAND()*1000, RAND()*1000);
 
 # Later we use binlog to check the order in which statements are
 # executed so let us reset it first.
 reset master;
+set session debug="+d,sleep_alter_enable_indexes";
 --send alter table t1 enable keys;
 connection addconroot;
-let $show_type= PROCESSLIST;
-let $show_pattern= '%Repair by sorting%alter table t1 enable keys%';
---source include/wait_show_pattern.inc
+--sleep 2
 # This statement should not be blocked by in-flight ALTER and therefore
 # should be executed and written to binlog before ALTER TABLE ... ENABLE KEYS
 # finishes.
@@ -51,12 +49,68 @@ insert into t2 values (1);
 insert into t1 values (1, 1, 1);
 connection default;
 --reap
+set session debug="-d,sleep_alter_enable_indexes";
 # Check that statements were executed/binlogged in correct order.
 --replace_column 2 # 5 #
-show binlog events in 'master-bin.000001' from 102;
+show binlog events in 'master-bin.000001' from 106;
 
 # Clean up
 drop tables t1, t2;
 
 
 --echo End of 5.0 tests
+
+#
+# Additional coverage for the main ALTER TABLE case
+#
+# We should be sure that table being altered is properly
+# locked during statement execution and in particular that
+# no DDL or DML statement can sneak in and get access to
+# the table when real operation has already taken place
+# but this fact has not been noted in binary log yet.
+--disable_warnings
+drop table if exists t1, t2, t3;
+--enable_warnings
+create table t1 (i int);
+# We are going to check that statements are logged in correct order
+reset master;
+set session debug="+d,sleep_alter_before_main_binlog";
+--send alter table t1 change i c char(10) default 'Test1';
+connection addconroot;
+--sleep 2
+insert into t1 values ();
+select * from t1;
+connection default;
+--reap
+--send alter table t1 change c vc varchar(100) default 'Test2';
+connection addconroot;
+--sleep 2
+rename table t1 to t2;
+connection default;
+--reap
+drop table t2;
+# And now tests for ALTER TABLE with RENAME clause. In this
+# case target table name should be properly locked as well.
+create table t1 (i int);
+--send alter table t1 change i c char(10) default 'Test3', rename to t2;
+connection addconroot;
+--sleep 2
+insert into t2 values ();
+select * from t2;
+connection default;
+--reap
+--send alter table t2 change c vc varchar(100) default 'Test2', rename to t1;
+connection addconroot;
+--sleep 2
+rename table t1 to t3;
+connection default;
+--reap
+drop table t3;
+set session debug="-d,sleep_alter_before_main_binlog";
+
+# Check that all statements were logged in correct order
+--replace_column 2 # 5 #
+show binlog events in 'master-bin.000001' from 106;
+
+
+--echo End of 5.1 tests

mysql-test/t/alter_table.test

@@ -727,7 +727,58 @@ ALTER TABLE t1 MODIFY COLUMN v VARCHAR(4);
 SELECT * FROM t1;
 DROP TABLE t1;
 
-# End of 5.0 tests
+--echo End of 5.0 tests
+
+#
+# Extended test coverage for ALTER TABLE behaviour under LOCK TABLES
+# It should be consistent across all platforms and for all engines
+# (Before 5.1 this was not true as behavior was different between 
+# Unix/Windows and transactional/non-transactional tables).
+# See also innodb_mysql.test
+#
+--disable_warnings
+drop table if exists t1, t2, t3;
+--enable_warnings
+create table t1 (i int);
+create table t3 (j int);
+insert into t1 values ();
+insert into t3 values ();
+# Table which is altered under LOCK TABLES it should stay in list of locked
+# tables and be available after alter takes place unless ALTER contains RENAME
+# clause. We should see the new definition of table, of course.
+lock table t1 write, t3 read;
+# Example of so-called 'fast' ALTER TABLE
+alter table t1 modify i int default 1;
+insert into t1 values ();
+select * from t1;
+# And now full-blown ALTER TABLE
+alter table t1 change i c char(10) default "Two";
+insert into t1 values ();
+select * from t1;
+# If table is renamed then it should be removed from the list
+# of locked tables. 'Fast' ALTER TABLE with RENAME clause:
+alter table t1 modify c char(10) default "Three", rename to t2;
+--error ER_TABLE_NOT_LOCKED
+select * from t1;
+--error ER_TABLE_NOT_LOCKED
+select * from t2;
+select * from t3;
+unlock tables;
+insert into t2 values ();
+select * from t2;
+lock table t2 write, t3 read;
+# Full ALTER TABLE with RENAME
+alter table t2 change c vc varchar(100) default "Four", rename to t1;
+--error ER_TABLE_NOT_LOCKED
+select * from t1;
+--error ER_TABLE_NOT_LOCKED
+select * from t2;
+select * from t3;
+unlock tables;
+insert into t1 values ();
+select * from t1;
+drop tables t1, t3;
+
 
 #
 # Bug#18775 - Temporary table from alter table visible to other threads

sql/mysql_priv.h

@@ -1032,8 +1032,11 @@ TABLE *table_cache_insert_placeholder(THD *thd, const char *key,
 bool lock_table_name_if_not_cached(THD *thd, const char *db,
                                    const char *table_name, TABLE **table);
 TABLE *find_locked_table(THD *thd, const char *db,const char *table_name);
+bool reopen_table(TABLE *table);
 bool reopen_tables(THD *thd,bool get_locks,bool in_refresh);
-bool close_data_tables(THD *thd,const char *db, const char *table_name);
+void close_data_files_and_morph_locks(THD *thd, const char *db,
+                                      const char *table_name);
+void close_handle_and_leave_table_as_lock(TABLE *table);
 bool wait_for_tables(THD *thd);
 bool table_is_used(TABLE *table, bool wait_for_name_lock);
 TABLE *drop_locked_tables(THD *thd,const char *db, const char *table_name);

sql/sql_base.cc

@@ -99,7 +99,6 @@ static bool open_new_frm(THD *thd, TABLE_SHARE *share, const char *alias,
                          TABLE_LIST *table_desc, MEM_ROOT *mem_root);
 static void close_old_data_files(THD *thd, TABLE *table, bool morph_locks,
                                  bool send_refresh);
-static bool reopen_table(TABLE *table);
 static bool
 has_two_write_locked_tables_with_auto_increment(TABLE_LIST *tables);
 
@@ -681,7 +680,7 @@ TABLE_SHARE *get_cached_table_share(const char *db, const char *table_name)
 */
 
 
-static void close_handle_and_leave_table_as_lock(TABLE *table)
+void close_handle_and_leave_table_as_lock(TABLE *table)
 {
   TABLE_SHARE *share, *old_share= table->s;
   char *key_buff;
@@ -2705,7 +2704,7 @@ TABLE *find_locked_table(THD *thd, const char *db,const char *table_name)
    1  error. The old table object is not changed.
 */
 
-static bool reopen_table(TABLE *table)
+bool reopen_table(TABLE *table)
 {
   TABLE tmp;
   bool error= 1;
@@ -2788,27 +2787,55 @@ static bool reopen_table(TABLE *table)
 }
 
 
-/*
-  Used with ALTER TABLE:
-  Close all instanses of table when LOCK TABLES is in used;
-  Close first all instances of table and then reopen them
+/**
+    @brief Close all instances of a table open by this thread and replace
+           them with exclusive name-locks.
+
+    @param thd        Thread context
+    @param db         Database name for the table to be closed
+    @param table_name Name of the table to be closed
+
+    @note This function assumes that if we are not under LOCK TABLES,
+          then there is only one table open and locked. This means that
+          the function probably has to be adjusted before it can be used
+          anywhere outside ALTER TABLE.
 */
 
-bool close_data_tables(THD *thd,const char *db, const char *table_name)
+void close_data_files_and_morph_locks(THD *thd, const char *db,
+                                      const char *table_name)
 {
   TABLE *table;
-  DBUG_ENTER("close_data_tables");
+  DBUG_ENTER("close_data_files_and_morph_locks");
 
+  safe_mutex_assert_owner(&LOCK_open);
+
+  if (thd->lock)
+  {
+    /*
+      If we are not under LOCK TABLES we should have only one table
+      open and locked so it makes sense to remove the lock at once.
+    */
+    mysql_unlock_tables(thd, thd->lock);
+    thd->lock= 0;
+  }
+
+  /*
+    Note that open table list may contain a name-lock placeholder
+    for target table name if we process ALTER TABLE ... RENAME.
+    So loop below makes sense even if we are not under LOCK TABLES.
+  */
   for (table=thd->open_tables; table ; table=table->next)
   {
     if (!strcmp(table->s->table_name.str, table_name) &&
 	!strcmp(table->s->db.str, db))
     {
-      mysql_lock_remove(thd, thd->locked_tables,table);
+      if (thd->locked_tables)
+        mysql_lock_remove(thd, thd->locked_tables, table);
+      table->open_placeholder= 1;
       close_handle_and_leave_table_as_lock(table);
     }
   }
-  DBUG_RETURN(0);                               // For the future
+  DBUG_VOID_RETURN;
 }
 
 

sql/sql_table.cc

@@ -5406,7 +5406,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
   HA_CREATE_INFO *create_info;
   frm_type_enum frm_type;
   uint need_copy_table= 0;
-  bool no_table_reopen= FALSE, varchar= FALSE;
+  bool varchar= FALSE;
 #ifdef WITH_PARTITION_STORAGE_ENGINE
   uint fast_alter_partition= 0;
   bool partition_changed= FALSE;
@@ -5665,6 +5665,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
       VOID(pthread_mutex_lock(&LOCK_open));
       wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN);
       VOID(pthread_mutex_unlock(&LOCK_open));
+      DBUG_EXECUTE_IF("sleep_alter_enable_indexes", my_sleep(6000000););
       error= table->file->enable_indexes(HA_KEY_SWITCH_NONUNIQ_SAVE);
       /* COND_refresh will be signaled in close_thread_tables() */
       break;
@@ -6580,9 +6581,19 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
   }
 
   /*
-    Data is copied.  Now we rename the old table to a temp name,
-    rename the new one to the old name, remove all entries about the old table
-    from the cache, free all locks, close the old table and remove it.
+    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.
   */
 
   thd->proc_info="rename result table";
@@ -6591,38 +6602,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
   if (lower_case_table_names)
     my_casedn_str(files_charset_info, old_name);
 
-#if !defined( __WIN__)
-  if (table->file->has_transactions())
-#endif
-  {
-    /*
-      Win32 and InnoDB can't drop a table that is in use, so we must
-      close the original table before doing the rename
-    */
-    close_cached_table(thd, table);
-    table=0;					// Marker that table is closed
-    no_table_reopen= TRUE;
-  }
-#if !defined( __WIN__)
-  else
-    table->file->extra(HA_EXTRA_FORCE_REOPEN);	// Don't use this file anymore
-#endif
-
-  if (new_name != table_name || new_db != db)
-  {
-    /*
-      Check that there is no table with target name. See the
-      comment describing code for 'simple' ALTER TABLE ... RENAME.
-    */
-    if (!access(new_name_buff,F_OK))
-    {
-      error=1;
-      my_error(ER_TABLE_EXISTS_ERROR, MYF(0), new_name_buff);
-      VOID(quick_rm_table(new_db_type, new_db, tmp_name, FN_IS_TMP));
-      VOID(pthread_mutex_unlock(&LOCK_open));
-      goto err;
-    }
-  }
+  wait_while_table_is_used(thd, table, HA_EXTRA_PREPARE_FOR_DELETE);
+  close_data_files_and_morph_locks(thd, db, table_name);
 
   error=0;
   save_old_db_type= old_db_type;
@@ -6667,121 +6648,64 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
 
   if (error)
   {
-    /*
-      This shouldn't happen.  We solve this the safe way by
-      closing the locked table.
-    */
-    if (table)
-    {
-      close_cached_table(thd,table);
-    }
-    VOID(pthread_mutex_unlock(&LOCK_open));
-    goto err;
+    /* This shouldn't happen. But let us play it safe. */
+    goto err_with_placeholders;
   }
+
   if (! need_copy_table)
   {
-    bool needs_unlink= FALSE;
-    if (! table)
-    {
-      if (new_name != table_name || new_db != db)
-      {
-        table_list->alias= new_name;
-        table_list->table_name= new_name;
-        table_list->table_name_length= strlen(new_name);
-        table_list->db= new_db;
-        table_list->db_length= strlen(new_db);
-      }
-      else
-      {
-        /*
-          TODO: Creation of name-lock placeholder here is a temporary
-          work-around. Long term we should change close_cached_table() call
-          which we invoke before table renaming operation in such way that
-          it will leave placeholders for table in table cache/THD::open_tables
-          list. By doing this we will be able easily reopen and relock these
-          tables later and therefore behave under LOCK TABLES in the same way
-          on all platforms.
-        */
-        char  key[MAX_DBKEY_LENGTH];
-        uint  key_length;
-        key_length= create_table_def_key(thd, key, table_list, 0);
-        if (!(name_lock= table_cache_insert_placeholder(thd, key,
-                                                        key_length)))
-        {
-          VOID(pthread_mutex_unlock(&LOCK_open));
-          goto err;
-        }
-        name_lock->next= thd->open_tables;
-        thd->open_tables= name_lock;
-      }
+    /*
+      Now we have to inform handler that new .FRM file is in place.
+      To do this we need to obtain a handler object for it.
+    */
+    TABLE *t_table;
+    if (new_name != table_name || new_db != db)
+    {
+      table_list->alias= new_name;
+      table_list->table_name= new_name;
+      table_list->table_name_length= strlen(new_name);
+      table_list->db= new_db;
+      table_list->db_length= strlen(new_db);
       table_list->table= name_lock;
       if (reopen_name_locked_table(thd, table_list, FALSE))
-      {
-        VOID(pthread_mutex_unlock(&LOCK_open));
-        goto err;
-      }
-      table= table_list->table;
-      /*
-        We can't rely on later close_cached_table() calls to close
-        this instance of the table since it was not properly locked.
-      */
-      needs_unlink= TRUE;
+        goto err_with_placeholders;
+      t_table= table_list->table;
     }
-    /* Tell the handler that a new frm file is in place. */
-    if (table->file->create_handler_files(path, NULL, CHF_INDEX_FLAG,
-                                          create_info))
+    else
     {
-      VOID(pthread_mutex_unlock(&LOCK_open));
-      goto err;
+      if (reopen_table(table))
+        goto err_with_placeholders;
+      t_table= table;
     }
-    if (needs_unlink)
+    /* Tell the handler that a new frm file is in place. */
+    if (t_table->file->create_handler_files(path, NULL, CHF_INDEX_FLAG,
+                                            create_info))
+      goto err_with_placeholders;
+    if (thd->locked_tables && new_name == table_name && new_db == db)
     {
-      unlink_open_table(thd, table, FALSE);
-      table= name_lock= 0;
+      /*
+        We are going to reopen table down on the road, so we have to restore
+        state of the TABLE object which we used for obtaining of handler
+        object to make it suitable for reopening.
+      */
+      DBUG_ASSERT(t_table == table);
+      table->open_placeholder= 1;
+      close_handle_and_leave_table_as_lock(table);
     }
   }
 
-  if (thd->lock || new_name != table_name || no_table_reopen)  // True if WIN32
-  {
-    /*
-      Not table locking or alter table with rename.
-      Free locks and remove old table
-    */
-    if (table)
-    {
-      close_cached_table(thd,table);
-    }
-    VOID(quick_rm_table(old_db_type, db, old_name, FN_IS_TMP));
-  }
-  else
+  VOID(quick_rm_table(old_db_type, db, old_name, FN_IS_TMP));
+
+  if (thd->locked_tables && new_name == table_name && new_db == db)
   {
-    /*
-      Using LOCK TABLES without rename.
-      This code is never executed on WIN32!
-      Remove old renamed table, reopen table and get new locks
-    */
-    if (table)
-    {
-      VOID(table->file->extra(HA_EXTRA_FORCE_REOPEN)); // Use new file
-      /* Mark in-use copies old */
-      remove_table_from_cache(thd,db,table_name,RTFC_NO_FLAG);
-      /* end threads waiting on lock */
-      mysql_lock_abort(thd,table, TRUE);
-    }
-    VOID(quick_rm_table(old_db_type, db, old_name, FN_IS_TMP));
-    if (close_data_tables(thd,db,table_name) ||
-	reopen_tables(thd,1,0))
-    {						// This shouldn't happen
-      if (table)
-      {
-	close_cached_table(thd,table);		// Remove lock for table
-      }
-      VOID(pthread_mutex_unlock(&LOCK_open));
-      goto err;
-    }
+    thd->in_lock_tables= 1;
+    error= reopen_tables(thd, 1, 0);
+    thd->in_lock_tables= 0;
+    if (error)
+      goto err_with_placeholders;
   }
   VOID(pthread_mutex_unlock(&LOCK_open));
-  broadcast_refresh();
+
   /*
     The ALTER TABLE is always in its own transaction.
     Commit must not be called while LOCK_open is locked. It could call
@@ -6798,6 +6722,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
   }
   thd->proc_info="end";
 
+  DBUG_EXECUTE_IF("sleep_alter_before_main_binlog", my_sleep(6000000););
+
   ha_binlog_log_query(thd, create_info->db_type, LOGCOM_ALTER_TABLE,
                       thd->query, thd->query_length,
                       db, table_name);
@@ -6815,12 +6741,13 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
       shutdown.
     */
     char path[FN_REFLEN];
+    TABLE *t_table;
     build_table_filename(path, sizeof(path), new_db, table_name, "", 0);
-    table=open_temporary_table(thd, path, new_db, tmp_name,0);
-    if (table)
+    t_table= open_temporary_table(thd, path, new_db, tmp_name, 0);
+    if (t_table)
     {
-      intern_close_table(table);
-      my_free((char*) table, MYF(0));
+      intern_close_table(t_table);
+      my_free((char*) t_table, MYF(0));
     }
     else
       sql_print_warning("Could not open table %s.%s after rename\n",
@@ -6830,9 +6757,16 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
   table_list->table=0;				// For query cache
   query_cache_invalidate3(thd, table_list, 0);
 
-  if (name_lock)
+  if (thd->locked_tables && (new_name != table_name || new_db != db))
   {
+    /*
+      If are we under LOCK TABLES and did ALTER TABLE with RENAME we need
+      to remove placeholders for the old table and for the target table
+      from the list of open tables and table cache. If we are not under
+      LOCK TABLES we can rely on close_thread_tables() doing this job.
+    */
     pthread_mutex_lock(&LOCK_open);
+    unlink_open_table(thd, table, FALSE);
     unlink_open_table(thd, name_lock, FALSE);
     pthread_mutex_unlock(&LOCK_open);
   }
@@ -6863,6 +6797,18 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
     pthread_mutex_unlock(&LOCK_open);
   }
   DBUG_RETURN(TRUE);
+
+err_with_placeholders:
+  /*
+    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.
+  */
+  unlink_open_table(thd, table, FALSE);
+  if (name_lock)
+    unlink_open_table(thd, name_lock, FALSE);
+  VOID(pthread_mutex_unlock(&LOCK_open));
+  DBUG_RETURN(TRUE);
 }
 /* mysql_alter_table */