branches/zip: Add index translation table to map mysql index

number to InnoDB index structure directly. Fix Bug #47622:
"the new index is added before the existing ones in MySQL,
but after one in SE".

rb://215, approved by Marko

handler/ha_innodb.cc

@@ -3067,6 +3067,208 @@ innobase_get_int_col_max_value(
 	return(max_value);
 }
 
+/*******************************************************************//**
+This function checks whether the index column information
+is consistent between KEY info from mysql and that from innodb index.
+@return TRUE if all column types match. */
+static
+ibool
+innobase_match_index_columns(
+/*=========================*/
+	const KEY*		key_info,	/*!< in: Index info
+						from mysql */
+	const dict_index_t*	index_info)	/*!< in: Index info
+						from Innodb */
+{
+	const KEY_PART_INFO*	key_part;
+	const KEY_PART_INFO*	key_end;
+	const dict_field_t*	innodb_idx_fld;
+	const dict_field_t*	innodb_idx_fld_end;
+
+	DBUG_ENTER("innobase_match_index_columns");
+
+	/* Check whether user defined index column count matches */
+	if (key_info->key_parts != index_info->n_user_defined_cols) {
+		DBUG_RETURN(FALSE);
+	}
+
+	key_part = key_info->key_part;
+	key_end = key_part + key_info->key_parts;
+	innodb_idx_fld = index_info->fields;
+	innodb_idx_fld_end = index_info->fields + index_info->n_fields;
+
+	/* Check each index column's datatype. We do not check
+	column name because there exists case that index
+	column name got modified in mysql but such change does not
+	propagate to InnoDB.
+	One hidden assumption here is that the index column sequences
+	are matched up between those in mysql and Innodb. */
+	for (; key_part != key_end; ++key_part) {
+		ulint	col_type;
+		ibool	is_unsigned;
+		ulint	mtype = innodb_idx_fld->col->mtype;
+
+		/* Need to translate to InnoDB column type before
+		comparison. */
+		col_type = get_innobase_type_from_mysql_type(&is_unsigned,
+							     key_part->field);
+
+		/* Ignore Innodb specific system columns. */
+		while (mtype == DATA_SYS) {
+			innodb_idx_fld++;
+
+			if (innodb_idx_fld >= innodb_idx_fld_end) {
+				DBUG_RETURN(FALSE);
+			}
+		}
+
+		if (col_type != mtype) {
+			/* Column Type mismatches */
+			DBUG_RETURN(FALSE);
+		}
+
+		innodb_idx_fld++;
+	}
+
+	DBUG_RETURN(TRUE);
+}
+
+/*******************************************************************//**
+This function builds a translation table in INNOBASE_SHARE
+structure for fast index location with mysql array number from its
+table->key_info structure. This also provides the necessary translation
+between the key order in mysql key_info and Innodb ib_table->indexes if
+they are not fully matched with each other.
+Note we do not have any mutex protecting the translation table
+building based on the assumption that there is no concurrent
+index creation/drop and DMLs that requires index lookup. All table
+handle will be closed before the index creation/drop.
+@return TRUE if index translation table built successfully */
+static
+ibool
+innobase_build_index_translation(
+/*=============================*/
+	const TABLE*		table,	  /*!< in: table in MySQL data
+					  dictionary */
+	dict_table_t*		ib_table, /*!< in: table in Innodb data
+					  dictionary */
+	INNOBASE_SHARE*		share)	  /*!< in/out: share structure
+					  where index translation table
+					  will be constructed in. */
+{
+	ulint		mysql_num_index;
+	ulint		ib_num_index;
+	dict_index_t**	index_mapping;
+	ibool		ret = TRUE;
+
+	DBUG_ENTER("innobase_build_index_translation");
+
+	mysql_num_index = table->s->keys;
+	ib_num_index = UT_LIST_GET_LEN(ib_table->indexes);
+
+	/* Innodb could have own system indexes. */
+	ut_a(ib_num_index >= mysql_num_index);
+
+	index_mapping = share->idx_trans_tbl.index_mapping;
+
+	/* If index entry count is non-zero, nothing has
+	changed since last update, directly return TRUE */
+	if (share->idx_trans_tbl.index_count) {
+		/* Index entry count should still match mysql_num_index */
+		ut_a(share->idx_trans_tbl.index_count == mysql_num_index);
+		goto func_exit;
+	}
+
+	/* The number of index increased, rebuild the mapping table */
+	if (mysql_num_index > share->idx_trans_tbl.array_size) {
+		index_mapping = (dict_index_t**) my_realloc(index_mapping,
+							mysql_num_index *
+							sizeof(*index_mapping),
+							MYF(MY_ALLOW_ZERO_PTR));
+
+		if (!index_mapping) {
+			ret = FALSE;
+			goto func_exit;
+		}
+
+		share->idx_trans_tbl.array_size = mysql_num_index;
+	}
+
+
+	/* For each index in the mysql key_info array, fetch its
+	corresponding InnoDB index pointer into index_mapping
+	array. */
+	for (ulint count = 0; count < mysql_num_index; count++) {
+
+		/* Fetch index pointers into index_mapping according to mysql
+		index sequence */
+		index_mapping[count] = dict_table_get_index_on_name(
+			ib_table, table->key_info[count].name);
+
+		if (!index_mapping[count]) {
+			sql_print_error("Cannot find index %s in InnoDB "
+					"index dictionary.",
+					table->key_info[count].name);
+			ret = FALSE;
+			goto func_exit;
+		}
+
+		/* Double check fetched index has the same
+		column info as those in mysql key_info. */
+		if (!innobase_match_index_columns(&table->key_info[count],
+					          index_mapping[count])) {
+			sql_print_error("Found index %s whose column info "
+					"does not match that of MySQL.",
+					table->key_info[count].name);
+			ret = FALSE;
+			goto func_exit;
+		}
+	}
+
+	/* Successfully built the translation table */
+	share->idx_trans_tbl.index_count = mysql_num_index;
+
+func_exit:
+	if (!ret) {
+		/* Build translation table failed. */
+		my_free(index_mapping, MYF(MY_ALLOW_ZERO_PTR));
+
+		share->idx_trans_tbl.array_size = 0;
+		share->idx_trans_tbl.index_count = 0;
+		index_mapping = NULL;
+	}
+
+	share->idx_trans_tbl.index_mapping = index_mapping;
+
+	DBUG_RETURN(ret);
+}
+
+/*******************************************************************//**
+This function uses index translation table to quickly locate the
+requested index structure.
+Note we do not have mutex protection for the index translatoin table
+access, it is based on the assumption that there is no concurrent
+translation table rebuild (fter create/drop index) and DMLs that
+require index lookup.
+@return dict_index_t structure for requested index. NULL if
+fail to locate the index structure. */
+static
+dict_index_t*
+innobase_index_lookup(
+/*==================*/
+	INNOBASE_SHARE*	share,	/*!< in: share structure for index
+				translation table. */
+	uint		keynr)	/*!< in: index number for the requested
+				index */
+{
+	if (!share->idx_trans_tbl.index_mapping
+	    || keynr >= share->idx_trans_tbl.index_count) {
+		return(NULL);
+	}
+
+	return(share->idx_trans_tbl.index_mapping[keynr]);
+}
+
 /************************************************************************
 Set the autoinc column max value. This should only be called once from
 ha_innobase::open(). Therefore there's no need for a covering lock. */
