List:Commits« Previous MessageNext Message »
From:Jon Olav Hauglid Date:January 25 2012 4:41pm
Subject:bzr push into mysql-trunk-wl5534 branch (jon.hauglid:3466 to 3467) WL#5534
View as plain text  
 3467 Jon Olav Hauglid	2012-01-25
      WL#5534 Online ALTER, Phase 1.
      
      Patch #75: Review changes.

    modified:
      mysql-test/r/alter_table.result
      mysql-test/r/innodb_mysql_sync.result
      mysql-test/r/mdl_sync.result
      mysql-test/r/truncate_coverage.result
      mysql-test/t/alter_table.test
      mysql-test/t/innodb_mysql_sync.test
      mysql-test/t/mdl_sync.test
      mysql-test/t/truncate_coverage.test
      sql/ha_partition.cc
      sql/ha_partition.h
      sql/handler.cc
      sql/handler.h
      sql/mdl.cc
      sql/sql_alter.cc
      sql/sql_insert.cc
      sql/sql_table.cc
      sql/sql_table.h
      sql/sql_yacc.yy
 3466 Dmitry Lenev	2012-01-25
      WL#5534 Online ALTER, Phase 1.
      
      Patch #74: Updated outdated comment.

    modified:
      sql/mdl.cc
=== modified file 'mysql-test/r/alter_table.result'
--- a/mysql-test/r/alter_table.result	2011-12-16 15:08:54 +0000
+++ b/mysql-test/r/alter_table.result	2012-01-25 16:40:54 +0000
@@ -1488,6 +1488,8 @@ INSERT INTO m1 VALUES (1,1), (2,2);
 #
 # 1: Test ALGORITHM keyword
 #
+# --enable_info allows us to see how many rows were updated
+# by ALTER TABLE. in-place will show 0 rows, while copy > 0.
 ALTER TABLE t1 ADD INDEX i1(b);
 affected rows: 0
 info: Records: 0  Duplicates: 0  Warnings: 0
@@ -1638,6 +1640,10 @@ INSERT INTO ti1 VALUES (1,1,1), (2,2,2);
 INSERT INTO ti2 VALUES (1,1,1), (2,2,2);
 INSERT INTO tm1 VALUES (1,1,1), (2,2,2);
 INSERT INTO tm2 VALUES (1,1,1), (2,2,2);
+ALTER TABLE ti1;
+affected rows: 0
+ALTER TABLE tm1;
+affected rows: 0
 ALTER TABLE ti1 ADD COLUMN d VARCHAR(200);
 affected rows: 2
 info: Records: 2  Duplicates: 0  Warnings: 0

=== modified file 'mysql-test/r/innodb_mysql_sync.result'
--- a/mysql-test/r/innodb_mysql_sync.result	2011-12-14 18:48:50 +0000
+++ b/mysql-test/r/innodb_mysql_sync.result	2012-01-25 16:40:54 +0000
@@ -285,5 +285,48 @@ DELETE FROM t1 WHERE a= 3;
 # 3: In-place + writes allowed.
 #
 # TODO: Enable this test once WL#5526 is pushed
+#
+# 4: In-place + reads and writes blocked.
+#
+# Connection default
+SET DEBUG_SYNC= 'alter_opened_table SIGNAL opened WAIT_FOR continue1';
+SET DEBUG_SYNC= 'alter_table_inplace_after_lock_upgrade SIGNAL upgraded WAIT_FOR continue2';
+SET DEBUG_SYNC= 'alter_table_inplace_before_commit SIGNAL beforecommit WAIT_FOR continue3';
+SET DEBUG_SYNC= 'alter_table_before_main_binlog SIGNAL binlog WAIT_FOR continue4';
+# Sending:
+ALTER TABLE t1 ADD INDEX i4(b), ALGORITHM= INPLACE, LOCK= EXCLUSIVE;
+# Connection con1;
+SET DEBUG_SYNC= 'now WAIT_FOR opened';
+# At this point, neither reads nor writes should be blocked.
+SELECT * FROM t1;
+a	b
+1	1
+2	2
+INSERT INTO t1 VALUES (3,3);
+SET DEBUG_SYNC= 'now SIGNAL continue1';
+SET DEBUG_SYNC= 'now WAIT_FOR upgraded';
+# Now both reads and writes should be blocked.
+SELECT * FROM t1;
+ERROR HY000: Lock wait timeout exceeded; try restarting transaction
+INSERT INTO t1 VALUES (4,4);
+ERROR HY000: Lock wait timeout exceeded; try restarting transaction
+SET DEBUG_SYNC= 'now SIGNAL continue2';
+SET DEBUG_SYNC= 'now WAIT_FOR beforecommit';
+# Same here.
+SELECT * FROM t1;
+ERROR HY000: Lock wait timeout exceeded; try restarting transaction
+INSERT INTO t1 VALUES (5,5);
+ERROR HY000: Lock wait timeout exceeded; try restarting transaction
+SET DEBUG_SYNC= 'now SIGNAL continue3';
+SET DEBUG_SYNC= 'now WAIT_FOR binlog';
+# Same here.
+SELECT * FROM t1;
+ERROR HY000: Lock wait timeout exceeded; try restarting transaction
+INSERT INTO t1 VALUES (6,6);
+ERROR HY000: Lock wait timeout exceeded; try restarting transaction
+SET DEBUG_SYNC= 'now SIGNAL continue4';
+# Connection default
+# Reaping ALTER TABLE ...
+SET DEBUG_SYNC= 'RESET';
 DROP TABLE t1;
 SET DEBUG_SYNC= 'RESET';

=== modified file 'mysql-test/r/mdl_sync.result'
--- a/mysql-test/r/mdl_sync.result	2011-12-20 15:11:54 +0000
+++ b/mysql-test/r/mdl_sync.result	2012-01-25 16:40:54 +0000
@@ -5,7 +5,7 @@ create table t2 (i int);
 connection: default
 lock tables t2 read;
 connection: con1
-set debug_sync='mdl_upgrade_shared_lock_to_exclusive SIGNAL parked WAIT_FOR go';
+set debug_sync='mdl_upgrade_lock SIGNAL parked WAIT_FOR go';
 alter table t1 rename t3;
 connection: default
 set debug_sync= 'now WAIT_FOR parked';
@@ -496,6 +496,8 @@ rename table t1 to t2;;
 set debug_sync= 'now SIGNAL finish';
 # 
 # Switching to connection 'default'.
+# Now we have ALTER TABLE with SU->SNW and RENAME TABLE with pending
+# X-lock. In this case ALTER TABLE should be chosen as victim.
 # Reaping ALTER TABLE.
 ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
 # 
@@ -3089,7 +3091,7 @@ INSERT INTO t2 VALUES (3), (4);
 # Connection con1
 # We need EXECUTE 2 since ALTER TABLE does SU => SNW => X and we want
 # to stop at the second upgrade.
-SET DEBUG_SYNC= 'mdl_upgrade_shared_lock_to_exclusive SIGNAL upgrade WAIT_FOR continue EXECUTE 2';
+SET DEBUG_SYNC= 'mdl_upgrade_lock SIGNAL upgrade WAIT_FOR continue EXECUTE 2';
 # Sending:
 ALTER TABLE m1 engine=MERGE UNION=(t2, t1);
 # Connection con2

=== modified file 'mysql-test/r/truncate_coverage.result'
--- a/mysql-test/r/truncate_coverage.result	2010-11-18 13:50:08 +0000
+++ b/mysql-test/r/truncate_coverage.result	2012-01-25 16:40:54 +0000
@@ -11,7 +11,7 @@ HANDLER t1 OPEN;
 #
 # connection default
 LOCK TABLE t1 WRITE;
