From 84d09ccc72947b726dec49833c50b738873a9b19 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Thu, 1 Apr 2010 15:30:11 +0400 Subject: [PATCH] Applying InnoDB snapshot, fixes BUG#47622. Detailed revision comments: r6526 | jyang | 2010-01-28 18:12:40 +0200 (Thu, 28 Jan 2010) | 8 lines 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 --- .../suite/innodb/r/innodb_bug47622.result | 23 ++ .../suite/innodb/t/innodb_bug47622.test | 55 ++++ storage/innodb_plugin/handler/ha_innodb.cc | 262 +++++++++++++++++- storage/innodb_plugin/handler/ha_innodb.h | 30 +- .../innodb_plugin/handler/handler0alter.cc | 8 + 5 files changed, 363 insertions(+), 15 deletions(-) create mode 100644 mysql-test/suite/innodb/r/innodb_bug47622.result create mode 100644 mysql-test/suite/innodb/t/innodb_bug47622.test diff --git a/mysql-test/suite/innodb/r/innodb_bug47622.result b/mysql-test/suite/innodb/r/innodb_bug47622.result new file mode 100644 index 00000000000..f5d13711c52 --- /dev/null +++ b/mysql-test/suite/innodb/r/innodb_bug47622.result @@ -0,0 +1,23 @@ +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; diff --git a/mysql-test/suite/innodb/t/innodb_bug47622.test b/mysql-test/suite/innodb/t/innodb_bug47622.test new file mode 100644 index 00000000000..9cf9d0e531b --- /dev/null +++ b/mysql-test/suite/innodb/t/innodb_bug47622.test @@ -0,0 +1,55 @@ +# 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; diff --git a/storage/innodb_plugin/handler/ha_innodb.cc b/storage/innodb_plugin/handler/ha_innodb.cc index 95b0d331076..9219bd2acc2 100644 --- a/storage/innodb_plugin/handler/ha_innodb.cc +++ b/storage/innodb_plugin/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 diff --git a/storage/innodb_plugin/handler/ha_innodb.h b/storage/innodb_plugin/handler/ha_innodb.h index 0e366a1eb2c..5dd9726ae4c 100644 --- a/storage/innodb_plugin/handler/ha_innodb.h +++ b/storage/innodb_plugin/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; diff --git a/storage/innodb_plugin/handler/handler0alter.cc b/storage/innodb_plugin/handler/handler0alter.cc index a5008991400..47999ae37f8 100644 --- a/storage/innodb_plugin/handler/handler0alter.cc +++ b/storage/innodb_plugin/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