From: Dmitry Lenev Date: July 13 2012 2:19pm Subject: bzr push into mysql-trunk branch (Dmitry.Lenev:4053 to 4054) Bug#14304973 List-Archive: http://lists.mysql.com/commits/144450 X-Bug: 14304973 Message-Id: <20120713141940.8965.28096.4054@jubjub> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 4054 Dmitry Lenev 2012-07-13 Follow-up for a fix for bug #14304973 "IN-PLACE ALTER TABLE INCORRECTLY HANDLES PREFIX KEYS". Moved code checking if index has changed in new version of table to a separate function. modified: sql/sql_table.cc 4053 Nirbhay Choubey 2012-07-13 [merge] Local merge from mysql-5.6. modified: client/mysqldump.c === modified file 'sql/sql_table.cc' --- a/sql/sql_table.cc 2012-07-11 12:05:38 +0000 +++ b/sql/sql_table.cc 2012-07-13 14:17:49 +0000 @@ -5271,6 +5271,67 @@ static Create_field *get_field_by_index( } +/** + Check if index has changed in a new version of table. + + @param alter_info Alter_info describing the changes to table + (is necessary to find correspondence between + fields in old and new version of table). + @param table_key Description of key in old version of table. + @param new_key Description of key in new version of table. + + @returns True - if index has changed, false -otherwise. +*/ + +static bool has_index_changed(Alter_info *alter_info, + const KEY *table_key, + const KEY *new_key) +{ + const KEY_PART_INFO *key_part, *new_part, *end; + const Create_field *new_field; + + /* 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)) + return true; + + /* + 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) + return true; + + new_field= get_field_by_index(alter_info, new_part->fieldnr); + + /* + For prefix keys KEY_PART_INFO::field points to cloned Field + object with adjusted length. So below we have to check field + indexes instead of simply comparing pointers to Field objects. + */ + if (! new_field->field || + new_field->field->field_index != key_part->fieldnr - 1) + return true; + } + + return false; +} + + static int compare_uint(const uint *s, const uint *t) { return (*s < *t) ? -1 : ((*s > *t) ? 1 : 0); @@ -5327,8 +5388,6 @@ static bool fill_alter_inplace_info(THD Field **f_ptr, *field; List_iterator_fast new_field_it; Create_field *new_field; - KEY_PART_INFO *key_part, *new_part; - KEY_PART_INFO *end; uint candidate_key_count= 0; Alter_info *alter_info= ha_alter_info->alter_info; DBUG_ENTER("fill_alter_inplace_info"); @@ -5542,55 +5601,17 @@ static bool fill_alter_inplace_info(THD continue; } - /* 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)) - 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++) + if (has_index_changed(alter_info, table_key, new_key)) { - /* - 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; - - new_field= get_field_by_index(alter_info, new_part->fieldnr); - - /* - For prefix keys KEY_PART_INFO::field points to cloned Field - object with adjusted length. So below we have to check field - indexes instead of simply comparing pointers to Field objects. - */ - if (! new_field->field || - new_field->field->field_index != key_part->fieldnr - 1) - goto index_changed; + /* Key modified. Add the key / key offset to both buffers. */ + ha_alter_info->index_drop_buffer + [ha_alter_info->index_drop_count++]= + table_key; + ha_alter_info->index_add_buffer + [ha_alter_info->index_add_count++]= + new_key - ha_alter_info->key_info_buffer; + DBUG_PRINT("info", ("index changed: '%s'", table_key->name)); } - continue; - - index_changed: - /* Key modified. Add the key / key offset to both buffers. */ - ha_alter_info->index_drop_buffer - [ha_alter_info->index_drop_count++]= - table_key; - ha_alter_info->index_add_buffer - [ha_alter_info->index_add_count++]= - new_key - ha_alter_info->key_info_buffer; - /* Mark all old fields which are used in newly created index. */ - DBUG_PRINT("info", ("index changed: '%s'", table_key->name)); } /*end of for (; table_key < table_key_end;) */ No bundle (reason: useless for push emails).