-SET DEBUG_SYNC='mdl_upgrade_shared_lock_to_exclusive SIGNAL waiting';
+SET DEBUG_SYNC='mdl_upgrade_lock SIGNAL waiting';
 TRUNCATE TABLE t1;
 #
 # connection con2
@@ -37,7 +37,7 @@ HANDLER t1 OPEN;
 #
 # connection default
 LOCK TABLE t1 WRITE;
-SET DEBUG_SYNC='mdl_upgrade_shared_lock_to_exclusive SIGNAL waiting';
+SET DEBUG_SYNC='mdl_upgrade_lock SIGNAL waiting';
 TRUNCATE TABLE t1;
 #
 # connection con2

=== modified file 'mysql-test/t/alter_table.test'
--- a/mysql-test/t/alter_table.test	2011-12-16 15:08:54 +0000
+++ b/mysql-test/t/alter_table.test	2012-01-25 16:40:54 +0000
@@ -1276,6 +1276,9 @@ INSERT INTO m1 VALUES (1,1), (2,2);
 --echo # 1: Test ALGORITHM keyword
 --echo #
 
+--echo # --enable_info allows us to see how many rows were updated
+--echo # by ALTER TABLE. in-place will show 0 rows, while copy > 0.
+
 --enable_info
 ALTER TABLE t1 ADD INDEX i1(b);
 ALTER TABLE t1 ADD INDEX i2(b), ALGORITHM= DEFAULT;
@@ -1437,6 +1440,9 @@ INSERT INTO tm1 VALUES (1,1,1), (2,2,2);
 INSERT INTO tm2 VALUES (1,1,1), (2,2,2);
 
 --enable_info
+ALTER TABLE ti1;
+ALTER TABLE tm1;
+
 ALTER TABLE ti1 ADD COLUMN d VARCHAR(200);
 ALTER TABLE tm1 ADD COLUMN d VARCHAR(200);
 ALTER TABLE ti1 ADD COLUMN e ENUM('a', 'b') FIRST;

=== modified file 'mysql-test/t/innodb_mysql_sync.test'
--- a/mysql-test/t/innodb_mysql_sync.test	2011-12-14 18:48:50 +0000
+++ b/mysql-test/t/innodb_mysql_sync.test	2012-01-25 16:40:54 +0000
@@ -476,6 +476,57 @@ DELETE FROM t1 WHERE a= 3 OR a= 4;
 --echo # TODO: Enable this test once WL#5526 is pushed
 --enable_parsing
 
+--echo #
+--echo # 4: In-place + reads and writes blocked.
+--echo #
+
+--echo # Connection default
+--connection default
+SET DEBUG_SYNC= 'alter_opened_table SIGNAL opened WAIT_FOR continue1';
+SET DEBUG_SYNC= 'alter_table_inplace_after_lock_upgrade SIGNAL upgraded WAIT_FOR continue2';
+SET DEBUG_SYNC= 'alter_table_inplace_before_commit SIGNAL beforecommit WAIT_FOR continue3';
+SET DEBUG_SYNC= 'alter_table_before_main_binlog SIGNAL binlog WAIT_FOR continue4';
+--echo # Sending:
+--send ALTER TABLE t1 ADD INDEX i4(b), ALGORITHM= INPLACE, LOCK= EXCLUSIVE
+
+--echo # Connection con1;
+--connection con1
+SET DEBUG_SYNC= 'now WAIT_FOR opened';
+--echo # At this point, neither reads nor writes should be blocked.
+SELECT * FROM t1;
+INSERT INTO t1 VALUES (3,3);
+
+SET DEBUG_SYNC= 'now SIGNAL continue1';
+SET DEBUG_SYNC= 'now WAIT_FOR upgraded';
+--echo # Now both reads and writes should be blocked.
+--error ER_LOCK_WAIT_TIMEOUT
+SELECT * FROM t1;
+--error ER_LOCK_WAIT_TIMEOUT
+INSERT INTO t1 VALUES (4,4);
+
+SET DEBUG_SYNC= 'now SIGNAL continue2';
+SET DEBUG_SYNC= 'now WAIT_FOR beforecommit';
+--echo # Same here.
+--error ER_LOCK_WAIT_TIMEOUT
+SELECT * FROM t1;
+--error ER_LOCK_WAIT_TIMEOUT
+INSERT INTO t1 VALUES (5,5);
+
+SET DEBUG_SYNC= 'now SIGNAL continue3';
+SET DEBUG_SYNC= 'now WAIT_FOR binlog';
+--echo # Same here.
+--error ER_LOCK_WAIT_TIMEOUT
+SELECT * FROM t1;
+--error ER_LOCK_WAIT_TIMEOUT
+INSERT INTO t1 VALUES (6,6);
+
+SET DEBUG_SYNC= 'now SIGNAL continue4';
+--echo # Connection default
+--connection default
+--echo # Reaping ALTER TABLE ...
+--reap
+SET DEBUG_SYNC= 'RESET';
+
 --connection default
 --disconnect con1
 DROP TABLE t1;

=== modified file 'mysql-test/t/mdl_sync.test'
--- a/mysql-test/t/mdl_sync.test	2011-12-20 15:11:54 +0000
+++ b/mysql-test/t/mdl_sync.test	2012-01-25 16:40:54 +0000
@@ -38,7 +38,7 @@ lock tables t2 read;
 
 connection con1;
 --echo connection: con1
-set debug_sync='mdl_upgrade_shared_lock_to_exclusive SIGNAL parked WAIT_FOR go';
+set debug_sync='mdl_upgrade_lock SIGNAL parked WAIT_FOR go';
 --send alter table t1 rename t3
 
 connection default;
@@ -697,6 +697,8 @@ set debug_sync= 'now SIGNAL finish';
 --echo # 
 --echo # Switching to connection 'default'.
 connection default;
+--echo # Now we have ALTER TABLE with SU->SNW and RENAME TABLE with pending
+--echo # X-lock. In this case ALTER TABLE should be chosen as victim.
 --echo # Reaping ALTER TABLE.
 --error ER_LOCK_DEADLOCK
 --reap
@@ -4799,7 +4801,7 @@ connect(con2, localhost, root);
 connection con1;
 --echo # We need EXECUTE 2 since ALTER TABLE does SU => SNW => X and we want
 --echo # to stop at the second upgrade.
-SET DEBUG_SYNC= 'mdl_upgrade_shared_lock_to_exclusive SIGNAL upgrade WAIT_FOR continue EXECUTE 2';
+SET DEBUG_SYNC= 'mdl_upgrade_lock SIGNAL upgrade WAIT_FOR continue EXECUTE 2';
 --echo # Sending:
 --send ALTER TABLE m1 engine=MERGE UNION=(t2, t1)
 

=== modified file 'mysql-test/t/truncate_coverage.test'
--- a/mysql-test/t/truncate_coverage.test	2010-10-29 14:10:53 +0000
+++ b/mysql-test/t/truncate_coverage.test	2012-01-25 16:40:54 +0000
@@ -40,7 +40,7 @@ HANDLER t1 OPEN;
 --connection default
 let $ID= `SELECT @id := CONNECTION_ID()`;
 LOCK TABLE t1 WRITE;
-SET DEBUG_SYNC='mdl_upgrade_shared_lock_to_exclusive SIGNAL waiting';
+SET DEBUG_SYNC='mdl_upgrade_lock SIGNAL waiting';
 send TRUNCATE TABLE t1;
 #
 # Get the default connection ID into a variable in an invisible statement.