@@ -3283,6 +3485,11 @@ retry:
 	primary_key = table->s->primary_key;
 	key_used_on_scan = primary_key;
 
+	if (!innobase_build_index_translation(table, ib_table, share)) {
+		  sql_print_error("Build InnoDB index translation table for"
+				  " Table %s failed", name);
+	}
+
 	/* Allocate a buffer for a 'row reference'. A row reference is
 	a string of bytes of length ref_length which uniquely specifies
 	a row in our table. Note that MySQL may also compare two row
@@ -5198,8 +5405,20 @@ ha_innobase::innobase_get_index(
 	if (keynr != MAX_KEY && table->s->keys > 0) {
 		key = table->key_info + keynr;
 
-		index = dict_table_get_index_on_name(prebuilt->table,
-						     key->name);
+		index = innobase_index_lookup(share, keynr);
+
+		if (index) {
+			ut_a(ut_strcmp(index->name, key->name) == 0);
+		} else {
+			sql_print_error("InnoDB could not find index %s "
+					"key no %u for table %s through "
+					" its index translation table",
+					key ? key->name : "NULL", keynr,
+					prebuilt->table->name);
+
+			index = dict_table_get_index_on_name(prebuilt->table,
+							     key->name);
+		}
 	} else {
 		index = dict_table_get_first_index(prebuilt->table);
 	}
@@ -6890,7 +7109,9 @@ ha_innobase::records_in_range(
 
 	key = table->key_info + active_index;
 
-	index = dict_table_get_index_on_name(prebuilt->table, key->name);
+	index = innobase_get_index(keynr);
+
+	ut_a(ut_strcmp(index->name, key->name) == 0);
 
 	/* MySQL knows about this index and so we must be able to find it.*/
 	ut_a(index);
