From: Jon Olav Hauglid Date: May 19 2011 12:44pm Subject: bzr push into mysql-trunk branch (jon.hauglid:3102 to 3103) Bug#11762345 List-Archive: http://lists.mysql.com/commits/137701 X-Bug: 11762345 Message-Id: <201105191244.p4JCiD10028585@acsmt358.oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 3103 Jon Olav Hauglid 2011-05-19 Bug#11762345 54927: DROPPING AND ADDING AN INDEX IN ONE COMMAND CAN FAIL IN INNODB PLUGIN 1.0 If a single ALTER TABLE statement drops and adds an index with the same name, the operation has to be done using full table copy rather than changing the table in-place. This problem was partly fixed by an index name check added in the patch for Bug#49838. However, a problem remained if the new index was not given an explicit name. In such cases the index is automatically given the same name as the first column in the index defintion. If this name happened to be the same name as the index to be dropped, the match would not be detected by the fix for Bug#49838. This caused ALTER TABLE to fail with "Incorrect index name". This patch fixes this problem by postponing the index name check until after unnamed new indexes have been given a name. This allows the check to catch situations described above and force the use of full table copy (rather than aborting with "Incorrect index name"). Test case added to innodb.innodb_mysql.test. modified: mysql-test/suite/innodb/r/innodb-index.result mysql-test/suite/innodb/r/innodb.result mysql-test/suite/innodb/r/innodb_mysql.result mysql-test/suite/innodb/t/innodb-index.test mysql-test/suite/innodb/t/innodb.test mysql-test/suite/innodb/t/innodb_mysql.test sql/sql_table.cc 3102 Jorgen Loland 2011-05-19 BUG#11765831: 'RANGE ACCESS' MAY INCORRECTLY FILTER AWAY QUALIFYING ROWS The problem was that the ranges created when OR'ing two conditions could be incorrect. Without the bugfix, "I <> 6 OR (I <> 8 AND J = 5)" would create these ranges: "NULL < I < 6", "6 <= I <= 6 AND 5 <= J <= 5", "6 < I < 8", "8 <= I <= 8 AND 5 <= J <= 5", "8 < I" While the correct ranges is "NULL < I < 6", "6 <= I <= 6 AND 5 <= J <= 5", "6 < I" The problem occurs when key_or() ORs (1) "NULL < I < 6, 6 <= I <= 6 AND 5 <= J <= 5, 6 < I" with (2) "8 < I AND 5 <= J <= 5" The reason for the bug is that in key_or(), SEL_ARG *tmp is used to point to the range in (1) above that is merged with (2) while key1 points to the root of the red-black tree of (1). When merging (1) and (2), tmp refers to the "6 < I" part whereas the root is the "6 <= ... AND 5 <= J <= 5" part. key_or() decides that the tmp range needs to be split into "6 < I < 8, 8 <= I <= 8, 8 < I", in which next_key_part of the second range should be that of tmp. However, next_key_part is set to key1->next_key_part ("5 <= J <= 5") instead of tmp->next_key_part (empty). Fixing this gives the correct but not optimal ranges: "NULL < I < 6", "6 <= I <= 6 AND 5 <= J <= 5", "6 < I < 8", "8 <= I <= 8", "8 < I" A second problem can be seen above: key_or() may create adjacent ranges that could be replaced with a single range. Fixes for this is also included in the patch so that the range above becomes correct AND optimal: "NULL < I < 6", "6 <= I <= 6 AND 5 <= J <= 5", "6 < I" Merging adjacent ranges like this gives a slightly lower cost estimate for the range access. @ mysql-test/include/range.inc Add test for BUG#11765831 @ mysql-test/r/group_min_max.result BUG#11765831 merges adjacent ranges, resulting in a slightly lower cost estimate for accessing the table through range access. @ mysql-test/r/range_icp.result Add test for BUG#11765831 @ mysql-test/r/range_icp_mrr.result Add test for BUG#11765831 @ mysql-test/r/range_mrr.result Add test for BUG#11765831 @ mysql-test/r/range_mrr_cost.result Add test for BUG#11765831 @ mysql-test/r/range_none.result Add test for BUG#11765831 @ sql/opt_range.cc In key_or(): When a range in key1 was split due to a partially overlapping range in key2, the non-overlaping part incorrectly got next_key_part from the R-B tree root node instead of the currently active range. key_or() could also create adjacent ranges that could be replaced by a single continous range. This is also fixed. modified: mysql-test/include/range.inc mysql-test/r/group_min_max.result mysql-test/r/range_icp.result mysql-test/r/range_icp_mrr.result mysql-test/r/range_mrr.result mysql-test/r/range_mrr_cost.result mysql-test/r/range_none.result sql/opt_range.cc === modified file 'mysql-test/suite/innodb/r/innodb-index.result' --- a/mysql-test/suite/innodb/r/innodb-index.result 2010-10-07 11:57:34 +0000 +++ b/mysql-test/suite/innodb/r/innodb-index.result 2011-05-19 12:43:26 +0000 @@ -434,7 +434,6 @@ t3 CREATE TABLE `t3` ( KEY `c` (`c`) ) ENGINE=InnoDB DEFAULT CHARSET=latin1 alter table t2 drop index b, add index (b); -ERROR 42000: Incorrect index name 'b' show create table t2; Table Create Table t2 CREATE TABLE `t2` ( @@ -445,8 +444,8 @@ t2 CREATE TABLE `t2` ( `e` int(11) DEFAULT NULL, PRIMARY KEY (`a`), UNIQUE KEY `dc` (`d`,`c`), - KEY `b` (`b`), KEY `c` (`c`), + KEY `b` (`b`), CONSTRAINT `t2_ibfk_1` FOREIGN KEY (`b`) REFERENCES `t1` (`b`) ON DELETE CASCADE, CONSTRAINT `t2_ibfk_2` FOREIGN KEY (`c`) REFERENCES `t3` (`c`), CONSTRAINT `t2_ibfk_3` FOREIGN KEY (`d`) REFERENCES `t4` (`d`) === modified file 'mysql-test/suite/innodb/r/innodb.result' --- a/mysql-test/suite/innodb/r/innodb.result 2011-02-21 02:57:30 +0000 +++ b/mysql-test/suite/innodb/r/innodb.result 2011-05-19 12:43:26 +0000 @@ -704,7 +704,6 @@ select count(*) from t1 where cat_code=' count(*) 0 alter table t1 drop index sca_pic, add index (sca_pic, cat_code); -ERROR 42000: Incorrect index name 'sca_pic' alter table t1 drop index sca_pic; alter table t1 add index (sca_pic, cat_code); select count(*) from t1 where sca_code='PD' and sca_pic is null; @@ -1673,7 +1672,7 @@ variable_value - @innodb_rows_deleted_or 71 SELECT variable_value - @innodb_rows_inserted_orig FROM information_schema.global_status WHERE LOWER(variable_name) = 'innodb_rows_inserted'; variable_value - @innodb_rows_inserted_orig -1066 +1069 SELECT variable_value - @innodb_rows_updated_orig FROM information_schema.global_status WHERE LOWER(variable_name) = 'innodb_rows_updated'; variable_value - @innodb_rows_updated_orig 866 === modified file 'mysql-test/suite/innodb/r/innodb_mysql.result' --- a/mysql-test/suite/innodb/r/innodb_mysql.result 2011-05-18 10:43:46 +0000 +++ b/mysql-test/suite/innodb/r/innodb_mysql.result 2011-05-19 12:43:26 +0000 @@ -2843,4 +2843,14 @@ Table Op Msg_type Msg_text test.t1 optimize note Table does not support optimize, doing recreate + analyze instead test.t1 optimize status OK DROP TABLE t1; +# +# Bug#11762345 54927: DROPPING AND ADDING AN INDEX IN ONE +# COMMAND CAN FAIL IN INNODB PLUGIN 1.0 +# +DROP TABLE IF EXISTS t1; +CREATE TABLE t1 (id int, a int, b int, PRIMARY KEY (id), +INDEX a (a)) ENGINE=innodb; +ALTER TABLE t1 DROP INDEX a, ADD INDEX a (b, a); +ALTER TABLE t1 DROP INDEX a, ADD INDEX (a, b); +DROP TABLE t1; End of 6.0 tests === modified file 'mysql-test/suite/innodb/t/innodb-index.test' --- a/mysql-test/suite/innodb/t/innodb-index.test 2010-07-16 21:00:50 +0000 +++ b/mysql-test/suite/innodb/t/innodb-index.test 2011-05-19 12:43:26 +0000 @@ -131,8 +131,6 @@ show create table t4; --error ER_CANT_CREATE_TABLE alter table t3 add constraint dc foreign key (a) references t1(a); show create table t3; -# this should be fixed by MySQL (see Bug #51451) ---error ER_WRONG_NAME_FOR_INDEX alter table t2 drop index b, add index (b); show create table t2; --error ER_ROW_IS_REFERENCED_2 === modified file 'mysql-test/suite/innodb/t/innodb.test' --- a/mysql-test/suite/innodb/t/innodb.test 2011-02-21 02:57:30 +0000 +++ b/mysql-test/suite/innodb/t/innodb.test 2011-05-19 12:43:26 +0000 @@ -452,8 +452,6 @@ alter table t1 add index sca_pic (cat_co select count(*) from t1 where sca_code='PD' and sca_pic is null; select count(*) from t1 where cat_code='E'; -# this should be fixed by MySQL (see Bug #51451) ---error ER_WRONG_NAME_FOR_INDEX alter table t1 drop index sca_pic, add index (sca_pic, cat_code); alter table t1 drop index sca_pic; alter table t1 add index (sca_pic, cat_code); === modified file 'mysql-test/suite/innodb/t/innodb_mysql.test' --- a/mysql-test/suite/innodb/t/innodb_mysql.test 2011-01-17 16:27:07 +0000 +++ b/mysql-test/suite/innodb/t/innodb_mysql.test 2011-05-19 12:43:26 +0000 @@ -1007,4 +1007,23 @@ OPTIMIZE TABLE t1; DROP TABLE t1; +--echo # +--echo # Bug#11762345 54927: DROPPING AND ADDING AN INDEX IN ONE +--echo # COMMAND CAN FAIL IN INNODB PLUGIN 1.0 +--echo # + +--disable_warnings +DROP TABLE IF EXISTS t1; +--enable_warnings + +CREATE TABLE t1 (id int, a int, b int, PRIMARY KEY (id), + INDEX a (a)) ENGINE=innodb; + +ALTER TABLE t1 DROP INDEX a, ADD INDEX a (b, a); +# This used to fail +ALTER TABLE t1 DROP INDEX a, ADD INDEX (a, b); + +DROP TABLE t1; + + --echo End of 6.0 tests === modified file 'sql/sql_table.cc' --- a/sql/sql_table.cc 2011-05-12 17:29:19 +0000 +++ b/sql/sql_table.cc 2011-05-19 12:43:26 +0000 @@ -4973,41 +4973,48 @@ err: /** @brief Check if both DROP and CREATE are present for an index in ALTER TABLE - - @details Checks if any index is being modified (present as both DROP INDEX - and ADD INDEX) in the current ALTER TABLE statement. Needed for disabling + + @details Checks if any index is being modified (present as both DROP INDEX + and ADD INDEX) in the current ALTER TABLE statement. Needed for disabling in-place ALTER TABLE. - - @param table The table being altered - @param alter_info The ALTER TABLE structure - @return presence of index being altered + + @param table The table being altered. + @param key_info_buffer Array of KEY structs for new indexes. + @param index_drop_buffer Array of offsets into table->key_info for indexes + to be dropped. + @param index_drop_count Number of indexes to be dropped. + @param index_add_buffer Array of offsets into key_info_buffer representing + new indexes. + @param index_add_count Number of indexes to add. + @return presence of index being both dropped and added @retval FALSE No such index @retval TRUE Have at least 1 index modified */ static bool -is_index_maintenance_unique (TABLE *table, Alter_info *alter_info) +is_index_maintenance_unique (const TABLE *table, const KEY *key_info_buffer, + const uint *index_drop_buffer, + uint index_drop_count, + const uint *index_add_buffer, uint index_add_count) { - List_iterator key_it(alter_info->key_list); - List_iterator drop_it(alter_info->drop_list); - Key *key; + const KEY *add_key; + const KEY *drop_key; + const uint *idx_add_p; + const uint *idx_drop_p; - while ((key= key_it++)) + for (idx_add_p= index_add_buffer; + idx_add_p < index_add_buffer + index_add_count; idx_add_p++) { - if (key->name.str) + add_key= key_info_buffer + *idx_add_p; + for (idx_drop_p= index_drop_buffer; + idx_drop_p < index_drop_buffer + index_drop_count; idx_drop_p++) { - Alter_drop *drop; - - drop_it.rewind(); - while ((drop= drop_it++)) - { - if (drop->type == Alter_drop::KEY && - !my_strcasecmp(system_charset_info, key->name.str, drop->name)) - return TRUE; - } + drop_key= table->key_info + *idx_drop_p; + if (!my_strcasecmp(system_charset_info, add_key->name, drop_key->name)) + return true; } } - return FALSE; + return false; } @@ -6298,15 +6305,9 @@ bool mysql_alter_table(THD *thd,char *ne */ new_db_type= create_info->db_type; - if (is_index_maintenance_unique (table, alter_info)) - need_copy_table= ALTER_TABLE_DATA_CHANGED; - if (mysql_prepare_alter_table(thd, table, create_info, alter_info)) goto err; - if (need_copy_table == ALTER_TABLE_METADATA_ONLY) - need_copy_table= alter_info->change_level; - set_table_default_charset(thd, create_info, db); if (thd->variables.old_alter_table @@ -6318,6 +6319,8 @@ bool mysql_alter_table(THD *thd,char *ne need_copy_table= ALTER_TABLE_DATA_CHANGED; else { + need_copy_table= alter_info->change_level; + Alter_table_change_level need_copy_table_res; /* Check how much the tables differ. */ if (mysql_compare_tables(table, alter_info, @@ -6328,16 +6331,24 @@ bool mysql_alter_table(THD *thd,char *ne &index_add_buffer, &index_add_count, &candidate_key_count, FALSE)) goto err; - + DBUG_EXECUTE_IF("alter_table_only_metadata_change", { if (need_copy_table_res != ALTER_TABLE_METADATA_ONLY) goto err; }); DBUG_EXECUTE_IF("alter_table_only_index_change", { if (need_copy_table_res != ALTER_TABLE_INDEX_CHANGED) goto err; }); - + if (need_copy_table == ALTER_TABLE_METADATA_ONLY) need_copy_table= need_copy_table_res; + + if (need_copy_table == ALTER_TABLE_INDEX_CHANGED) + { + if (is_index_maintenance_unique(table, key_info_buffer, + index_drop_buffer, index_drop_count, + index_add_buffer, index_add_count)) + need_copy_table= ALTER_TABLE_DATA_CHANGED; + } } /* No bundle (reason: useless for push emails).