@@ -92,7 +92,7 @@ HANDLER t1 OPEN;
 --echo # connection default
 --connection default
 LOCK TABLE t1 WRITE;
-SET DEBUG_SYNC='mdl_upgrade_shared_lock_to_exclusive SIGNAL waiting';
+SET DEBUG_SYNC='mdl_upgrade_lock SIGNAL waiting';
 send TRUNCATE TABLE t1;
 #
 # Remove datafile.

=== modified file 'sql/ha_partition.cc'
--- a/sql/ha_partition.cc	2012-01-16 10:32:15 +0000
+++ b/sql/ha_partition.cc	2012-01-25 16:40:54 +0000
@@ -6834,52 +6834,8 @@ bool ha_partition::get_error_message(int
 
 
 /****************************************************************************
-                MODULE handler characteristics
+                MODULE in-place ALTER
 ****************************************************************************/
-/**
-  alter_table_flags must be on handler/table level, not on hton level
-  due to the ha_partition hton does not know what the underlying hton is.
-*/
-uint ha_partition::alter_table_flags(uint flags)
-{
-  uint flags_to_return, flags_to_check;
-  DBUG_ENTER("ha_partition::alter_table_flags");
-
-  flags_to_return= ht->alter_table_flags(flags);
-  flags_to_return|= m_file[0]->alter_table_flags(flags); 
-
-  /*
-    If one partition fails we must be able to revert the change for the other,
-    already altered, partitions. So both ADD and DROP can only be supported in
-    pairs.
-  */
-  flags_to_check= HA_INPLACE_ADD_INDEX_NO_READ_WRITE;
-  flags_to_check|= HA_INPLACE_DROP_INDEX_NO_READ_WRITE;
-  if ((flags_to_return & flags_to_check) != flags_to_check)
-    flags_to_return&= ~flags_to_check;
-  flags_to_check= HA_INPLACE_ADD_UNIQUE_INDEX_NO_READ_WRITE;
-  flags_to_check|= HA_INPLACE_DROP_UNIQUE_INDEX_NO_READ_WRITE;
-  if ((flags_to_return & flags_to_check) != flags_to_check)
-    flags_to_return&= ~flags_to_check;
-  flags_to_check= HA_INPLACE_ADD_PK_INDEX_NO_READ_WRITE;
-  flags_to_check|= HA_INPLACE_DROP_PK_INDEX_NO_READ_WRITE;
-  if ((flags_to_return & flags_to_check) != flags_to_check)
-    flags_to_return&= ~flags_to_check;
-  flags_to_check= HA_INPLACE_ADD_INDEX_NO_WRITE;
-  flags_to_check|= HA_INPLACE_DROP_INDEX_NO_WRITE;
-  if ((flags_to_return & flags_to_check) != flags_to_check)
-    flags_to_return&= ~flags_to_check;
-  flags_to_check= HA_INPLACE_ADD_UNIQUE_INDEX_NO_WRITE;
-  flags_to_check|= HA_INPLACE_DROP_UNIQUE_INDEX_NO_WRITE;
-  if ((flags_to_return & flags_to_check) != flags_to_check)
-    flags_to_return&= ~flags_to_check;
-  flags_to_check= HA_INPLACE_ADD_PK_INDEX_NO_WRITE;
-  flags_to_check|= HA_INPLACE_DROP_PK_INDEX_NO_WRITE;
-  if ((flags_to_return & flags_to_check) != flags_to_check)
-    flags_to_return&= ~flags_to_check;
-  DBUG_RETURN(flags_to_return);
-}
-
 
 /**
   check if copy of data is needed in alter table.
@@ -6907,6 +6863,38 @@ bool ha_partition::check_if_incompatible
 /**
   Support of in-place alter table.
 */
+
+/**
+  Helper class for in-place alter, see handler.h
+*/
+
+class ha_partition_inplace_ctx : public inplace_alter_handler_ctx
+{
+public:
+  inplace_alter_handler_ctx **handler_ctx_array;
+  bool rollback_done;
+private:
+  uint m_tot_parts;
+
+public:
+  ha_partition_inplace_ctx(THD *thd, uint tot_parts)
+    : inplace_alter_handler_ctx(),
+      handler_ctx_array(NULL),
+      rollback_done(false),
+      m_tot_parts(tot_parts)
+  {}
+
+  ~ha_partition_inplace_ctx()
+  {
+    if (handler_ctx_array)
+    {
+      for (uint index= 0; index < m_tot_parts; index++)
+        delete handler_ctx_array[index];
+    }
+  }
+};
+
+
 enum_alter_inplace_result
 ha_partition::check_if_supported_inplace_alter(TABLE *altered_table,
                                                HA_CREATE_INFO *create_info,
@@ -6915,9 +6903,24 @@ ha_partition::check_if_supported_inplace
 {
   uint index= 0;
   enum_alter_inplace_result result= HA_ALTER_INPLACE_NO_LOCK;
+  ha_partition_inplace_ctx *part_inplace_ctx;
+  THD *thd= ha_thd();
 
   DBUG_ENTER("ha_partition::check_if_supported_inplace_alter");
 
+  part_inplace_ctx=
+    new (thd->mem_root) ha_partition_inplace_ctx(thd, m_tot_parts);
+  if (!part_inplace_ctx)
+    DBUG_RETURN(HA_ALTER_ERROR);
+
+  part_inplace_ctx->handler_ctx_array= (inplace_alter_handler_ctx **)
+    thd->alloc(sizeof(inplace_alter_handler_ctx *) * m_tot_parts);
+  if (!part_inplace_ctx->handler_ctx_array)
+    DBUG_RETURN(HA_ALTER_ERROR);
+
+  for (index= 0; index < m_tot_parts; index++)
+    part_inplace_ctx->handler_ctx_array[index]= NULL;
+
   for (index= 0; index < m_tot_parts; index++)
   {
     enum_alter_inplace_result p_result=
@@ -6925,34 +6928,19 @@ ha_partition::check_if_supported_inplace
                                                       create_info,
                                                       alter_info,
                                                       ha_alter_info);
+    part_inplace_ctx->handler_ctx_array[index]= ha_alter_info->handler_ctx;
+
     if (p_result < result)
       result= p_result;
     if (result == HA_ALTER_ERROR)
       break;
   }
+  ha_alter_info->handler_ctx= part_inplace_ctx;
 
   DBUG_RETURN(result);
 }
 
 
-/**
-  Helper class for in-place alter, see handler.h
-*/
-
-class ha_partition_inplace_ctx : public inplace_alter_handler_ctx
-{
-public:
-  inplace_alter_handler_ctx **handler_ctx_array;
-  bool rollback_done;
-  ha_partition_inplace_ctx()
-    : inplace_alter_handler_ctx(),
-      handler_ctx_array(NULL),
-      rollback_done(false)
-  {}
-  ~ha_partition_inplace_ctx() {}
-};
-
-
 bool ha_partition::prepare_inplace_alter_table(TABLE *altered_table,
                                                HA_CREATE_INFO *create_info,
                                                Alter_inplace_info *ha_alter_info)
@@ -6960,22 +6948,15 @@ bool ha_partition::prepare_inplace_alter
   uint index= 0;
   bool error= false;
   ha_partition_inplace_ctx *part_inplace_ctx;
-  THD *thd= ha_thd();
 
   DBUG_ENTER("ha_partition::prepare_inplace_alter_table");
 
-  part_inplace_ctx= new (thd->mem_root) ha_partition_inplace_ctx();
-  if (!part_inplace_ctx)
-    DBUG_RETURN(HA_ALTER_ERROR);
-
-  part_inplace_ctx->handler_ctx_array= (inplace_alter_handler_ctx **)
-    thd->alloc(sizeof(void *) * m_tot_parts);
-  if (!part_inplace_ctx->handler_ctx_array)
-    DBUG_RETURN(HA_ALTER_ERROR);
+  part_inplace_ctx=
+    static_cast<class ha_partition_inplace_ctx*>(ha_alter_info->handler_ctx);
 
   for (index= 0; index < m_tot_parts && !error; index++)
   {
-    ha_alter_info->handler_ctx= NULL;
+    ha_alter_info->handler_ctx= part_inplace_ctx->handler_ctx_array[index];
     if (m_file[index]->ha_prepare_inplace_alter_table(altered_table, create_info,
                                                       ha_alter_info))
       error= true;
@@ -7014,6 +6995,13 @@ bool ha_partition::inplace_alter_table(T
 }
 
 
+/*
+  Note that this function will try rollback failed ADD INDEX by
+  executing DROP INDEX for the indexes that were committed (if any)
+  before the error occured. This means that the underlying storage
+  engine must be able to drop index in-place with X-lock held.
+  (As X-lock will be held here if new indexes are to be committed)
+*/
 bool ha_partition::commit_inplace_alter_table(TABLE *altered_table,
                                               HA_CREATE_INFO *create_info,
                                               Alter_inplace_info *ha_alter_info,
@@ -7022,7 +7010,7 @@ bool ha_partition::commit_inplace_alter_
   uint index= 0;
   ha_partition_inplace_ctx *part_inplace_ctx;
 
-  DBUG_ENTER("ha_partition::inplace_alter_table");
+  DBUG_ENTER("ha_partition::commit_inplace_alter_table");
 
   part_inplace_ctx=
     static_cast<class ha_partition_inplace_ctx*>(ha_alter_info->handler_ctx);
@@ -7035,7 +7023,11 @@ bool ha_partition::commit_inplace_alter_
     ha_alter_info->handler_ctx= part_inplace_ctx->handler_ctx_array[index];
     if (m_file[index]->ha_commit_inplace_alter_table(altered_table, create_info,
                                                      ha_alter_info, commit))
+    {
+      part_inplace_ctx->handler_ctx_array[index]= ha_alter_info->handler_ctx;
       goto err;
+    }
+    part_inplace_ctx->handler_ctx_array[index]= ha_alter_info->handler_ctx;
     DBUG_EXECUTE_IF("ha_partition_fail_final_add_index", {
       /* Simulate failure by rollback of the second partition */
       if (m_tot_parts > 1)
@@ -7044,6 +7036,7 @@ bool ha_partition::commit_inplace_alter_
         ha_alter_info->handler_ctx= part_inplace_ctx->handler_ctx_array[index];
         m_file[index]->ha_commit_inplace_alter_table(altered_table, create_info,
                                                      ha_alter_info, false);
+        part_inplace_ctx->handler_ctx_array[index]= ha_alter_info->handler_ctx;
         goto err;
       }
     });
@@ -7053,6 +7046,7 @@ bool ha_partition::commit_inplace_alter_
   DBUG_RETURN(false);
 
 err:
+  ha_alter_info->handler_ctx= part_inplace_ctx;
   /*
     Reverting committed changes is (for now) only possible for ADD INDEX
     For other changes we will just try to rollback changes.

=== modified file 'sql/ha_partition.h'
--- a/sql/ha_partition.h	2011-12-21 09:35:29 +0000
+++ b/sql/ha_partition.h	2012-01-25 16:40:54 +0000
@@ -870,11 +870,6 @@ public:
            ~HA_DO_INDEX_COND_PUSHDOWN;
   }
 
-  /**
-    wrapper function for handlerton alter_table_flags, since
-    the ha_partition_hton cannot know all its capabilities
-  */
-  virtual uint alter_table_flags(uint flags);
   /*
      extensions of table handler files
   */

=== modified file 'sql/handler.cc'
--- a/sql/handler.cc	2012-01-20 03:54:02 +0000
+++ b/sql/handler.cc	2012-01-25 16:40:54 +0000
@@ -3667,6 +3667,28 @@ handler::ha_discard_or_import_tablespace
 }
 
 
+bool handler::ha_commit_inplace_alter_table(TABLE *altered_table,
+                                            HA_CREATE_INFO *create_info,
+                                            Alter_inplace_info *ha_alter_info,
+                                            bool commit)
+{
+   /*
+     At this point we should have an exclusive metadata lock on the table.
+     The exception is if we're about to roll back changes (commit= false).
+     In this case, we might be rolling back after a failed lock upgrade,
+     so we could be holding the same lock level as for inplace_alter_table().
+   */
+   DBUG_ASSERT(ha_thd()->mdl_context.is_lock_owner(MDL_key::TABLE,
+                                                   table->s->db.str,
+                                                   table->s->table_name.str,
+                                                   MDL_EXCLUSIVE) ||
+               !commit);
+
+   return commit_inplace_alter_table(altered_table, create_info,
+                                     ha_alter_info, commit);
+}
+
+
 /**
    @brief Check if both DROP and CREATE are present for an index in ALTER TABLE
 
@@ -3675,16 +3697,13 @@ handler::ha_discard_or_import_tablespace
    in-place ALTER TABLE.
 
    @param ha_alter_info     Structure holding data used during in-place alter.
-   @param table             The table being altered
 
    @return presence of index being altered
    @retval FALSE  No such index
    @retval TRUE   Have at least 1 index modified
 */
 
-static bool
-is_index_maintenance_unique(const Alter_inplace_info *ha_alter_info,
-                            const TABLE *table)
+static bool is_index_maintenance_unique(const Alter_inplace_info *ha_alter_info)
 {
   const KEY *add_key;
   const KEY *drop_key;
@@ -3708,8 +3727,8 @@ is_index_maintenance_unique(const Alter_
 
 
 /**
-   Check if *_NO_WRITE or *_NO_READ_WRITE flags are set and update
-   enum_alter_inplace_result accordingly.
+   Check if engine supports in-place operation for this type of index and
+   update enum_alter_inplace_result accordingly.
 
    @param[in]      handler_alter_flags  Flags specifying handler support for
                                         in-place alter.
@@ -3781,7 +3800,7 @@ handler::check_if_supported_inplace_alte
     Bug #49838 DROP INDEX and ADD UNIQUE INDEX for same index may
     corrupt definition at engine
   */
-  if (is_index_maintenance_unique(ha_alter_info, table))
+  if (is_index_maintenance_unique(ha_alter_info))
     DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED);
 
   /*
@@ -4057,7 +4076,8 @@ bool handler::commit_inplace_alter_table
     Alter_inplace_info::DROP_UNIQUE_INDEX |
     Alter_inplace_info::DROP_PK_INDEX;
 
-  if (ha_alter_info->handler_flags & dropping)
+  // Do not call final_drop_index() if commit= false.
+  if (commit && (ha_alter_info->handler_flags & dropping))
   {
     if ((error= final_drop_index()))
     {
@@ -4077,6 +4097,12 @@ bool handler::commit_inplace_alter_table
     }
   }
 
+  /*
+    For safety. With old API this object was deleted by the handler.
+    With new API the deletion is done by the SQL layer.
+  */
+  ha_alter_info->handler_ctx= NULL;
+
   DBUG_RETURN(false);
 }
 

=== modified file 'sql/handler.h'
--- a/sql/handler.h	2012-01-20 03:54:02 +0000
+++ b/sql/handler.h	2012-01-25 16:40:54 +0000
@@ -201,8 +201,11 @@ enum enum_alter_inplace_result {
 
 
 
-/*
+/**
   bits in alter_table_flags:
+
+  @deprecated with WL#5534: Online ALTER TABLE.
+  @see check_if_supported_inplace_alter()
 */
 /*
   These bits are set if different kinds of indexes can be created or dropped
@@ -911,9 +914,18 @@ typedef struct st_ha_create_information
 } HA_CREATE_INFO;
 
 /**
-  In-place alter handler context. Created and destroyed by the handler.
-  And finally freed at the end of the statement.
-  (Sql_alloc does not free in delete).
+  In-place alter handler context.
+
+  This is a superclass indented to be subclassed by individual handler
+  in order to store handler unique context between in-place alter API calls.
+
+  The handler is responsible for creating the object. This can be done
+  as early as during check_if_supported_inplace_alter().
+
+  The SQL layer is responsible for destroying the object.
+  The class extends Sql_alloc so the memory will be mem root allocated.
+
+  @see Alter_inplace_info
 */
 
 class inplace_alter_handler_ctx : public Sql_alloc
@@ -1023,16 +1035,46 @@ public:
 
     KEYs are sorted - see sort_keys().
   */
+
+  /** Array of KEYs for new version of table - including KEYs to be added. */
   KEY  *key_info_buffer;
+
+  /** Size of key_info_buffer array. */
   uint key_count;
+
+  /** Size of index_drop_buffer array. */
   uint index_drop_count;
+
+  /**
+     Array of pointers to KEYs to be dropped belonging to the TABLE instance
+     for the old version of the table.
+  */
   KEY  **index_drop_buffer;
+
+  /** Size of index_add_buffer array. */
   uint index_add_count;
-  // Indexes into key_info_buffer, sorted in increasing order.
+
+  /**
+     Array of indexes into key_info_buffer for KEYs to be added,
+     sorted in increasing order.
+  */
   uint *index_add_buffer;
+
+  /**
+     Context information to allow handlers to keep context between in-place
+     alter API calls.
+
+     @see inplace_alter_handler_ctx for information about object lifecycle.
+  */
   inplace_alter_handler_ctx *handler_ctx;
+
+  /**
+     Flags describing in detail which operations the storage engine is to execute.
+  */
   HA_ALTER_FLAGS handler_flags;
-  const bool ignore;                      // ALTER TABLE ... IGNORE
+
+  /** true for ALTER IGNORE TABLE ... */
+  const bool ignore;
 
   Alter_inplace_info(bool ignore_arg)
     :key_info_buffer(NULL),
@@ -1045,6 +1087,11 @@ public:
     handler_flags(0),
     ignore(ignore_arg)
   {}
+
+  ~Alter_inplace_info()
+  {
+    delete handler_ctx;
+  }
 };
 
 
@@ -2369,8 +2416,15 @@ public:
  }
 
  /**
-   Part of old, deprecated in-place ALTER API.
+   These two functions are part of old, deprecated in-place ALTER API.
  */