@@ -7079,6 +7300,7 @@ ha_innobase::info(
 	char		path[FN_REFLEN];
 	os_file_stat_t	stat_info;
 
+
 	DBUG_ENTER("info");
 
 	/* If we are forcing recovery at a high level, we will suppress
@@ -7239,13 +7461,29 @@ ha_innobase::info(
 	}
 
 	if (flag & HA_STATUS_CONST) {
-		index = dict_table_get_first_index(ib_table);
+		/* Verify the number of index in InnoDB and MySQL
+		matches up. If prebuilt->clust_index_was_generated
+		holds, InnoDB defines GEN_CLUST_INDEX internally */
+		ulint	num_innodb_index = UT_LIST_GET_LEN(ib_table->indexes)
+					- prebuilt->clust_index_was_generated;
 
-		if (prebuilt->clust_index_was_generated) {
-			index = dict_table_get_next_index(index);
+		if (table->s->keys != num_innodb_index) {
+			sql_print_error("Table %s contains %lu "
+					"indexes inside InnoDB, which "
+					"is different from the number of "
+					"indexes %u defined in the MySQL ",
+					ib_table->name, num_innodb_index,
+					table->s->keys);
 		}
 
 		for (i = 0; i < table->s->keys; i++) {
+			/* We could get index quickly through internal
+			index mapping with the index translation table.
+			The identity of index (match up index name with
+			that of table->key_info[i]) is already verified in
+			innobase_get_index().  */
+			index = innobase_get_index(i);
+
 			if (index == NULL) {
 				sql_print_error("Table %s contains fewer "
 						"indexes inside InnoDB than "
@@ -7297,8 +7535,6 @@ ha_innobase::info(
 				  rec_per_key >= ~(ulong) 0 ? ~(ulong) 0 :
 				  (ulong) rec_per_key;
 			}
-
-			index = dict_table_get_next_index(index);
 		}
 	}
 
@@ -8462,6 +8698,11 @@ static INNOBASE_SHARE* get_share(const char* table_name)
 			    innobase_open_tables, fold, share);
 
 		thr_lock_init(&share->lock);
+
+		/* Index translation table initialization */
+		share->idx_trans_tbl.index_mapping = NULL;
+		share->idx_trans_tbl.index_count = 0;
+		share->idx_trans_tbl.array_size = 0;
 	}
 
 	share->use_count++;
@@ -8492,6 +8733,11 @@ static void free_share(INNOBASE_SHARE* share)
 		HASH_DELETE(INNOBASE_SHARE, table_name_hash,
 			    innobase_open_tables, fold, share);
 		thr_lock_delete(&share->lock);
+
+		/* Free any memory from index translation table */
+		my_free(share->idx_trans_tbl.index_mapping,
+			MYF(MY_ALLOW_ZERO_PTR));
+
 		my_free(share, MYF(0));
 
 		/* TODO: invoke HASH_MIGRATE if innobase_open_tables

handler/ha_innodb.h

@@ -27,15 +27,31 @@ Place, Suite 330, Boston, MA 02111-1307 USA
 #pragma interface			/* gcc class implementation */
 #endif
 
+/* Structure defines translation table between mysql index and innodb
+index structures */
+typedef struct innodb_idx_translate_struct {
+	ulint		index_count;	/*!< number of valid index entries
+					in the index_mapping array */
+	ulint		array_size;	/*!< array size of index_mapping */
+	dict_index_t**	index_mapping;	/*!< index pointer array directly
+					maps to index in Innodb from MySQL
+					array index */
+} innodb_idx_translate_t;
+
+
 /** InnoDB table share */
 typedef struct st_innobase_share {
-	THR_LOCK	lock;		/*!< MySQL lock protecting
-					this structure */
-	const char*	table_name;	/*!< InnoDB table name */
-	uint		use_count;	/*!< reference count,
-					incremented in get_share()
-					and decremented in free_share() */
-	void*		table_name_hash;/*!< hash table chain node */
+	THR_LOCK		lock;		/*!< MySQL lock protecting
+						this structure */
+	const char*		table_name;	/*!< InnoDB table name */
+	uint			use_count;	/*!< reference count,
+						incremented in get_share()
+						and decremented in
+						free_share() */
+	void*			table_name_hash;/*!< hash table chain node */
+	innodb_idx_translate_t	idx_trans_tbl;	/*!< index translation
+						table between MySQL and
+						Innodb */
 } INNOBASE_SHARE;
 
 

handler/handler0alter.cc

@@ -764,6 +764,10 @@ err_exit:
 
 	ut_ad(error == DB_SUCCESS);
 
+	/* We will need to rebuild index translation table. Set
+	valid index entry count in the translation table to zero */
+	share->idx_trans_tbl.index_count = 0;
+
 	/* Commit the data dictionary transaction in order to release
 	the table locks on the system tables.  This means that if
 	MySQL crashes while creating a new primary key inside
@@ -1198,6 +1202,10 @@ ha_innobase::final_drop_index(
 		ut_a(!index->to_be_dropped);
 	}
 
+	/* We will need to rebuild index translation table. Set
+	valid index entry count in the translation table to zero */
+	share->idx_trans_tbl.index_count = 0;
+
 #ifdef UNIV_DEBUG
 	dict_table_check_for_dup_indexes(prebuilt->table);
 #endif

mysql-test/innodb_bug47622.result

+CREATE TABLE bug47622(
+`rule_key` int(11) NOT NULL DEFAULT '0',
+`seq` smallint(6) NOT NULL DEFAULT '0',
+`action` smallint(6) NOT NULL DEFAULT '0',
+`arg_id` smallint(6) DEFAULT NULL,
+`else_ind` TINYINT NOT NULL,
+KEY IDX_A (`arg_id`)
+) ENGINE=InnoDB;
+ALTER TABLE bug47622 ADD UNIQUE IDX_B (rule_key,else_ind,seq,action,arg_id);
+drop index IDX_B on bug47622;
+create index idx on bug47622(seq, arg_id);
+ALTER TABLE bug47622 ADD UNIQUE IDX_X (rule_key,else_ind,seq,action);
+drop table bug47622;
+CREATE TABLE bug47622 (
+`a` int(11) NOT NULL,
+`b` int(11) DEFAULT NULL,
+`c` char(10) DEFAULT NULL,
+`d` varchar(20) DEFAULT NULL,
+PRIMARY KEY (`a`),
+KEY `b` (`b`)
+) ENGINE=InnoDB;
+alter table bug47622 add unique index (c), add index (d);
+drop table bug47622;

mysql-test/innodb_bug47622.test

+# This is the test for bug 47622. There could be index
+# metadata sequence mismatch between MySQL and Innodb
+# after creating index through FIC interfaces.
+# We resolve the problem by sync the index sequence
+# up when opening the table.
+
+--source include/have_innodb.inc
+
+connect (a,localhost,root,,);
+connect (b,localhost,root,,);
+
+# Create a table with a non-unique index
+CREATE TABLE bug47622(
+	`rule_key` int(11) NOT NULL DEFAULT '0',
+	`seq` smallint(6) NOT NULL DEFAULT '0',
+	`action` smallint(6) NOT NULL DEFAULT '0',
+	`arg_id` smallint(6) DEFAULT NULL,
+	`else_ind` TINYINT NOT NULL,
+	KEY IDX_A (`arg_id`)
+) ENGINE=InnoDB;
+
+connection a;
+
+# A subsequent creating unique index should not trigger
+# any error message. Unique index would be ranked ahead
+# of regular index.
+ALTER TABLE bug47622 ADD UNIQUE IDX_B (rule_key,else_ind,seq,action,arg_id);
+
+drop index IDX_B on bug47622;
+
+# In another connection, create additional set of normal
+# index and unique index. Again, unique index would be ranked
+# ahead of regular index.
+connection b;
+create index idx on bug47622(seq, arg_id);
+
+ALTER TABLE bug47622 ADD UNIQUE IDX_X (rule_key,else_ind,seq,action);
+
+drop table bug47622;
+
+# Create a table with one Primary key and a non-unique key
+CREATE TABLE bug47622 (
+  `a` int(11) NOT NULL,
+  `b` int(11) DEFAULT NULL,
+  `c` char(10) DEFAULT NULL,
+  `d` varchar(20) DEFAULT NULL,
+  PRIMARY KEY (`a`),
+  KEY `b` (`b`)
+) ENGINE=InnoDB;
+
+# Add two index with one unique and one non-unique.
+# Index sequence is "PRIMARY", "c", "b" and "d"
+alter table bug47622 add unique index (c), add index (d);
+
+drop table bug47622;