From: Dmitry Lenev Date: November 21 2011 9:13am Subject: bzr push into mysql-trunk-wl5534 branch (Dmitry.Lenev:3427 to 3428) WL#5534 List-Archive: http://lists.mysql.com/commits/142090 Message-Id: <20111121091329.17204422176@jubjub> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 3428 Dmitry Lenev 2011-11-21 WL#5534 Online ALTER, Phase 1. Patch #51: Review changes. - Removed Alter_inplace_info::CHANGE_COLUMN flag. - Changed fill_alter_inplace_info() to properly handle key comparison in cases when some columns change their positions. - Updated result of innodb-index.test in which one query started to use pre-WL#5534 execution plan. modified: mysql-test/suite/innodb/r/innodb-index.result sql/handler.cc sql/handler.h sql/sql_table.cc 3427 Jon Olav Hauglid 2011-11-16 [merge] Merge from mysql-trunk to mysql-trunk-wl5534 No conflicts removed: mysql-test/extra/rpl_tests/rpl_parallel_benchmark_load.test mysql-test/suite/rpl/t/rpl_parallel_benchmark-master.opt mysql-test/suite/rpl/t/rpl_parallel_benchmark-slave.opt mysql-test/suite/rpl/t/rpl_parallel_benchmark.test added: mysql-test/suite/rpl/r/rpl_row_find_row_debug.result mysql-test/suite/rpl/t/rpl_row_find_row_debug.test mysql-test/suite/sys_vars/r/metadata_locks_cache_size_basic.result mysql-test/suite/sys_vars/t/metadata_locks_cache_size_basic-master.opt mysql-test/suite/sys_vars/t/metadata_locks_cache_size_basic.test modified: client/mysql_upgrade.c mysql-test/extra/rpl_tests/rpl_parallel_load.test mysql-test/r/mysql_upgrade.result mysql-test/r/mysqld--help-notwin.result mysql-test/r/mysqld--help-win.result mysql-test/t/mysql_upgrade.test sql/item_func.cc sql/log_event.cc sql/log_event.h sql/mdl.cc sql/mdl.h sql/rpl_rli.cc sql/rpl_rli.h sql/sql_class.h sql/sql_partition.cc sql/sql_plist.h sql/sys_vars.cc === modified file 'mysql-test/suite/innodb/r/innodb-index.result' --- a/mysql-test/suite/innodb/r/innodb-index.result 2011-10-24 13:25:09 +0000 +++ b/mysql-test/suite/innodb/r/innodb-index.result 2011-11-21 09:13:00 +0000 @@ -951,7 +951,7 @@ Table Op Msg_type Msg_text test.t1 check status OK explain select * from t1 where b like 'adfd%'; id select_type table type possible_keys key key_len ref rows Extra -1 SIMPLE t1 range b b 769 NULL 11 Using where +1 SIMPLE t1 ALL b NULL NULL NULL 15 Using where drop table t1; set global innodb_file_per_table=on; set global innodb_file_format='Barracuda'; === modified file 'sql/handler.cc' --- a/sql/handler.cc 2011-11-16 14:12:49 +0000 +++ b/sql/handler.cc 2011-11-21 09:13:00 +0000 @@ -3799,7 +3799,6 @@ handler::check_if_supported_inplace_alte Alter_inplace_info::ADD_PK_INDEX | Alter_inplace_info::DROP_PK_INDEX; Alter_inplace_info::HA_ALTER_FLAGS inplace_offline_operations= - Alter_inplace_info::CHANGE_COLUMN | Alter_inplace_info::ALTER_COLUMN_NAME | Alter_inplace_info::COLUMN_DEFAULT_VALUE | Alter_inplace_info::CHANGE_CREATE_OPTION; === modified file 'sql/handler.h' --- a/sql/handler.h 2011-11-16 14:12:49 +0000 +++ b/sql/handler.h 2011-11-21 09:13:00 +0000 @@ -927,6 +927,14 @@ public: }; +/** + Class describing changes to be done by ALTER TABLE. + Instance of this class is passed to storage engine in order + to determine if this ALTER TABLE can be done using in-place + algorithm. It is also used for executing the ALTER TABLE + using in-place algorithm. +*/ + class Alter_inplace_info { public: @@ -939,6 +947,11 @@ public: be performed by copying the table to a temporary table, will not have their own flags here (e.g. ALTER TABLE FORCE, ALTER TABLE ENGINE). + + We generally try to specify handler flags only if there are real + changes. But in cases when it is cumbersome to determine if some + attribute has really changed we might choose to set flag + pessimistically, for example, relying on parser output only. */ typedef ulong HA_ALTER_FLAGS; @@ -966,36 +979,33 @@ public: // Drop column static const HA_ALTER_FLAGS DROP_COLUMN = 1L << 7; - // General flag for CHANGE [COLUMN] | MODIFY [COLUMN], See ALTER_COLUMN_* - static const HA_ALTER_FLAGS CHANGE_COLUMN = 1L << 8; - // Rename column - static const HA_ALTER_FLAGS ALTER_COLUMN_NAME = 1L << 9; + static const HA_ALTER_FLAGS ALTER_COLUMN_NAME = 1L << 8; // Change column datatype - static const HA_ALTER_FLAGS ALTER_COLUMN_TYPE = 1L << 10; + static const HA_ALTER_FLAGS ALTER_COLUMN_TYPE = 1L << 9; // Reorder column - static const HA_ALTER_FLAGS ALTER_COLUMN_ORDER = 1L << 11; + static const HA_ALTER_FLAGS ALTER_COLUMN_ORDER = 1L << 10; // Change column from NULL to NOT NULL or vice versa - static const HA_ALTER_FLAGS ALTER_COLUMN_NULLABLE = 1L << 12; + static const HA_ALTER_FLAGS ALTER_COLUMN_NULLABLE = 1L << 11; // Set or remove default column value - static const HA_ALTER_FLAGS COLUMN_DEFAULT_VALUE = 1L << 13; + static const HA_ALTER_FLAGS COLUMN_DEFAULT_VALUE = 1L << 12; // Add foreign key - static const HA_ALTER_FLAGS ADD_FOREIGN_KEY = 1L << 14; + static const HA_ALTER_FLAGS ADD_FOREIGN_KEY = 1L << 13; // Drop foreign key - static const HA_ALTER_FLAGS DROP_FOREIGN_KEY = 1L << 15; + static const HA_ALTER_FLAGS DROP_FOREIGN_KEY = 1L << 14; // Change character set - static const HA_ALTER_FLAGS CHANGE_CHARACTER_SET = 1L << 16; - static const HA_ALTER_FLAGS SET_DEFAULT_CHARACTER_SET = 1L << 17; + static const HA_ALTER_FLAGS CHANGE_CHARACTER_SET = 1L << 15; + static const HA_ALTER_FLAGS SET_DEFAULT_CHARACTER_SET = 1L << 16; // table_options changed, see HA_CREATE_INFO::used_fields for details. - static const HA_ALTER_FLAGS CHANGE_CREATE_OPTION = 1L << 18; + static const HA_ALTER_FLAGS CHANGE_CREATE_OPTION = 1L << 17; KEY *key_info_buffer; uint key_count; === modified file 'sql/sql_table.cc' --- a/sql/sql_table.cc 2011-11-16 15:29:56 +0000 +++ b/sql/sql_table.cc 2011-11-21 09:13:00 +0000 @@ -5011,6 +5011,79 @@ err: /** + Check if key is a candidate key, i.e. a unique index with no index + fields partial or nullable. +*/ + +static bool is_candidate_key(KEY *key) +{ + KEY_PART_INFO *key_part; + KEY_PART_INFO *key_part_end= key->key_part + key->key_parts; + + if (!(key->flags & HA_NOSAME)) + return false; + + for (key_part= key->key_part; key_part < key_part_end; key_part++) + { + if (key_part->field->maybe_null()) + return false; + if (key_part->key_part_flag & HA_PART_KEY_SEG) + return false; + } + + return true; +} + + +/** + Get Create_field object for newly created table by field index. + + @param alter_info Alter_info describing newly created table. + @param idx Field index. +*/ + +static Create_field *get_field_by_index(Alter_info *alter_info, uint idx) +{ + List_iterator_fast field_it(alter_info->create_list); + uint field_idx= 0; + Create_field *field; + + while ((field= field_it++) && field_idx < idx) + { field_idx++; } + + return field; +} + + +/** + Auxiliary function that marks all existing fields in newly added key + with FIELD_IN_ADD_INDEX flag. +*/ + +static void mark_old_fields_in_added_key(Alter_info *alter_info, const KEY *key) +{ + const KEY_PART_INFO *key_part, *end; + Create_field *field; + + key_part= key->key_part; + end= key_part + key->key_parts; + for(; key_part != end; key_part++) + { + field= get_field_by_index(alter_info, key_part->fieldnr); + /* + If the field has existed in old version of table mark it to + be part of new key. + */ + if (field->field) + field->field->flags|= FIELD_IN_ADD_INDEX; + } +} + + +/** + Compare original and new versions of a table and fill Alter_inplace_info + describing differences between those versions. + @param thd Thread @param table The original table. @param alter_info Alter options, fields and keys for the new @@ -5025,10 +5098,13 @@ err: By comparing the changes between the original and new table we can determine how much it has changed after ALTER TABLE - and whether we need to make a copy of the table, or just change - the .frm file. + and whether storage engine can carry out this ALTER in-place. Mark any changes detected in the ha_alter_flags. + We generally try to specify handler flags only if there are real + changes. But in cases when it is cumbersome to determine if some + attribute has really changed we might choose to set flag + pessimistically, for example, relying on parser output only. If there are no data changes, but index changes, 'index_drop_buffer' and/or 'index_add_buffer' are populated with offsets into @@ -5049,7 +5125,7 @@ static bool fill_alter_inplace_info(THD Field **f_ptr, *field; List_iterator_fast new_field_it, tmp_new_field_it; Create_field *new_field, *tmp_new_field; - KEY_PART_INFO *key_part; + KEY_PART_INFO *key_part, *new_part; KEY_PART_INFO *end; /* Remember if the new definition has new VARCHAR column; @@ -5057,7 +5133,6 @@ static bool fill_alter_inplace_info(THD */ bool varchar= create_info->varchar; uint candidate_key_count= 0; - bool not_nullable= true; DBUG_ENTER("fill_alter_inplace_info"); /* @@ -5096,20 +5171,14 @@ static bool fill_alter_inplace_info(THD tmp_alter_info.key_list.elements))) DBUG_RETURN(true); - /* - First we setup ha_alter_flags based on what was detected - by parser - */ + /* First we setup ha_alter_flags based on what was detected by parser. */ if (alter_info->flags & Alter_info::ALTER_ADD_COLUMN) ha_alter_info->handler_flags|= Alter_inplace_info::ADD_COLUMN; if (alter_info->flags & Alter_info::ALTER_DROP_COLUMN) ha_alter_info->handler_flags|= Alter_inplace_info::DROP_COLUMN; - if (alter_info->flags & Alter_info::ALTER_CHANGE_COLUMN) - ha_alter_info->handler_flags|= Alter_inplace_info::CHANGE_COLUMN; - if (alter_info->flags & Alter_info::ALTER_CHANGE_COLUMN_DEFAULT) + if (alter_info->flags & (Alter_info::ALTER_CHANGE_COLUMN | + Alter_info::ALTER_CHANGE_COLUMN_DEFAULT)) ha_alter_info->handler_flags|= Alter_inplace_info::COLUMN_DEFAULT_VALUE; - if (alter_info->flags & Alter_info::ALTER_COLUMN_ORDER) - ha_alter_info->handler_flags|= Alter_inplace_info::ALTER_COLUMN_ORDER; if (alter_info->flags & Alter_info::ADD_FOREIGN_KEY) ha_alter_info->handler_flags|= Alter_inplace_info::ADD_FOREIGN_KEY; if (alter_info->flags & Alter_info::DROP_FOREIGN_KEY) @@ -5117,111 +5186,142 @@ static bool fill_alter_inplace_info(THD if (alter_info->flags & Alter_info::ALTER_OPTIONS) ha_alter_info->handler_flags|= Alter_inplace_info::CHANGE_CREATE_OPTION; - /* - Some very basic checks. If number of fields changes, or the - handler, we need to run full ALTER TABLE. In the future - new fields can be added and old dropped without copy, but - not yet. - - Test also that engine was not given during ALTER TABLE, or - we are force to run regular alter table (copy). - E.g. ALTER TABLE tbl_name ENGINE=MyISAM. + /* TODO: do we want to have separate SE flags for the below cases? */ + if (create_info->used_fields & HA_CREATE_USED_CHARSET) + ha_alter_info->handler_flags|= Alter_inplace_info::CHANGE_CHARACTER_SET; + if (create_info->used_fields & HA_CREATE_USED_DEFAULT_CHARSET) + ha_alter_info->handler_flags|= Alter_inplace_info::SET_DEFAULT_CHARACTER_SET; - For the following ones we also want to run regular alter table: - ALTER TABLE tbl_name ORDER BY .. - ALTER TABLE tbl_name CONVERT TO CHARACTER SET .. - - At the moment we can't handle altering temporary tables without a copy. - We also test if OPTIMIZE TABLE was given and was mapped to alter table. - In that case we always do full copy. - */ - if (table->s->fields != alter_info->create_list.elements || - table->s->db_type() != create_info->db_type || - table->s->tmp_table || - create_info->used_fields & HA_CREATE_USED_ENGINE || - create_info->used_fields & HA_CREATE_USED_CHARSET || - create_info->used_fields & HA_CREATE_USED_DEFAULT_CHARSET || - (table->s->row_type != create_info->row_type) || - create_info->used_fields & HA_CREATE_USED_PACK_KEYS || - create_info->used_fields & HA_CREATE_USED_MAX_ROWS || - (alter_info->flags & (Alter_info::ALTER_RECREATE | - Alter_info::ADD_FOREIGN_KEY | - Alter_info::DROP_FOREIGN_KEY)) || - order_num || - (table->s->frm_version < FRM_VER_TRUE_VARCHAR && varchar)) - { - /* - Check what has changed and set alter_flags - */ - if (table->s->fields < alter_info->create_list.elements) - ha_alter_info->handler_flags|= Alter_inplace_info::ADD_COLUMN; - else if (table->s->fields > alter_info->create_list.elements) - ha_alter_info->handler_flags|= Alter_inplace_info::DROP_COLUMN; - if (create_info->used_fields & HA_CREATE_USED_CHARSET) - ha_alter_info->handler_flags|= Alter_inplace_info::CHANGE_CHARACTER_SET; - if (create_info->used_fields & HA_CREATE_USED_DEFAULT_CHARSET) - ha_alter_info->handler_flags|= Alter_inplace_info::SET_DEFAULT_CHARACTER_SET; - /* TODO check for ADD/DROP FOREIGN KEY */ - if (table->s->frm_version < FRM_VER_TRUE_VARCHAR && varchar) - ha_alter_info->handler_flags|= Alter_inplace_info::ALTER_COLUMN_TYPE; - } /* - Use transformed info to evaluate possibility of fast ALTER TABLE - but use the preserved field to persist modifications. + If we altering table with old VARCHAR fields we will be automatically + upgrading VARCHAR column types. */ - new_field_it.init(alter_info->create_list); - tmp_new_field_it.init(tmp_alter_info.create_list); + if (table->s->frm_version < FRM_VER_TRUE_VARCHAR && varchar) + ha_alter_info->handler_flags|= Alter_inplace_info::ALTER_COLUMN_TYPE; /* - Go through fields and check if the original ones are compatible - with new table. + Go through fields in old version of table and detect changes to them. + We don't want to rely solely on Alter_info flags for this since: + a) new definition of column can be fully identical to the old one + despite the fact that this column is mentioned in MODIFY clause. + b) even if new column type differs from its old column from metadata + point of view, it might be identical from storage engine point + of view (e.g. when ENUM('a','b') is changed to ENUM('a','b',c')). + c) flags passed to storage engine contain more detailed information + about nature of changes than those provided from parser. */ - for (f_ptr= table->field, new_field= new_field_it++, - tmp_new_field= tmp_new_field_it++; - (new_field && (field= *f_ptr)); - f_ptr++, new_field= new_field_it++, - tmp_new_field= tmp_new_field_it++) + for (f_ptr= table->field; (field= *f_ptr); f_ptr++) { - /* Make sure we have at least the default charset in use. */ - if (!new_field->charset) - new_field->charset= create_info->default_table_charset; + /* + Clear markers for renamed and indexed fields which we are going to + set later. + */ + field->flags&= ~(FIELD_IS_RENAMED|FIELD_IN_ADD_INDEX); - /* Don't pack rows in old tables if the user has requested this. */ - if (create_info->row_type == ROW_TYPE_DYNAMIC || - (tmp_new_field->flags & BLOB_FLAG) || - (tmp_new_field->sql_type == MYSQL_TYPE_VARCHAR && - create_info->row_type != ROW_TYPE_FIXED)) - create_info->table_options|= HA_OPTION_PACK_RECORD; + /* Use transformed info to evaluate flags for storage engine. */ + uint new_field_index= 0; + tmp_new_field_it.init(tmp_alter_info.create_list); + while ((tmp_new_field= tmp_new_field_it++)) + { + if (tmp_new_field->field == field) + break; + new_field_index++; + } - /* Check how fields have been modified */ - if (alter_info->flags & Alter_info::ALTER_CHANGE_COLUMN) + if (tmp_new_field) { - uint table_changes_local= 0; - /* Evaluate changes bitmap and send to check_if_incompatible_data() */ - if (!(table_changes_local= field->is_equal(tmp_new_field))) + /* Field is not dropped. Evaluate changes bitmap for it. */ + + /* + Check if type of column has changed to some incompatible type. + + We don't have separate flag for case when IS_EQUAL_PACK_LENGTH + is returned as this value is a part of legacy + handler::check_if_incompatible_data() API and almost no storage + engine uses it at the moment. + The only exception is CSV engine that treats all tables passing + SQL-layer checks as compatible. I.e. it is possible to change + length of VARCHAR() columns for CSV engine without rebuild. + TODO: Does it worth separate flag? + */ + if ((field->is_equal(tmp_new_field) != IS_EQUAL_YES)) ha_alter_info->handler_flags|= Alter_inplace_info::ALTER_COLUMN_TYPE; /* Check if field was renamed */ - field->flags&= ~FIELD_IS_RENAMED; - if (my_strcasecmp(system_charset_info, - field->field_name, + if (my_strcasecmp(system_charset_info, field->field_name, tmp_new_field->field_name)) { field->flags|= FIELD_IS_RENAMED; ha_alter_info->handler_flags|= Alter_inplace_info::ALTER_COLUMN_NAME; } - if (table_changes_local == IS_EQUAL_PACK_LENGTH) - ha_alter_info->handler_flags|= Alter_inplace_info::ALTER_COLUMN_TYPE; - /* Check that NULL behavior is same for old and new fields */ if ((tmp_new_field->flags & NOT_NULL_FLAG) != (uint) (field->flags & NOT_NULL_FLAG)) ha_alter_info->handler_flags|= Alter_inplace_info::ALTER_COLUMN_NULLABLE; + + /* + Comparing new and old default values of column is cumbersome. + So instead of using such a comparison for detecting if default + has really changed we rely on flags set by parser to get an + approximate value for storage engine flag. This is done before + looping over individual columns. + */ + + /* + Detect changes in column order. + */ + if (field->field_index != new_field_index) + ha_alter_info->handler_flags|= Alter_inplace_info::ALTER_COLUMN_ORDER; + } + else + { + /* + Field is not present in new version of table and therefore was dropped. + Corresponding storage engine flag should be already set. + */ + DBUG_ASSERT(ha_alter_info->handler_flags & Alter_inplace_info::DROP_COLUMN); } + } - /* Clear indexed marker */ - field->flags&= ~FIELD_IN_ADD_INDEX; +#ifndef DBUG_OFF + tmp_new_field_it.init(tmp_alter_info.create_list); + while ((tmp_new_field= tmp_new_field_it++)) + { + if (! tmp_new_field->field) + { + /* + Field is not present in old version of table and therefore was added. + Again corresponding storage engine flag should be already set. + */ + DBUG_ASSERT(ha_alter_info->handler_flags & Alter_inplace_info::ADD_COLUMN); + break; + } + } +#endif /* DBUG_OFF */ + + /* + Perform some modifications to original Alter_info and HA_CREATE_INFO. + + TODO: Are these changes really necessary and belong here at all? + */ + new_field_it.init(alter_info->create_list); + while ((new_field= new_field_it++)) + { + /* Make sure we have at least the default charset in use. */ + if (!new_field->charset) + new_field->charset= create_info->default_table_charset; + } + + tmp_new_field_it.init(tmp_alter_info.create_list); + while ((tmp_new_field= tmp_new_field_it++)) + { + /* Don't pack rows in old tables if the user has requested this. */ + if (create_info->row_type == ROW_TYPE_DYNAMIC || + (tmp_new_field->flags & BLOB_FLAG) || + (tmp_new_field->sql_type == MYSQL_TYPE_VARCHAR && + create_info->row_type != ROW_TYPE_FIXED)) + create_info->table_options|= HA_OPTION_PACK_RECORD; } /* @@ -5237,31 +5337,6 @@ static bool fill_alter_inplace_info(THD DBUG_PRINT("info", ("index count old: %d new: %d", table->s->keys, ha_alter_info->key_count)); - /* Count all candidate keys. */ - - for (table_key= table->key_info; table_key < table_key_end; table_key++) - { - KEY_PART_INFO *table_part; - KEY_PART_INFO *table_part_end= table_key->key_part + table_key->key_parts; - - /* - Check if key is a candidate key, i.e. a unique index with no index - fields nullable, then key is either already primary key or could - be promoted to primary key if the original primary key is dropped. - */ - not_nullable= true; - bool prefix_key= false; - for (table_part= table_key->key_part; - table_part < table_part_end; - table_part++) - { - not_nullable= not_nullable && (! table_part->field->maybe_null()); - prefix_key= prefix_key || (table_part->key_part_flag & HA_PART_KEY_SEG); - } - if ((table_key->flags & HA_NOSAME) && not_nullable && !prefix_key) - candidate_key_count++; - } - /* Step through all keys of the old table and search matching new keys. */ @@ -5283,111 +5358,44 @@ static bool fill_alter_inplace_info(THD ha_alter_info->index_drop_buffer [ha_alter_info->index_drop_count++]= table_key; - if (table_key->flags & HA_NOSAME) - { - /* Unique key. Check for "PRIMARY". */ - if ((uint) (table_key - table->key_info) == table->s->primary_key) - { - ha_alter_info->handler_flags|= Alter_inplace_info::DROP_PK_INDEX; - candidate_key_count--; - } - else - { - bool is_not_null= true; - - ha_alter_info->handler_flags|= Alter_inplace_info::DROP_UNIQUE_INDEX; - key_part= table_key->key_part; - end= key_part + table_key->key_parts; - - /* - Check if all fields in key are declared - NOT NULL and adjust candidate_key_count - */ - for(; key_part != end; key_part++) - { - is_not_null= - (is_not_null && - (!table->field[key_part->fieldnr-1]->maybe_null())); - } - if (is_not_null) - candidate_key_count--; - } - } - else - ha_alter_info->handler_flags|= Alter_inplace_info::DROP_INDEX; DBUG_PRINT("info", ("index dropped: '%s'", table_key->name)); continue; } - bool is_not_null= true; - bool no_pk= ((table->s->primary_key == MAX_KEY) || - (ha_alter_info->handler_flags & - Alter_inplace_info::DROP_PK_INDEX)); - key_part= new_key->key_part; - end= key_part + new_key->key_parts; - for(; key_part != end; key_part++) - { - /* - Check if all fields in key are declared - NOT NULL - */ - if (key_part->fieldnr < table->s->fields) - { - /* Mark field to be part of new key */ - field= table->field[key_part->fieldnr]; - field->flags|= FIELD_IN_ADD_INDEX; - is_not_null= (is_not_null && (!field->maybe_null())); - } - else - { - /* Index is defined over a newly added column */ - List_iterator_fast - new_field_it(alter_info->create_list); - Create_field *new_field; - uint fieldnr; - - for (fieldnr= 0, new_field= new_field_it++; - fieldnr != key_part->fieldnr; - fieldnr++, new_field= new_field_it++) - is_not_null= - (is_not_null && (new_field->flags & NOT_NULL_FLAG)); - } - } - /* Check that the key types are compatible between old and new tables. */ if ((table_key->algorithm != new_key->algorithm) || ((table_key->flags & HA_KEYFLAG_MASK) != (new_key->flags & HA_KEYFLAG_MASK)) || (table_key->key_parts != new_key->key_parts)) { - if (table_key->flags & HA_NOSAME) - { - /* Unique key. Check for "PRIMARY". */ - if ((uint) (table_key - table->key_info) == table->s->primary_key) - { - ha_alter_info->handler_flags|= Alter_inplace_info::DROP_PK_INDEX | - Alter_inplace_info::ADD_INDEX; - } - else - { - ha_alter_info->handler_flags|= Alter_inplace_info::DROP_UNIQUE_INDEX | - Alter_inplace_info::ADD_INDEX; - } - } - else - { - ha_alter_info->handler_flags|= Alter_inplace_info::DROP_INDEX; - if ((!my_strcasecmp(system_charset_info, - new_key->name, primary_key_name)) || - (no_pk && candidate_key_count == 0 && is_not_null && - !(new_key->flags & HA_KEY_HAS_PART_KEY_SEG))) - ha_alter_info->handler_flags|= Alter_inplace_info::ADD_PK_INDEX; - else - ha_alter_info->handler_flags|= Alter_inplace_info::ADD_UNIQUE_INDEX; - } goto index_changed; } + /* + Check that the key parts remain compatible between the old and + new tables. + */ + end= table_key->key_part + table_key->key_parts; + for (key_part= table_key->key_part, new_part= new_key->key_part; + key_part < end; + key_part++, new_part++) + { + /* + Key definition has changed if we are using a different field or + if the used key part length is different. It makes sense to + check lengths first as in case when fields differ it is likely + that lengths differ too and checking fields is more expensive + in general case. + */ + if (key_part->length != new_part->length) + goto index_changed; + + tmp_new_field= get_field_by_index(&tmp_alter_info, new_part->fieldnr); + + if (! tmp_new_field->field || tmp_new_field->field != key_part->field) + goto index_changed; + } + continue; index_changed: @@ -5398,14 +5406,12 @@ static bool fill_alter_inplace_info(THD ha_alter_info->index_add_buffer [ha_alter_info->index_add_count++]= new_key - ha_alter_info->key_info_buffer; - key_part= new_key->key_part; - end= key_part + new_key->key_parts; - for(; key_part != end; key_part++) - { - /* Mark field to be part of new key */ - if ((field= table->field[key_part->fieldnr])) - field->flags|= FIELD_IN_ADD_INDEX; - } + /* Mark all old fields which are used in newly created index. */ + /* + TODO: Check with Cluster if this step is still necessary. + It doesn't make much sense to me... + */ + mark_old_fields_in_added_key(&tmp_alter_info, new_key); DBUG_PRINT("info", ("index changed: '%s'", table_key->name)); } /*end of for (; table_key < table_key_end;) */ @@ -5425,65 +5431,93 @@ static bool fill_alter_inplace_info(THD } if (table_key >= table_key_end) { - bool is_not_null= true; - bool no_pk= ((table->s->primary_key == MAX_KEY) || - (ha_alter_info->handler_flags & - Alter_inplace_info::DROP_PK_INDEX)); - /* Key not found. Add the offset of the key to the add buffer. */ ha_alter_info->index_add_buffer [ha_alter_info->index_add_count++]= new_key - ha_alter_info->key_info_buffer; - key_part= new_key->key_part; - end= key_part + new_key->key_parts; - for(; key_part != end; key_part++) + /* Mark all old fields which are used in newly created index. */ + mark_old_fields_in_added_key(&tmp_alter_info, new_key); + DBUG_PRINT("info", ("index added: '%s'", new_key->name)); + } + } + + /* Now let us calculate flags for storage engine API. */ + + /* Count all existing candidate keys. */ + for (table_key= table->key_info; table_key < table_key_end; table_key++) + { + /* + Check if key is a candidate key, This key is either already primary key + or could be promoted to primary key if the original primary key is + dropped. + In MySQL one is allowed to create primary key with partial fields (i.e. + primary key which is not considered candidate). For simplicity we count + such key as a candidate key here. + */ + if (((uint) (table_key - table->key_info) == table->s->primary_key) || + is_candidate_key(table_key)) + candidate_key_count++; + } + + /* Figure out what kind of indexes we are dropping. */ + KEY **dropped_key; + KEY **dropped_key_end= ha_alter_info->index_drop_buffer + + ha_alter_info->index_drop_count; + + for (dropped_key= ha_alter_info->index_drop_buffer; + dropped_key < dropped_key_end; dropped_key++) + { + table_key= *dropped_key; + + if (table_key->flags & HA_NOSAME) + { + /* + Unique key. Check for PRIMARY KEY. Also see comment about primary + and candidate keys above. + */ + if ((uint) (table_key - table->key_info) == table->s->primary_key) { - /* - Check if all fields in key are declared - NOT NULL - */ - if (key_part->fieldnr < table->s->fields) - { - /* Mark field to be part of new key */ - field= table->field[key_part->fieldnr]; - field->flags|= FIELD_IN_ADD_INDEX; - is_not_null= (is_not_null && (!field->maybe_null())); - } - else - { - /* Index is defined over a newly added column */ - List_iterator_fast - new_field_it(alter_info->create_list); - Create_field *new_field; - uint fieldnr; - - for (fieldnr= 0, new_field= new_field_it++; - fieldnr != key_part->fieldnr; - fieldnr++, new_field= new_field_it++) - is_not_null= - (is_not_null && (new_field->flags & NOT_NULL_FLAG)); - } + ha_alter_info->handler_flags|= Alter_inplace_info::DROP_PK_INDEX; + candidate_key_count--; + } + else + { + ha_alter_info->handler_flags|= Alter_inplace_info::DROP_UNIQUE_INDEX; + if (is_candidate_key(table_key)) + candidate_key_count--; } - if (new_key->flags & HA_NOSAME) + } + else + ha_alter_info->handler_flags|= Alter_inplace_info::DROP_INDEX; + } + + /* Now figure out what kind of indexes we are adding. */ + for (uint add_key_idx= 0; add_key_idx < ha_alter_info->index_add_count; add_key_idx++) + { + new_key= ha_alter_info->key_info_buffer + ha_alter_info->index_add_buffer[add_key_idx]; + + if (new_key->flags & HA_NOSAME) + { + bool is_pk= !my_strcasecmp(system_charset_info, new_key->name, primary_key_name); + + if ((!(new_key->flags & HA_KEY_HAS_PART_KEY_SEG) && + !(new_key->flags & HA_NULL_PART_KEY)) || + is_pk) { - /* Unique key. Check for "PRIMARY" - or if adding first unique key - defined on non-nullable - */ - DBUG_PRINT("info",("no_pk %s, candidate_key_count %u, is_not_null %s", - (no_pk)?"yes":"no", candidate_key_count, (is_not_null)?"yes":"no")); - if ((!my_strcasecmp(system_charset_info, - new_key->name, primary_key_name)) || - (no_pk && candidate_key_count == 0 && is_not_null && - !(new_key->flags & HA_KEY_HAS_PART_KEY_SEG))) + /* Candidate key or primary key! */ + if (candidate_key_count == 0 || is_pk) ha_alter_info->handler_flags|= Alter_inplace_info::ADD_PK_INDEX; else ha_alter_info->handler_flags|= Alter_inplace_info::ADD_UNIQUE_INDEX; + candidate_key_count++; } else - ha_alter_info->handler_flags|= Alter_inplace_info::ADD_INDEX; - DBUG_PRINT("info", ("index added: '%s'", new_key->name)); + { + ha_alter_info->handler_flags|= Alter_inplace_info::ADD_UNIQUE_INDEX; + } } + else + ha_alter_info->handler_flags|= Alter_inplace_info::ADD_INDEX; } DBUG_RETURN(false); @@ -6013,15 +6047,31 @@ static bool is_inplace_alter_impossible( { DBUG_ENTER("is_inplace_alter_impossible"); - if (table->s->tmp_table) // In-place not supported for tmp tables + /* At the moment we can't handle altering temporary tables without a copy. */ + if (table->s->tmp_table) DBUG_RETURN(true); + + /* + We also test if OPTIMIZE TABLE was given and was mapped to alter table. + In that case we always do full copy (ALTER_RECREATE is set in this case). + + For the ALTER TABLE tbl_name ORDER BY ... we also want to run regular + alter table. TODO: Can be done in-place in theory? + */ if (alter_info->flags & (Alter_info::ALTER_RECREATE | Alter_info::ALTER_TABLE_REORG | Alter_info::ALTER_RENAME | Alter_info::ALTER_ORDER)) DBUG_RETURN(true); + /* + Test also that engine was not given during ALTER TABLE, or + we are force to run regular alter table (copy). + E.g. ALTER TABLE tbl_name ENGINE=MyISAM. + + TODO: Do non-engine flags can be done in-place? + */ if (alter_info->flags & Alter_info::ALTER_OPTIONS) { if (create_info->db_type != table->s->db_type() || No bundle (reason: useless for push emails).