+ virtual uint alter_table_flags(uint flags)
+ {
+   if (ht->alter_table_flags)
+     return ht->alter_table_flags(flags);
+   return 0;
+ }
+
  virtual bool check_if_incompatible_data(HA_CREATE_INFO *create_info,
 					 uint table_changes)
  { return COMPATIBLE_DATA_NO; }
@@ -2405,6 +2459,8 @@ public:
       passed to it as well as on its own checks. If the in-place algorithm
       can be used for this ALTER TABLE, the level of required concurrency for
       its execution is also returned.
+      If any errors occur during the handler call, ALTER TABLE is aborted
+      and no further handler functions are called.
    *) Locking requirements of the in-place algorithm are compared to any
       concurrency requirements specified by user. If there is a conflict
       between them, we either switch to the copy algorithm or emit an error.
@@ -2419,20 +2475,22 @@ public:
       and requested by the user. This lock is held for most of the
       duration of in-place ALTER (if HA_ALTER_INPLACE_NO_LOCK_AFTER_PREPARE
       was returned we acquire a write lock for duration of the next step only).
-   *) After that we call handler::prepare_inplace_alter_table() to give the
+   *) After that we call handler::ha_prepare_inplace_alter_table() to give the
       storage engine a chance to update its internal structures with a higher
       lock level than the one that will be used for the main step of algorithm.
       After that we downgrade the lock if it is necessary.
    *) After that, the main step of this phase and algorithm is executed.
