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 List-Archive: http://lists.mysql.com/commits/142542 Message-Id: <201201251641.q0PGfgax027605@acsmt356.oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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(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(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 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= , 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).