From: Dmitry Lenev Date: June 16 2011 10:03pm Subject: bzr push into mysql-5.1 branch (Dmitry.Lenev:3653 to 3654) Bug#12652385 List-Archive: http://lists.mysql.com/commits/139375 X-Bug: 12652385 Message-Id: <20110616220323.4D9F37406A5@bandersnatch> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 3654 Dmitry Lenev 2011-06-17 Fix for bug #12652385 - "61493: REORDERING COLUMNS TO POSITION FIRST CAN CAUSE DATA TO BE CORRUPTED". ALTER TABLE MODIFY/CHANGE ... FIRST did nothing except renaming columns if new version of the table had exactly the same structure as the old one (i.e. as result of such statement, names of columns changed their order as specified but data in columns didn't). The same thing happened for ALTER TABLE DROP COLUMN/ADD COLUMN statements which were supposed to produce new version of table with exactly the same structure as the old version of table. I.e. in the latter case the result was the same as if old column was renamed instead of being dropped and new column with default as value being created. Both these problems were caused by the fact that ALTER TABLE implementation incorrectly interpreted both these situations as simple renaming of columns and assumed that in-place ALTER TABLE algorithm could have been used for them. This patch fixes this problem by ensuring that in cases when some column is moved to the first position or some column is dropped the default ALTER TABLE algorithm involving table copying is always used. This is achieved by detecting such situations in mysql_prepare_alter_table() and setting Alter_info::change_level to ALTER_TABLE_DATA_CHANGED for them. @ mysql-test/r/alter_table.result Added test for bug #12652385 - "61493: REORDERING COLUMNS TO POSITION FIRST CAN CAUSE DATA TO BE CORRUPTED". @ mysql-test/t/alter_table.test Added test for bug #12652385 - "61493: REORDERING COLUMNS TO POSITION FIRST CAN CAUSE DATA TO BE CORRUPTED". @ sql/sql_table.cc Changed mysql_prepare_alter_table() to detect situations in which we some column moved to the first position or some column is dropped and ensure that such ALTER TABLE statements won't be carried out using in-place algorithm. The latter could have happened before this patch if new version of table had the same structure as the old one (except the column names). modified: mysql-test/r/alter_table.result mysql-test/t/alter_table.test sql/sql_table.cc 3653 Vasil Dimov 2011-06-16 Silence bogus compiler warning introduced in marko.makela@stripped modified: storage/innobase/btr/btr0btr.c storage/innodb_plugin/btr/btr0btr.c === modified file 'mysql-test/r/alter_table.result' --- a/mysql-test/r/alter_table.result 2009-12-18 12:00:30 +0000 +++ b/mysql-test/r/alter_table.result 2011-06-16 22:02:52 +0000 @@ -1345,4 +1345,33 @@ DROP TABLE t1; CREATE TABLE t1 (a TEXT, id INT, b INT); ALTER TABLE t1 DROP COLUMN a, ADD COLUMN c TEXT FIRST; DROP TABLE t1; +# +# Test for bug #12652385 - "61493: REORDERING COLUMNS TO POSITION +# FIRST CAN CAUSE DATA TO BE CORRUPTED". +# +drop table if exists t1; +# Use MyISAM engine as the fact that InnoDB doesn't support +# in-place ALTER TABLE in cases when columns are being renamed +# hides some bugs. +create table t1 (i int, j int) engine=myisam; +insert into t1 value (1, 2); +# First, test for original problem described in the bug report. +select * from t1; +i j +1 2 +# Change of column order by the below ALTER TABLE statement should +# affect both column names and column contents. +alter table t1 modify column j int first; +select * from t1; +j i +2 1 +# Now test for similar problem with the same root. +# The below ALTER TABLE should change not only the name but +# also the value for the last column of the table. +alter table t1 drop column i, add column k int default 0; +select * from t1; +j k +2 0 +# Clean-up. +drop table t1; End of 5.1 tests === modified file 'mysql-test/t/alter_table.test' --- a/mysql-test/t/alter_table.test 2009-12-18 12:00:30 +0000 +++ b/mysql-test/t/alter_table.test 2011-06-16 22:02:52 +0000 @@ -1073,4 +1073,31 @@ ALTER TABLE t1 DROP COLUMN a, ADD COLUMN DROP TABLE t1; +--echo # +--echo # Test for bug #12652385 - "61493: REORDERING COLUMNS TO POSITION +--echo # FIRST CAN CAUSE DATA TO BE CORRUPTED". +--echo # +--disable_warnings +drop table if exists t1; +--enable_warnings +--echo # Use MyISAM engine as the fact that InnoDB doesn't support +--echo # in-place ALTER TABLE in cases when columns are being renamed +--echo # hides some bugs. +create table t1 (i int, j int) engine=myisam; +insert into t1 value (1, 2); +--echo # First, test for original problem described in the bug report. +select * from t1; +--echo # Change of column order by the below ALTER TABLE statement should +--echo # affect both column names and column contents. +alter table t1 modify column j int first; +select * from t1; +--echo # Now test for similar problem with the same root. +--echo # The below ALTER TABLE should change not only the name but +--echo # also the value for the last column of the table. +alter table t1 drop column i, add column k int default 0; +select * from t1; +--echo # Clean-up. +drop table t1; + + --echo End of 5.1 tests === modified file 'sql/sql_table.cc' --- a/sql/sql_table.cc 2011-05-16 20:04:01 +0000 +++ b/sql/sql_table.cc 2011-06-16 22:02:52 +0000 @@ -6170,6 +6170,12 @@ mysql_prepare_alter_table(THD *thd, TABL if (drop) { drop_it.remove(); + /* + ALTER TABLE DROP COLUMN always changes table data even in cases + when new version of the table has the same structure as the old + one. + */ + alter_info->change_level= ALTER_TABLE_DATA_CHANGED; continue; } /* Check if field is changed */ @@ -6247,7 +6253,14 @@ mysql_prepare_alter_table(THD *thd, TABL if (!def->after) new_create_list.push_back(def); else if (def->after == first_keyword) + { new_create_list.push_front(def); + /* + Re-ordering columns in table can't be done using in-place algorithm + as it always changes table data. + */ + alter_info->change_level= ALTER_TABLE_DATA_CHANGED; + } else { Create_field *find; @@ -6263,6 +6276,10 @@ mysql_prepare_alter_table(THD *thd, TABL goto err; } find_it.after(def); // Put element after this + /* + Re-ordering columns in table can't be done using in-place algorithm + as it always changes table data. + */ alter_info->change_level= ALTER_TABLE_DATA_CHANGED; } } No bundle (reason: useless for push emails).