-      We call the handler::inplace_alter_table() method, which carries out the
+      We call the handler::ha_inplace_alter_table() method, which carries out the
       changes requested by ALTER TABLE but does not makes them visible to other
       connections yet.
    *) We ensure that no other connection uses the table by upgrading our
       lock on it to exclusive.
-   *) a) If the previous step succeeds, handler::commit_inplace_alter_table() is
+   *) a) If the previous step succeeds, handler::ha_commit_inplace_alter_table() is
          called to allow the storage engine to do any final updates to its structures,
          to make all earlier changes durable and visible to other connections.
-      b) If we have failed to upgrade lock, we call commit_inplace_alter_table()
+      b) If we have failed to upgrade lock or any errors have occured during the
+         handler functions calls (including commit), we call
+         handler::ha_commit_inplace_alter_table()
          to rollback all changes which were done during previous steps.
 
   Phase 3 : Final
@@ -2443,7 +2501,8 @@ public:
   *) Update SQL-layer data-dictionary by installing .FRM file for the new version
      of the table.
   *) Inform the storage engine about this change by calling the
-     handler::notify_table_changed() method.
+     handler::ha_notify_table_changed() method.
+  *) Destroy the Alter_inplace_info and handler_ctx objects.
 
  */
 
@@ -2483,12 +2542,19 @@ public:
                                   Alter_inplace_info *ha_alter_info);
 
 
- /* Public functions wrapping the actual handler calls. */
+ /**
+    Public functions wrapping the actual handler call.
+    @see prepare_inplace_alter_table()
+ */
  bool ha_prepare_inplace_alter_table(TABLE *altered_table,
                                      HA_CREATE_INFO *create_info,
                                      Alter_inplace_info *ha_alter_info);
 
 
+ /**
+    Public function wrapping the actual handler call.
+    @see inplace_alter_table()
+ */
  bool ha_inplace_alter_table(TABLE *altered_table,
                              HA_CREATE_INFO *create_info,
                              Alter_inplace_info *ha_alter_info)
@@ -2497,16 +2563,21 @@ public:
  }
 
 
+ /**
+    Public function wrapping the actual handler call.
+    Allows us to enforce asserts regardless of handler implementation.
+    @see commit_inplace_alter_table()
+ */
  bool ha_commit_inplace_alter_table(TABLE *altered_table,
                                     HA_CREATE_INFO *create_info,
                                     Alter_inplace_info *ha_alter_info,
-                                    bool commit)
- {
-   return commit_inplace_alter_table(altered_table, create_info,
-                                     ha_alter_info, commit);
- }
+                                    bool commit);
 
 
+ /**
+    Public function wrapping the actual handler call.
+    @see notify_table_changed()
+ */
  void ha_notify_table_changed()
  {
    notify_table_changed();
@@ -2521,7 +2592,17 @@ protected:
     writes blocked, otherwise the same level of locking as for
     inplace_alter_table() will be used.
 
-    @note Storage engines are responsible for reporting any errors.
+    @note Storage engines are responsible for reporting any errors by
+    calling my_error()/print_error()
+
+    @note If this function reports error, commit_inplace_alter_table()
+    will be called with commit= false.
+
+    @note For partitioning, failing to prepare one partition, means that
+    commit_inplace_alter_table() will be called to roll back changes for
+    all partitions. This means that commit_inplace_alter_table() might be
+    called without prepare_inplace_alter_table() having been called first
+    for a given partition.
 
     @param    altered_table     TABLE object for new version of table.
     @param    create_info       Information from the parsing phase about new
@@ -2542,7 +2623,11 @@ protected:
     and Alter_inplace_info. The level of concurrency allowed during this
     operation depends on the return value from check_if_supported_inplace_alter().
 
-    @note Storage engines are responsible for reporting any errors.
+    @note Storage engines are responsible for reporting any errors by
+    calling my_error()/print_error()
+
+    @note If this function reports error, commit_inplace_alter_table()
+    will be called with commit= false.
 
     @param    altered_table     TABLE object for new version of table.
     @param    create_info       Information from the parsing phase about new
@@ -2567,7 +2652,15 @@ protected:
     concurrent writes were blocked during prepare, but might not be during
     rollback).
 
