Hello!
On 06/09/2011 12:35 PM, Jorgen Loland wrote:
> 3172 Jorgen Loland 2011-06-09
> BUG#11882110: UPDATE REPORTS ER_KEY_NOT_FOUND IF TABLE IS
> UPDATED TWICE
Patch approved, but please consider the comments below.
> For multi update it is not allowed to update certain columns
certain columns? Please clarify this comment.
> of a table if that table is accessed through multiple aliases
> if either
> 1) one of the updated columns is used as partitioning key
> 2) one of the updated columns is part of the primary key
> and the primary key is clustered
>
> This check is done in unsafe_key_update().
>
> The bug was that for case 2), it was checked whether
> updated_column_number == table_share->primary_key
> However, the primary_key variable is the index number of the
> primary key, not a column number.
Consider including a sentence or two about what the visible effects
of this bug was to users. Also mention that the issue was specific to
InnoDB and why.
> The columns covered by an index is found in
> table->key_info[idx_number]->key_part. The bugfix is to check
> if any of the columns in the keyparts of the primary key is
> updated.
> @ mysql-test/r/multi_update.result
> Add test for bug#11882110
I think it should be mentioned that this MyISAM test is added to
document existing behavior rather than serve as a regression test
for this bug. Please also include this as a comment in the test
case itself.
> @ mysql-test/r/multi_update_innodb.result
> Add test for bug#11882110
> @ mysql-test/t/multi_update.test
> Add test for bug#11882110
> @ mysql-test/t/multi_update_innodb.test
> Add test for bug#11882110
> @ sql/sql_update.cc
> unsafe_key_update() wrongly checked if the primary key index number was the
> same as updated column number. Now it is checked whether any of the columns making up the
> primary key is updated.
Long line.
> @ sql/table.h
> Remove comment that has been introduced by a merge conflict (as per dlenev)
>
> modified:
> mysql-test/r/multi_update.result
> mysql-test/r/multi_update_innodb.result
> mysql-test/t/multi_update.test
> mysql-test/t/multi_update_innodb.test
> sql/sql_update.cc
> sql/table.h
> === modified file 'sql/sql_update.cc'
> --- a/sql/sql_update.cc 2011-06-09 08:58:41 +0000
> +++ b/sql/sql_update.cc 2011-06-09 10:35:26 +0000
> @@ -1069,17 +1069,27 @@ bool unsafe_key_update(TABLE_LIST *leave
> return true;
> }
>
> - if (primkey_clustered&&
> - (bitmap_is_set(table1->write_set, table1->s->primary_key) ||
> - bitmap_is_set(table2->write_set, table2->s->primary_key)))
> - {
> - // Clustered primary key is updated
> - my_error(ER_MULTI_UPDATE_KEY_CONFLICT, MYF(0),
> - tl->belong_to_view ? tl->belong_to_view->alias
> - : tl->alias,
> - tl2->belong_to_view ? tl2->belong_to_view->alias
> - : tl2->alias);
> - return true;
> + if (primkey_clustered)
> + {
Trailing space.
> + // The primary key can cover multiple columns
> + KEY key_info= table1->key_info[table1->s->primary_key];
> + KEY_PART_INFO *key_part= key_info.key_part;
> + KEY_PART_INFO *key_part_end= key_part + key_info.key_parts;
> +
> + for (;key_part != key_part_end; ++key_part)
> + {
> + if (bitmap_is_set(table1->write_set, key_part->fieldnr-1) ||
> + bitmap_is_set(table2->write_set, key_part->fieldnr-1))
> + {
> + // Clustered primary key is updated
> + my_error(ER_MULTI_UPDATE_KEY_CONFLICT, MYF(0),
> + tl->belong_to_view ? tl->belong_to_view->alias
> + : tl->alias,
> + tl2->belong_to_view ? tl2->belong_to_view->alias
> + : tl2->alias);
> + return true;
> + }
> + }
> }
> }
> }
>
> === modified file 'sql/table.h'
> --- a/sql/table.h 2011-05-26 15:20:09 +0000
> +++ b/sql/table.h 2011-06-09 10:35:26 +0000
> @@ -669,7 +669,6 @@ struct TABLE_SHARE
> uint db_options_in_use; /* Options in use */
> uint db_record_offset; /* if HA_REC_IN_SEQ */
> uint rowid_field_offset; /* Field_nr +1 to rowid field */
> - /* Index of auto-updated TIMESTAMP field in field array */
> uint primary_key;
> uint next_number_index; /* autoincrement key number */
> uint next_number_key_offset; /* autoinc keypart offset in a key */
Maybe add a comment explaining primary_key? Mention that this is the
index number and not the column number :-)
Thanks,
--- Jon Olav
