diff options
author | Mattias Jonsson <mattias.jonsson@oracle.com> | 2012-02-02 12:47:17 +0100 |
---|---|---|
committer | Mattias Jonsson <mattias.jonsson@oracle.com> | 2012-02-02 12:47:17 +0100 |
commit | 7ebeb1433e567dece2601651d306448408656da8 (patch) | |
tree | 2954bdaac8f88095fc3a3d0ebf2b9980bf022cda | |
parent | 225f0cd53d547927dc6d528a4e78c1be5e07cbc6 (diff) | |
download | mariadb-git-7ebeb1433e567dece2601651d306448408656da8.tar.gz |
Bug#13593865 - 64037: CRASH IN HA_PARTITION::CREATE_HANDLERS ON
ALTER TABLE AFTER DROP PARTITION
Bug#13608188 - 64038: CRASH IN HANDLER::HA_THD ON ALTER TABLE AFTER
REPAIR NON-EXISTING PARTITION
Backport of bug#13357766 from -trunk to -5.5.
The state of some partitions was not reset on failure, leading
to invalid states of partitions in consequent statements.
Fixed by reverting back to original state for all partitions
if not all partition names was resolved.
Also adding extra security by forcing tables to be reopened
in case of error in mysql_alter_table.
(There is also removal of \r at the end of some lines.)
-rw-r--r-- | mysql-test/r/partition_error.result | 26 | ||||
-rw-r--r-- | mysql-test/t/partition_error.test | 28 | ||||
-rw-r--r-- | sql/ha_partition.cc | 76 | ||||
-rw-r--r-- | sql/sql_admin.cc | 10 | ||||
-rw-r--r-- | sql/sql_partition.cc | 39 | ||||
-rw-r--r-- | sql/sql_partition.h | 2 |
6 files changed, 124 insertions, 57 deletions
diff --git a/mysql-test/r/partition_error.result b/mysql-test/r/partition_error.result index 252a6b4386a..0e4f89b70db 100644 --- a/mysql-test/r/partition_error.result +++ b/mysql-test/r/partition_error.result @@ -1,5 +1,31 @@ drop table if exists t1, t2; # +# Bug#13608188 - 64038: CRASH IN HANDLER::HA_THD ON ALTER TABLE AFTER +# REPAIR NON-EXISTING PARTITION +# +CREATE TABLE t1 ( a INT, b INT ); +INSERT INTO t1 VALUES (5,3),(5,6); +ALTER TABLE t1 PARTITION BY KEY(b) PARTITIONS 3 ; +ALTER TABLE t1 REPAIR PARTITION p1, p3; +Table Op Msg_type Msg_text +test.t1 repair error Error in list of partitions to test.t1 +ALTER TABLE t1 ORDER BY b; +DROP TABLE t1; +# +# Bug#13593865 - 64037: CRASH IN HA_PARTITION::CREATE_HANDLERS ON +# ALTER TABLE AFTER DROP PARTITION +# +CREATE TABLE t1 (a INT) +PARTITION BY RANGE (a) +(PARTITION p0 VALUES LESS THAN (0), +PARTITION p1 VALUES LESS THAN MAXVALUE ) ; +ALTER TABLE t1 DROP PARTITION p1; +ALTER TABLE t1 ANALYZE PARTITION p0, p1; +Table Op Msg_type Msg_text +test.t1 analyze error Error in list of partitions to test.t1 +ALTER TABLE t1 COMMENT 'altered'; +DROP TABLE t1; +# # Bug#57924: crash when creating partitioned table with # multiple columns in the partition key # diff --git a/mysql-test/t/partition_error.test b/mysql-test/t/partition_error.test index 536935420a4..c0f49a4d414 100644 --- a/mysql-test/t/partition_error.test +++ b/mysql-test/t/partition_error.test @@ -11,6 +11,34 @@ drop table if exists t1, t2; let $MYSQLD_DATADIR= `SELECT @@datadir`; --echo # +--echo # Bug#13608188 - 64038: CRASH IN HANDLER::HA_THD ON ALTER TABLE AFTER +--echo # REPAIR NON-EXISTING PARTITION +--echo # +CREATE TABLE t1 ( a INT, b INT ); +INSERT INTO t1 VALUES (5,3),(5,6); +ALTER TABLE t1 PARTITION BY KEY(b) PARTITIONS 3 ; +ALTER TABLE t1 REPAIR PARTITION p1, p3; +ALTER TABLE t1 ORDER BY b; +DROP TABLE t1; + +--echo # +--echo # Bug#13593865 - 64037: CRASH IN HA_PARTITION::CREATE_HANDLERS ON +--echo # ALTER TABLE AFTER DROP PARTITION +--echo # + +CREATE TABLE t1 (a INT) +PARTITION BY RANGE (a) +(PARTITION p0 VALUES LESS THAN (0), + PARTITION p1 VALUES LESS THAN MAXVALUE ) ; + +ALTER TABLE t1 DROP PARTITION p1; +ALTER TABLE t1 ANALYZE PARTITION p0, p1; + +ALTER TABLE t1 COMMENT 'altered'; + +DROP TABLE t1; + +--echo # --echo # Bug#57924: crash when creating partitioned table with --echo # multiple columns in the partition key --echo # diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc index fdaa1b0cda6..b78e2be9664 100644 --- a/sql/ha_partition.cc +++ b/sql/ha_partition.cc @@ -3607,14 +3607,14 @@ int ha_partition::truncate_partition(Alter_info *alter_info, bool *binlog_stmt) uint num_parts= m_part_info->num_parts; uint num_subparts= m_part_info->num_subparts; uint i= 0; - uint num_parts_set= alter_info->partition_names.elements; - uint num_parts_found= set_part_state(alter_info, m_part_info, - PART_ADMIN); DBUG_ENTER("ha_partition::truncate_partition"); /* Only binlog when it starts any call to the partitions handlers */ *binlog_stmt= false; + if (set_part_state(alter_info, m_part_info, PART_ADMIN)) + DBUG_RETURN(HA_ERR_NO_PARTITION_FOUND); + /* TRUNCATE also means resetting auto_increment. Hence, reset it so that it will be initialized again at the next use. @@ -3624,10 +3624,6 @@ int ha_partition::truncate_partition(Alter_info *alter_info, bool *binlog_stmt) table_share->ha_part_data->auto_inc_initialized= FALSE; unlock_auto_increment(); - if (num_parts_set != num_parts_found && - (!(alter_info->flags & ALTER_ALL_PARTITION))) - DBUG_RETURN(HA_ERR_NO_PARTITION_FOUND); - *binlog_stmt= true; do @@ -6059,7 +6055,7 @@ int ha_partition::extra(enum ha_extra_function operation) 0 Success DESCRIPTION - Called at end of each statement to reste buffers + Called at end of each statement to reset buffers */ int ha_partition::reset(void) @@ -6792,8 +6788,8 @@ int ha_partition::final_add_index(handler_add_index *add, bool commit) DBUG_RETURN(ret); err: uint j; - uint *key_numbers= NULL;
- KEY *old_key_info= NULL;
+ uint *key_numbers= NULL; + KEY *old_key_info= NULL; uint num_of_keys= 0; int error; @@ -6803,27 +6799,27 @@ err: if (i > 0) { num_of_keys= part_add_index->num_of_keys; - key_numbers= (uint*) ha_thd()->alloc(sizeof(uint) * num_of_keys);
- if (!key_numbers)
- {
- sql_print_error("Failed with error handling of adding index:\n"
- "committing index failed, and when trying to revert "
- "already committed partitions we failed allocating\n"
- "memory for the index for table '%s'",
- table_share->table_name.str);
- DBUG_RETURN(HA_ERR_OUT_OF_MEM);
- }
- old_key_info= table->key_info;
- /*
- Use the newly added key_info as table->key_info to remove them.
- Note that this requires the subhandlers to use name lookup of the
- index. They must use given table->key_info[key_number], they cannot
- use their local view of the keys, since table->key_info only include
- the indexes to be removed here.
- */
- for (j= 0; j < num_of_keys; j++)
- key_numbers[j]= j;
- table->key_info= part_add_index->key_info;
+ key_numbers= (uint*) ha_thd()->alloc(sizeof(uint) * num_of_keys); + if (!key_numbers) + { + sql_print_error("Failed with error handling of adding index:\n" + "committing index failed, and when trying to revert " + "already committed partitions we failed allocating\n" + "memory for the index for table '%s'", + table_share->table_name.str); + DBUG_RETURN(HA_ERR_OUT_OF_MEM); + } + old_key_info= table->key_info; + /* + Use the newly added key_info as table->key_info to remove them. + Note that this requires the subhandlers to use name lookup of the + index. They must use given table->key_info[key_number], they cannot + use their local view of the keys, since table->key_info only include + the indexes to be removed here. + */ + for (j= 0; j < num_of_keys; j++) + key_numbers[j]= j; + table->key_info= part_add_index->key_info; } for (j= 0; j < m_tot_parts; j++) @@ -6831,15 +6827,15 @@ err: if (j < i) { /* Remove the newly added index */ - error= m_file[j]->prepare_drop_index(table, key_numbers, num_of_keys);
- if (error || m_file[j]->final_drop_index(table))
- {
- sql_print_error("Failed with error handling of adding index:\n"
- "committing index failed, and when trying to revert "
- "already committed partitions we failed removing\n"
- "the index for table '%s' partition nr %d",
- table_share->table_name.str, j);
- }
+ error= m_file[j]->prepare_drop_index(table, key_numbers, num_of_keys); + if (error || m_file[j]->final_drop_index(table)) + { + sql_print_error("Failed with error handling of adding index:\n" + "committing index failed, and when trying to revert " + "already committed partitions we failed removing\n" + "the index for table '%s' partition nr %d", + table_share->table_name.str, j); + } } else if (j > i) { diff --git a/sql/sql_admin.cc b/sql/sql_admin.cc index 5bb777437b0..8f1b24741f6 100644 --- a/sql/sql_admin.cc +++ b/sql/sql_admin.cc @@ -406,12 +406,7 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, my_error(ER_PARTITION_MGMT_ON_NONPARTITIONED, MYF(0)); DBUG_RETURN(TRUE); } - uint num_parts_found; - uint num_parts_opt= alter_info->partition_names.elements; - num_parts_found= set_part_state(alter_info, table->table->part_info, - PART_ADMIN); - if (num_parts_found != num_parts_opt && - (!(alter_info->flags & ALTER_ALL_PARTITION))) + if (set_part_state(alter_info, table->table->part_info, PART_ADMIN)) { char buff[FN_REFLEN + MYSQL_ERRMSG_SIZE]; size_t length; @@ -872,6 +867,9 @@ send_result_message: DBUG_RETURN(FALSE); err: + /* Make sure this table instance is not reused after the failure. */ + if (table && table->table) + table->table->m_needs_reopen= true; trans_rollback_stmt(thd); trans_rollback(thd); close_thread_tables(thd); // Shouldn't be needed diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc index 354548cbda4..8f946b02747 100644 --- a/sql/sql_partition.cc +++ b/sql/sql_partition.cc @@ -4469,11 +4469,20 @@ error: } -/* - Sets which partitions to be used in the command +/** + Sets which partitions to be used in the command. + + @param alter_info Alter_info pointer holding partition names and flags. + @param tab_part_info partition_info holding all partitions. + @param part_state Which state to set for the named partitions. + + @return Operation status + @retval false Success + @retval true Failure */ -uint set_part_state(Alter_info *alter_info, partition_info *tab_part_info, - enum partition_state part_state) + +bool set_part_state(Alter_info *alter_info, partition_info *tab_part_info, + enum partition_state part_state) { uint part_count= 0; uint num_parts_found= 0; @@ -4499,7 +4508,21 @@ uint set_part_state(Alter_info *alter_info, partition_info *tab_part_info, else part_elem->part_state= PART_NORMAL; } while (++part_count < tab_part_info->num_parts); - return num_parts_found; + + if (num_parts_found != alter_info->partition_names.elements && + !(alter_info->flags & ALTER_ALL_PARTITION)) + { + /* Not all given partitions found, revert and return failure */ + part_it.rewind(); + part_count= 0; + do + { + partition_element *part_elem= part_it++; + part_elem->part_state= PART_NORMAL; + } while (++part_count < tab_part_info->num_parts); + return true; + } + return false; } @@ -5018,11 +5041,7 @@ that are reorganised. } else if (alter_info->flags & ALTER_REBUILD_PARTITION) { - uint num_parts_found; - uint num_parts_opt= alter_info->partition_names.elements; - num_parts_found= set_part_state(alter_info, tab_part_info, PART_CHANGED); - if (num_parts_found != num_parts_opt && - (!(alter_info->flags & ALTER_ALL_PARTITION))) + if (set_part_state(alter_info, tab_part_info, PART_CHANGED)) { my_error(ER_DROP_PARTITION_NON_EXISTENT, MYF(0), "REBUILD"); goto err; diff --git a/sql/sql_partition.h b/sql/sql_partition.h index 7a7b73e9f01..998e4b25f0b 100644 --- a/sql/sql_partition.h +++ b/sql/sql_partition.h @@ -254,7 +254,7 @@ uint fast_alter_partition_table(THD *thd, TABLE *table, char *db, const char *table_name, TABLE *fast_alter_table); -uint set_part_state(Alter_info *alter_info, partition_info *tab_part_info, +bool set_part_state(Alter_info *alter_info, partition_info *tab_part_info, enum partition_state part_state); uint prep_alter_part_table(THD *thd, TABLE *table, Alter_info *alter_info, HA_CREATE_INFO *create_info, |