-    @note Storage engines are responsible for reporting any errors.
+    @note Storage engines are responsible for reporting any errors by
+    calling my_error()/print_error()
+
+    @note If this function with commit= true reports error, it will be called
+    again with commit= false.
+
+    @note In case of partitioning, this function might be called for rollback
+    without prepare_inplace_alter_table() having been called first.
+    @see prepare_inplace_alter_table().
 
     @param    altered_table     TABLE object for new version of table.
     @param    create_info       Information from the parsing phase about new
@@ -2602,12 +2695,6 @@ public:
     but we don't have a primary key
   */
   virtual void use_hidden_primary_key();
-  virtual uint alter_table_flags(uint flags)
-  {
-    if (ht->alter_table_flags)
-      return ht->alter_table_flags(flags);
-    return 0;
-  }
 
 protected:
   /* Service methods for use by storage engines. */

=== modified file 'sql/mdl.cc'
--- a/sql/mdl.cc	2012-01-25 12:55:32 +0000
+++ b/sql/mdl.cc	2012-01-25 16:40:54 +0000
@@ -2186,8 +2186,8 @@ MDL_context::upgrade_shared_lock(MDL_tic
   MDL_savepoint mdl_svp= mdl_savepoint();
   bool is_new_ticket;
 
-  DBUG_ENTER("MDL_ticket::upgrade_shared_lock");
-  DEBUG_SYNC(get_thd(), "mdl_upgrade_shared_lock_to_exclusive");
+  DBUG_ENTER("MDL_context::upgrade_shared_lock");
+  DEBUG_SYNC(get_thd(), "mdl_upgrade_lock");
 
   /*
     Do nothing if already upgraded. Used when we FLUSH TABLE under
@@ -2200,9 +2200,6 @@ MDL_context::upgrade_shared_lock(MDL_tic
   DBUG_ASSERT(mdl_ticket->m_type == MDL_SHARED_UPGRADABLE ||
               mdl_ticket->m_type == MDL_SHARED_NO_WRITE ||
               mdl_ticket->m_type == MDL_SHARED_NO_READ_WRITE);
-  /* Only allow upgrade to SHARED_NO_WRITE/EXCLUSIVE */
-  DBUG_ASSERT(new_type == MDL_SHARED_NO_WRITE ||
-              new_type == MDL_EXCLUSIVE);
 
   mdl_xlock_request.init(&mdl_ticket->m_lock->key, new_type,
                          MDL_TRANSACTION);

=== modified file 'sql/sql_alter.cc'
--- a/sql/sql_alter.cc	2011-12-20 12:49:28 +0000
+++ b/sql/sql_alter.cc	2012-01-25 16:40:54 +0000
@@ -315,23 +315,26 @@ bool Sql_cmd_discard_import_tablespace::
     it is the case.
     TODO: this design is obsolete and will be removed.
   */
-  if (table_list && table_list->db && table_list->table_name)
-  {
-    int table_kind= check_if_log_table(table_list->db_length, table_list->db,
-                                       table_list->table_name_length,
-                                       table_list->table_name, false);
+  int table_kind= check_if_log_table(table_list->db_length, table_list->db,
+                                     table_list->table_name_length,
+                                     table_list->table_name, false);
 
-    if (table_kind)
+  if (table_kind)
+  {
+    /* Disable alter of enabled log tables */
+    if (logger.is_log_table_enabled(table_kind))
     {
-      /* Disable alter of enabled log tables */
-      if (logger.is_log_table_enabled(table_kind))
-      {
-        my_error(ER_BAD_LOG_STATEMENT, MYF(0), "ALTER");
-        return true;
-      }
+      my_error(ER_BAD_LOG_STATEMENT, MYF(0), "ALTER");
+      return true;
     }
   }
 
+  /*
+    Add current database to the list of accessed databases
+    for this statement. Needed for MTS.
+  */
+  thd->add_to_binlog_accessed_dbs(table_list->db);
+
   return
     mysql_discard_or_import_tablespace(thd, table_list,
                                        m_tablespace_op == DISCARD_TABLESPACE);

=== modified file 'sql/sql_insert.cc'
--- a/sql/sql_insert.cc	2011-12-16 16:40:15 +0000
+++ b/sql/sql_insert.cc	2012-01-25 16:40:54 +0000
@@ -3793,8 +3793,8 @@ static TABLE *create_table_from_items(TH
     if (!mysql_create_table_no_lock(thd, create_table->db,
                                     create_table->table_name,
                                     create_info, alter_info, 0,
-                                    select_field_count, NULL,
-                                    false))
+                                    select_field_count, false,
+                                    NULL))
     {
       DEBUG_SYNC(thd,"create_table_select_before_open");
 

=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2012-01-24 14:42:05 +0000
+++ b/sql/sql_table.cc	2012-01-25 16:40:54 +0000
@@ -2590,7 +2590,8 @@ end:
   @param base        The handlerton handle.
   @param db          The database name.
   @param table_name  The table name.
-  @param flags       flags for build_table_filename().
+  @param flags       Flags for build_table_filename() as well as describing
+                     if handler files / .FRM should be deleted as well.
 
   @return False in case of success, True otherwise.
 */
@@ -2610,6 +2611,8 @@ bool quick_rm_table(THD *thd, handlerton
   if (flags & NO_HA_TABLE)
   {
     handler *file= get_new_handler((TABLE_SHARE*) 0, thd->mem_root, base);
+    if (!file)
+      DBUG_RETURN(true);
     (void) file->ha_create_handler_files(path, NULL, CHF_DELETE_FLAG, NULL);
     delete file;
   }
@@ -4059,11 +4062,11 @@ void sp_prepare_create_field(THD *thd, C
   @param select_field_count  Number of fields coming from SELECT part of
                              CREATE TABLE ... SELECT statement. Must be zero
                              for standard create of table.
-  @param is_trans            Identifies the type of engine where the table
-                             was created: either trans or non-trans.
   @param no_ha_table         Indicates that only .FRM file (and PAR file if table
                              is partitioned) needs to be created and not a table
                              in the storage engine.
+  @param[out] is_trans       Identifies the type of engine where the table
+                             was created: either trans or non-trans.
 
   If one creates a temporary table, this is automatically opened
 
@@ -4082,8 +4085,8 @@ bool mysql_create_table_no_lock(THD *thd
                                 Alter_info *alter_info,
                                 bool internal_tmp_table,
                                 uint select_field_count,
-                                bool *is_trans,
-                                bool no_ha_table)
+                                bool no_ha_table,
+                                bool *is_trans)
 {
   char		path[FN_REFLEN + 1];
   uint          path_length;
@@ -4558,7 +4561,7 @@ bool mysql_create_table(THD *thd, TABLE_
 
   result= mysql_create_table_no_lock(thd, create_table->db,
                                      create_table->table_name, create_info,
-                                     alter_info, false, 0, &is_trans, false);
+                                     alter_info, false, 0, false, &is_trans);
   /*
     Don't write statement if:
     - Table creation has failed
@@ -4823,7 +4826,7 @@ bool mysql_create_like_table(THD* thd, T
 
   if ((res= mysql_create_table_no_lock(thd, table->db, table->table_name,
                                        &local_create_info, &local_alter_info,
-                                       false, 0, &is_trans, false)))
+                                       false, 0, false, &is_trans)))
     goto err;
 
   /*
@@ -5052,7 +5055,7 @@ static Create_field *get_field_by_index(
 
 static int compare_uint(const uint *s, const uint *t)
 {
-  return ((*s < *t) ? -1 : *s > *t ? 1 : 0);
+  return (*s < *t) ? -1 : ((*s > *t) ? 1 : 0);
 }
 
 
@@ -5071,9 +5074,9 @@ static int compare_uint(const uint *s, c
    table, which includes all corresponding parts that the new
    table has in arguments create_list, key_list and create_info.
 
-   By comparing the changes between the original and new table
-   we can determine how much it has changed after ALTER TABLE
-   and whether storage engine can carry out this ALTER in-place.
+   Compare the changes between the original and new table definitions.
+   The result of this comparison is then passed to SE which determines
+   whether it can carry out these changes in-place.
 
    Mark any changes detected in the ha_alter_flags.
    We generally try to specify handler flags only if there are real
@@ -5498,12 +5501,13 @@ bool mysql_compare_tables(TABLE *table,
                           HA_CREATE_INFO *create_info,
                           bool *metadata_equal)
 {
+  DBUG_ENTER("mysql_compare_tables");
+
   uint changes= IS_EQUAL_NO;
   uint key_count;
   List_iterator_fast<Create_field> tmp_new_field_it;
   THD *thd= table->in_use;
   *metadata_equal= false;
-  DBUG_ENTER("mysql_compare_tables");
 
   /*
     Create a copy of alter_info.
@@ -5904,6 +5908,7 @@ static bool mysql_inplace_alter_table(TH
   }
 
   close_all_tables_for_name(thd, table->s, alter_ctx->is_table_renamed());
+  table_list->table= table= NULL;
   close_temporary_table(thd, altered_table, true, false);
 
   /*
@@ -5921,7 +5926,6 @@ static bool mysql_inplace_alter_table(TH
     DBUG_RETURN(true);
   }
 
-  table_list->table= table= NULL;
   table_list->mdl_request.ticket= mdl_ticket;
   if (open_table(thd, table_list, thd->mem_root, &ot_ctx))
     DBUG_RETURN(true);
@@ -6473,11 +6477,11 @@ err:
   @param alter_ctx      ALTER TABLE runtime context.
 
   @return Operation status
-    @retval 0               Success
-    @retval != 0            Failure
+    @retval false           Success
+    @retval true            Failure
 */
 
-static int
+static bool
 simple_rename_or_index_change(THD *thd, TABLE_LIST *table_list,
                               Alter_info::enum_enable_or_disable keys_onoff,
                               Alter_table_ctx *alter_ctx)
@@ -6490,11 +6494,11 @@ simple_rename_or_index_change(THD *thd,
   if (keys_onoff != Alter_info::LEAVE_AS_IS)
   {
     if (wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN))
-      DBUG_RETURN(-1);
+      DBUG_RETURN(true);
 
     // It's now safe to take the table level lock.
     if (lock_tables(thd, table_list, alter_ctx->tables_opened, 0))
-      DBUG_RETURN(-1);
+      DBUG_RETURN(true);
 
     if (keys_onoff == Alter_info::ENABLE)
     {
@@ -6532,7 +6536,7 @@ simple_rename_or_index_change(THD *thd,
       without additional clean-up.
     */
     if (wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN))
-      DBUG_RETURN(-1);
+      DBUG_RETURN(true);
     close_all_tables_for_name(thd, table->s, true);
 
     if (mysql_rename_table(old_db_type, alter_ctx->db, alter_ctx->table_name,
@@ -6574,7 +6578,7 @@ simple_rename_or_index_change(THD *thd,
     else
       mdl_ticket->downgrade_lock(MDL_SHARED_NO_READ_WRITE);
   }
-  DBUG_RETURN(error);
+  DBUG_RETURN(error != 0);
 }
 
 
@@ -6608,12 +6612,12 @@ simple_rename_or_index_change(THD *thd,
   See how `create_list` is used to generate the new FRM regarding the
   structure of the fields. The same is done for the indices of the table.
 
-  Important is the fact, that this function tries to do as little work as
-  possible, by finding out whether a intermediate table is needed to copy
-  data into and when finishing the altering to use it as the original table.
-  For this reason the function prepare_inplace_alter_table() is called,
-  which decides based on all kind of data how similar are the new and the
-  original tables.
+  Altering a table can be done in two ways. The table can be modified
+  directly using an in-place algorithm, or the changes can be done using
+  an intermediate temporary table (copy). In-place is the preferred
+  algorithm as it avoids copying table data. The storage engine
+  selects which algorithm to use in check_if_supported_inplace_alter()
+  based on information about the table changes from fill_alter_inplace_info().
 */
 
 bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
@@ -6630,38 +6634,35 @@ bool mysql_alter_table(THD *thd,char *ne
     it is the case.
     TODO: this design is obsolete and will be removed.
   */
-  if (table_list && table_list->db && table_list->table_name)
-  {
-    int table_kind= check_if_log_table(table_list->db_length, table_list->db,
-                                       table_list->table_name_length,
-                                       table_list->table_name, false);
+  int table_kind= check_if_log_table(table_list->db_length, table_list->db,
+                                     table_list->table_name_length,
+                                     table_list->table_name, false);
 
-    if (table_kind)
+  if (table_kind)
+  {
+    /* Disable alter of enabled log tables */
+    if (logger.is_log_table_enabled(table_kind))
     {
-      /* Disable alter of enabled log tables */
-      if (logger.is_log_table_enabled(table_kind))
-      {
-        my_error(ER_BAD_LOG_STATEMENT, MYF(0), "ALTER");
-        DBUG_RETURN(true);
-      }
+      my_error(ER_BAD_LOG_STATEMENT, MYF(0), "ALTER");
+      DBUG_RETURN(true);
+    }
 
-      /* Disable alter of log tables to unsupported engine */
-      if ((create_info->used_fields & HA_CREATE_USED_ENGINE) &&
-          (!create_info->db_type || /* unknown engine */
-           !(create_info->db_type->flags & HTON_SUPPORT_LOG_TABLES)))
-      {
-        my_error(ER_UNSUPORTED_LOG_ENGINE, MYF(0));
-        DBUG_RETURN(true);
-      }
+    /* Disable alter of log tables to unsupported engine */
+    if ((create_info->used_fields & HA_CREATE_USED_ENGINE) &&
+        (!create_info->db_type || /* unknown engine */
+         !(create_info->db_type->flags & HTON_SUPPORT_LOG_TABLES)))
+    {
+      my_error(ER_UNSUPORTED_LOG_ENGINE, MYF(0));
+      DBUG_RETURN(true);
+    }
 
 #ifdef WITH_PARTITION_STORAGE_ENGINE
-      if (alter_info->flags & Alter_info::ALTER_PARTITION)
-      {
-        my_error(ER_WRONG_USAGE, MYF(0), "PARTITION", "log table");
-        DBUG_RETURN(true);
-      }
-#endif
+    if (alter_info->flags & Alter_info::ALTER_PARTITION)
+    {
+      my_error(ER_WRONG_USAGE, MYF(0), "PARTITION", "log table");
+      DBUG_RETURN(true);
     }
+#endif
   }
 
   THD_STAGE_INFO(thd, stage_init);
@@ -6889,9 +6890,23 @@ bool mysql_alter_table(THD *thd,char *ne
         alter_info->requested_algorithm !=
         Alter_info::ALTER_TABLE_ALGORITHM_DEFAULT)
     {
+      close_temporary(table_for_fast_alter_partition, true, false);
       my_error(ER_NOT_SUPPORTED_YET, MYF(0), thd->query());
       DBUG_RETURN(true);
     }
+
+    /*
+      Upgrade from MDL_SHARED_UPGRADABLE to MDL_SHARED_NO_WRITE.
+      Afterwards it's safe to take the table level lock.
+    */
+    if (thd->mdl_context.upgrade_shared_lock(mdl_ticket, MDL_SHARED_NO_WRITE,
+                                             thd->variables.lock_wait_timeout)
+        || lock_tables(thd, table_list, alter_ctx.tables_opened, 0))
+    {
+      close_temporary(table_for_fast_alter_partition, true, false);
+      DBUG_RETURN(true);
+    }
+
     // In-place execution of ALTER TABLE for partitioning.
     DBUG_RETURN(fast_alter_partition_table(thd, table, alter_info,
                                            create_info, table_list,
@@ -6925,8 +6940,18 @@ bool mysql_alter_table(THD *thd,char *ne
     alter_info->requested_algorithm= Alter_info::ALTER_TABLE_ALGORITHM_COPY;
   }
 
-  handlerton *old_db_type= table->s->db_type();
+  /*
+    If the old table had partitions and we are doing ALTER TABLE ...
+    engine= <new_engine>, the new table must preserve the original
+    partitioning. This means that the new engine is still the
+    partitioning engine, not the engine specified in the parser.
+    This is discovered in prep_alter_part_table, which in such case
+    updates create_info->db_type.
+    It's therefore important that the assignment below is done
+    after prep_alter_part_table.
+  */
   handlerton *new_db_type= create_info->db_type;
+  handlerton *old_db_type= table->s->db_type();
   TABLE *new_table= NULL;
   ha_rows copied=0,deleted=0;
 
@@ -7011,7 +7036,7 @@ bool mysql_alter_table(THD *thd,char *ne
   tmp_disable_binlog(thd);
   error= mysql_create_table_no_lock(thd, alter_ctx.new_db, alter_ctx.tmp_name,
                                     create_info, alter_info,
-                                    true, 0, NULL, true);
+                                    true, 0, true, NULL);
   reenable_binlog(thd);
 
   if (error)
@@ -7042,9 +7067,14 @@ bool mysql_alter_table(THD *thd,char *ne
       goto err_new_table_cleanup;
 
     enum_alter_inplace_result inplace_supported;
-    if (ha_alter_info.handler_flags == 0) // This shouldn't really happen...
+    if (alter_info->flags == 0)
+      // No-op ALTER, no need to call handler API functions.
+      goto end_inplace;
+    else if (ha_alter_info.handler_flags == 0)
+      // Unrecognized ALTER command, do copy for safety
       inplace_supported= HA_ALTER_INPLACE_NOT_SUPPORTED;
     else
+      // Ask storage engine whether to use copy or in-place
       inplace_supported=
         table->file->check_if_supported_inplace_alter(altered_table,
                                                       create_info,
@@ -7723,7 +7753,8 @@ copy_data_between_tables(TABLE *from,TAB
                  (to->key_info[0].key_part[0].field->flags &
                   AUTO_INCREMENT_FLAG))
                err_msg= ER(ER_DUP_ENTRY_AUTOINCREMENT_CASE);
-             to->file->print_keydup_error(&to->key_info[key_nr], err_msg);
+             to->file->print_keydup_error(key_nr == MAX_KEY ? NULL :
+                                          &to->key_info[key_nr], err_msg);
              break;
            }
          }

=== modified file 'sql/sql_table.h'
--- a/sql/sql_table.h	2012-01-20 03:54:02 +0000
+++ b/sql/sql_table.h	2012-01-25 16:40:54 +0000
@@ -156,8 +156,8 @@ bool mysql_create_table_no_lock(THD *thd
                                 HA_CREATE_INFO *create_info,
                                 Alter_info *alter_info,
                                 bool tmp_table, uint select_field_count,
-                                bool *is_trans,
-                                bool no_ha_table);
+                                bool no_ha_table,
+                                bool *is_trans);
 int mysql_discard_or_import_tablespace(THD *thd,
                                        TABLE_LIST *table_list,
                                        bool discard);

=== modified file 'sql/sql_yacc.yy'
--- a/sql/sql_yacc.yy	2011-12-21 08:49:47 +0000
+++ b/sql/sql_yacc.yy	2012-01-25 16:40:54 +0000
@@ -759,7 +759,7 @@ static bool add_create_index_prepare (LE
   lex->sql_command= SQLCOM_CREATE_INDEX;
   if (!lex->current_select->add_table_to_list(lex->thd, table, NULL,
                                               TL_OPTION_UPDATING,
-                                              TL_READ,
+                                              TL_READ_NO_INSERT,
                                               MDL_SHARED_UPGRADABLE))
     return TRUE;
   lex->alter_info.reset();
@@ -6549,7 +6549,7 @@ alter:
             lex->duplicates= DUP_ERROR; 
             if (!lex->select_lex.add_table_to_list(thd, $4, NULL,
                                                    TL_OPTION_UPDATING,
-                                                   TL_READ,
+                                                   TL_READ_NO_INSERT,
                                                    MDL_SHARED_UPGRADABLE))
               MYSQL_YYABORT;
             lex->col_list.empty();
@@ -7185,7 +7185,11 @@ alter_list_item:
             LEX *lex=Lex;
             lex->alter_info.flags|= Alter_info::ALTER_ORDER;
           }
-        | ALGORITHM_SYM opt_equal DEFAULT {}
+        | ALGORITHM_SYM opt_equal DEFAULT
+          {
+            Lex->alter_info.requested_algorithm=
+              Alter_info::ALTER_TABLE_ALGORITHM_DEFAULT;
+          }
         | ALGORITHM_SYM opt_equal ident
           {
             if (Lex->alter_info.set_requested_algorithm(&$3))
@@ -7194,7 +7198,11 @@ alter_list_item:
               MYSQL_YYABORT;
             }
           }
-        | LOCK_SYM opt_equal DEFAULT {}
+        | LOCK_SYM opt_equal DEFAULT
+          {
+            Lex->alter_info.requested_lock=
+              Alter_info::ALTER_TABLE_LOCK_DEFAULT;
+          }
         | LOCK_SYM opt_equal ident
           {
             if (Lex->alter_info.set_requested_lock(&$3))
@@ -10802,7 +10810,7 @@ drop:
             lex->alter_info.drop_list.push_back(ad);
             if (!lex->current_select->add_table_to_list(lex->thd, $5, NULL,
                                                         TL_OPTION_UPDATING,
-                                                        TL_READ,
+                                                        TL_READ_NO_INSERT,
                                                         MDL_SHARED_UPGRADABLE))
               MYSQL_YYABORT;
           }

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk-wl5534 branch (jon.hauglid:3466 to 3467) WL#5534Jon Olav Hauglid25 Jan