List:Commits« Previous MessageNext Message »
From:marko.makela Date:June 1 2012 9:17am
Subject:bzr push into mysql-trunk-wl6255 branch (marko.makela:3923 to 3925)
Bug#14145138 WL#5545
View as plain text  
 3925 Marko Mäkelä	2012-06-01
      Bug#14145138 work-around for WL#5545 ALTER_COLUMN_NAME (rename column).
      
      When a table contains or is referenced by FOREIGN KEY constraints, and
      the table is rebuilt (ADD PRIMARY KEY or otherwise) while renaming
      columns, the columns were not renamed in the FOREIGN KEY constraints.
      This would cause an implicit DROP TABLE (Bug#14145138), because the
      dict_load_foreigns() call in row_merge_rename_tables() would fail.
      
      This fix does not address Bug#14145138, it just prevents one scenario
      leading to it.
      
      innobase_rename_column(): Add the parameter new_clustered, so that
      column names in FOREIGN KEY constraints can be renamed when rebuilding
      a table. Previously, we only renamed columns if the table was not being
      rebuilt.
      
      innobase_rename_columns(): New function, to invoke
      innobase_rename_column() on all renamed columns. Invoke this also when
      rebuilding a table (new_clustered==true).

    modified:
      mysql-test/suite/innodb/r/innodb-index.result
      mysql-test/suite/innodb/t/innodb-index.test
      storage/innobase/handler/handler0alter.cc
 3924 Marko Mäkelä	2012-06-01
      Remove dependence on TABLE_ID in innodb-alter-nullable.test.

    modified:
      mysql-test/suite/innodb/r/innodb-alter-nullable.result
      mysql-test/suite/innodb/t/innodb-alter-nullable.test
 3923 Marko Mäkelä	2012-06-01
      Enable ALTER TABLE info in a test.
      
      Add a test case that demonstrates an inconsistency in WL#6251.

    modified:
      mysql-test/suite/innodb/r/innodb-index.result
      mysql-test/suite/innodb/t/innodb-index.test
=== modified file 'mysql-test/suite/innodb/r/innodb-alter-nullable.result'
--- a/mysql-test/suite/innodb/r/innodb-alter-nullable.result	revid:marko.makela@stripped13g63r1tas679u
+++ b/mysql-test/suite/innodb/r/innodb-alter-nullable.result	revid:marko.makela@stripped091526-xbtmjnaw20exebzq
@@ -39,5 +39,5 @@ ALTER TABLE t MODIFY c2 INT NULL, ALGORI
 SELECT * FROM INFORMATION_SCHEMA.INNODB_SYS_TABLES
 WHERE NAME='test/t';
 TABLE_ID	NAME	FLAG	N_COLS	SPACE
-19	test/t	1	6	0
+#	test/t	1	6	0
 DROP TABLE t;

=== modified file 'mysql-test/suite/innodb/r/innodb-index.result'
--- a/mysql-test/suite/innodb/r/innodb-index.result	revid:marko.makela@stripped46-y813g63r1tas679u
+++ b/mysql-test/suite/innodb/r/innodb-index.result	revid:marko.makela@stripped26-xbtmjnaw20exebzq
@@ -445,7 +445,8 @@ info: Records: 0  Duplicates: 0  Warning
 create index dc on t1 (b,c);
 affected rows: 0
 info: Records: 0  Duplicates: 0  Warnings: 0
-alter table t2 add primary key (alpha), change a alpha int;
+alter table t2 add primary key (alpha), change a alpha int,
+change b beta int not null;
 affected rows: 0
 info: Records: 0  Duplicates: 0  Warnings: 0
 insert into t1 values (1,1,1);
@@ -482,22 +483,22 @@ t3	CREATE TABLE `t3` (
   KEY `c` (`c`),
   CONSTRAINT `dc` FOREIGN KEY (`a`) REFERENCES `t1` (`a`)
 ) ENGINE=InnoDB DEFAULT CHARSET=latin1
-alter table t2 drop index b, add index (b);
+alter table t2 drop index b, add index (beta);
 affected rows: 0
 info: Records: 0  Duplicates: 0  Warnings: 0
 show create table t2;
 Table	Create Table
 t2	CREATE TABLE `t2` (
   `alpha` int(11) NOT NULL DEFAULT '0',
-  `b` int(11) NOT NULL,
+  `beta` int(11) NOT NULL,
   `c` int(11) NOT NULL,
   `d` int(11) NOT NULL,
   `e` int(11) DEFAULT NULL,
   PRIMARY KEY (`alpha`),
   UNIQUE KEY `dc` (`d`,`c`),
   KEY `c` (`c`),
-  KEY `b` (`b`),
-  CONSTRAINT `t2_ibfk_1` FOREIGN KEY (`b`) REFERENCES `t1` (`b`) ON DELETE CASCADE,
+  KEY `beta` (`beta`),
+  CONSTRAINT `t2_ibfk_1` FOREIGN KEY (`beta`) 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`)
 ) ENGINE=InnoDB DEFAULT CHARSET=latin1
@@ -512,11 +513,11 @@ alter table t4 drop foreign key dc;
 affected rows: 0
 info: Records: 0  Duplicates: 0  Warnings: 0
 select * from t2;
-alpha	b	c	d	e
+alpha	beta	c	d	e
 1	1	1	1	1
 delete from t1;
 select * from t2;
-alpha	b	c	d	e
+alpha	beta	c	d	e
 drop table t2,t4,t3,t1;
 create table t1(a int not null, b int, c char(10), d varchar(20), primary key (a)) engine = innodb default charset=utf8;
 insert into t1 values (1,1,'ab','ab'),(2,2,'ac','ac'),(3,2,'ad','ad'),(4,4,'afe','afe');

=== modified file 'mysql-test/suite/innodb/t/innodb-alter-nullable.test'
--- a/mysql-test/suite/innodb/t/innodb-alter-nullable.test	revid:marko.makela@stripped46-y813g63r1tas679u
+++ b/mysql-test/suite/innodb/t/innodb-alter-nullable.test	revid:marko.makela@stripped601091526-xbtmjnaw20exebzq
@@ -56,6 +56,7 @@ connection default;
 # This should be no-op.
 ALTER TABLE t MODIFY c2 INT NULL, ALGORITHM=INPLACE;
 
+--replace_column 1 #
 SELECT * FROM INFORMATION_SCHEMA.INNODB_SYS_TABLES
 WHERE NAME='test/t';
 

=== modified file 'mysql-test/suite/innodb/t/innodb-index.test'
--- a/mysql-test/suite/innodb/t/innodb-index.test	revid:marko.makela@stripped0346-y813g63r1tas679u
+++ b/mysql-test/suite/innodb/t/innodb-index.test	revid:marko.makela@stripped26-xbtmjnaw20exebzq
@@ -152,7 +152,8 @@ alter table t2 drop index b, drop index
 create unique index dc on t2 (d,c);
 create index dc on t1 (b,c);
 # This should preserve the foreign key constraints.
-alter table t2 add primary key (alpha), change a alpha int;
+alter table t2 add primary key (alpha), change a alpha int,
+change b beta int not null;
 --disable_info
 insert into t1 values (1,1,1);
 insert into t3 values (1,1,1);
@@ -173,7 +174,7 @@ alter table t3 add constraint dc foreign
 SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS;
 show create table t3;
 --enable_info
-alter table t2 drop index b, add index (b);
+alter table t2 drop index b, add index (beta);
 --disable_info
 show create table t2;
 --error ER_ROW_IS_REFERENCED_2

=== modified file 'storage/innobase/handler/handler0alter.cc'
--- a/storage/innobase/handler/handler0alter.cc	revid:marko.makela@stripped79u
+++ b/storage/innobase/handler/handler0alter.cc	revid:marko.makela@stripped
@@ -3293,6 +3293,7 @@ innobase_drop_foreign(
 @param nth_col		0-based index of the column
 @param from		old column name
 @param to		new column name
+@param new_clustered	whether the table has been rebuilt
 @retval true		Failure
 @retval false		Success */
 static __attribute__((nonnull, warn_unused_result))
@@ -3304,20 +3305,26 @@ innobase_rename_column(
 	trx_t*			trx,
 	ulint			nth_col,
 	const char*		from,
-	const char*		to)
+	const char*		to,
+	bool			new_clustered)
 {
 	pars_info_t*	info;
 	dberr_t		error;
 
 	DBUG_ENTER("innobase_rename_column");
 
-	DBUG_ASSERT(trx_get_dict_operation(trx) == TRX_DICT_OP_INDEX);
+	DBUG_ASSERT(trx_get_dict_operation(trx)
+		    == new_clustered ? TRX_DICT_OP_TABLE : TRX_DICT_OP_INDEX);
 	ut_ad(trx->dict_operation_lock_mode == RW_X_LATCH);
 	ut_ad(mutex_own(&dict_sys->mutex));
 #ifdef UNIV_SYNC_DEBUG
 	ut_ad(rw_lock_own(&dict_operation_lock, RW_LOCK_EX));
 #endif /* UNIV_SYNC_DEBUG */
 
+	if (new_clustered) {
+		goto rename_foreign;
+	}
+
 	info = pars_info_create();
 
 	pars_info_add_ull_literal(info, "tableid", prebuilt->table->id);
@@ -3392,6 +3399,7 @@ err_exit:
 		}
 	}
 
+rename_foreign:
 	trx->op_info = "renaming column in SYS_FOREIGN_COLS";
 
 	for (dict_foreign_t* foreign = UT_LIST_GET_FIRST(
@@ -3461,11 +3469,63 @@ err_exit:
 	}
 
 	trx->op_info = "";
-	/* Rename the column in the data dictionary cache. */
-	dict_mem_table_col_rename(prebuilt->table, nth_col, from, to);
+	if (!new_clustered) {
+		/* Rename the column in the data dictionary cache. */
+		dict_mem_table_col_rename(prebuilt->table, nth_col, from, to);
+	}
 	DBUG_RETURN(false);
 }
 
+/** Rename columns.
+@param ha_alter_info	Data used during in-place alter.
+@param new_clustered	whether the table has been rebuilt
+@param table		the TABLE
+@param table_share	the TABLE_SHARE
+@param prebuilt		the prebuilt struct
+@param trx		data dictionary transaction
+@retval true		Failure
+@retval false		Success */
+static __attribute__((nonnull, warn_unused_result))
+bool
+innobase_rename_columns(
+/*====================*/
+	Alter_inplace_info*	ha_alter_info,
+	bool			new_clustered,
+	const TABLE*		table,
+	const TABLE_SHARE*	table_share,
+	row_prebuilt_t*		prebuilt,
+	trx_t*			trx)
+{
+	List_iterator_fast<Create_field> cf_it;
+	uint i = 0;
+
+	for (Field** fp = table->field; *fp; fp++, i++) {
+		if (!((*fp)->flags & FIELD_IS_RENAMED)) {
+			continue;
+		}
+
+		cf_it.init(ha_alter_info->alter_info->create_list);
+		while (Create_field* cf = cf_it++) {
+			if (cf->field == *fp) {
+				if (innobase_rename_column(
+					    table_share,
+					    prebuilt, trx, i,
+					    cf->field->field_name,
+					    cf->field_name, new_clustered)) {
+					return(true);
+				}
+				goto processed_field;
+			}
+		}
+
+		ut_error;
+processed_field:
+		continue;
+	}
+
+	return(false);
+}
+
 /** Commit or rollback the changes made during
 prepare_inplace_alter_table() and inplace_alter_table() inside
 the storage engine. Note that the allowed level of concurrency
@@ -3605,6 +3665,14 @@ ha_innobase::commit_inplace_alter_table(
 			}
 		}
 
+		if ((ha_alter_info->handler_flags
+		     & Alter_inplace_info::ALTER_COLUMN_NAME)
+		    && innobase_rename_columns(ha_alter_info, true, table,
+					       table_share, prebuilt, trx)) {
+			err = -1;
+			goto drop_new_clustered;
+		}
+
 		/* A new clustered index was defined for the table
 		and there was no error at this point. We can
 		now rename the old table as a temporary table,
@@ -3716,33 +3784,10 @@ ha_innobase::commit_inplace_alter_table(
 
 	if (err == 0 && !new_clustered
 	    && (ha_alter_info->handler_flags
-		& Alter_inplace_info::ALTER_COLUMN_NAME)) {
-		List_iterator_fast<Create_field> cf_it;
-		uint i = 0;
-
-		for (Field** fp = table->field; *fp && err == 0; fp++, i++) {
-			if (!((*fp)->flags & FIELD_IS_RENAMED)) {
-				continue;
-			}
-
-			cf_it.init(ha_alter_info->alter_info->create_list);
-			while (Create_field* cf = cf_it++) {
-				if (cf->field == *fp) {
-					if (innobase_rename_column(
-						    table_share,
-						    prebuilt, trx, i,
-						    cf->field->field_name,
-						    cf->field_name)) {
-						err = -1;
-					}
-					goto processed_field;
-				}
-			}
-
-			ut_error;
-processed_field:
-			continue;
-		}
+		& Alter_inplace_info::ALTER_COLUMN_NAME)
+	    && innobase_rename_columns(ha_alter_info, false, table,
+				       table_share, prebuilt, trx)) {
+		err = -1;
 	}
 
 	if (err == 0 && ctx && ctx->num_to_add_fk > 0) {

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk-wl6255 branch (marko.makela:3923 to 3925)Bug#14145138 WL#5545marko.makela1 Jun