From: Dmitry Lenev Date: June 21 2012 9:50am Subject: bzr push into mysql-trunk branch (Dmitry.Lenev:4047 to 4048) Bug#14156617 List-Archive: http://lists.mysql.com/commits/144294 X-Bug: 14156617 Message-Id: <20120621095034.20088.81500.4048@jubjub> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 4048 Dmitry Lenev 2012-06-21 Fix for bug #14156617 "INPLACE ALTER: NOT SUPPORTED ALTER TABLE ... ADD PARTITION CAUSES CRASH". Attempt to add a new partition to a partitioned NDB table led to crash due to debug assertion in server built from mysql-trunk-cluster tree. This also caused ndb_partition_range test failures in this tree. The assertion failure occurred due to fact that prep_alter_part_table() has added new partition partition_info object for TABLE instance representing old version of table. Since NDB tables do not support "fast" changes of partitioning, after that we tried to carry out ALTER TABLE by copying data from old version of table to the new version. As in the process we tried to use TABLE object with modified partition_info for reading data, which resulted in assertion failure due to discrepancy between real table structure and its description in partition_info. This patch solves the problem by ensuring that we don't modify partition_info object for TABLE instances representing old version of table in cases when "fast" changes of partitioning is not going to be used. Instead we create a copy of partition_info, adjust it and use for .FRM/.PAR files creation. Unfortunately we can't easily use this copy in all cases, since current "fast" partitioning API assumes that changes to partitioning are communicated to it through TABLE::part_info object. There is no test case as bug is not repeatable without NDB (and thus not repeatable in trunk) and mysql-trunk-cluster tree already contains ndb_partition_range.test which provides test coverage for this bug. modified: sql/sql_partition.cc 4047 viswanatham gudipati 2012-06-21 WL5980: Added a perl for copying the zip file modified: mysql-test/suite/innodb/t/innodb-wl5980-linux.test === modified file 'sql/sql_partition.cc' --- a/sql/sql_partition.cc 2012-06-15 15:32:44 +0000 +++ b/sql/sql_partition.cc 2012-06-21 09:50:01 +0000 @@ -4763,9 +4763,6 @@ uint prep_alter_part_table(THD *thd, TAB alter_ctx->table_name, MDL_INTENTION_EXCLUSIVE)); - /* We change the part_info, so the table must be reopened. */ - table->m_needs_reopen= true; - tab_part_info= table->part_info; if (alter_info->flags & Alter_info::ALTER_TABLE_REORG) @@ -4787,6 +4784,8 @@ uint prep_alter_part_table(THD *thd, TAB without any changes at all. */ *fast_alter_table= true; + /* Force table re-open for consistency with the main case. */ + table->m_needs_reopen= true; thd->work_part_info= tab_part_info; DBUG_RETURN(FALSE); } @@ -4815,7 +4814,28 @@ uint prep_alter_part_table(THD *thd, TAB goto err; } if ((flags & (HA_FAST_CHANGE_PARTITION | HA_PARTITION_ONE_PHASE)) != 0) + { + /* + "Fast" change of partitioning is supported in this case. + We will change TABLE::part_info (as this is how we pass + information to storage engine in this case), so the table + must be reopened. + */ *fast_alter_table= true; + table->m_needs_reopen= true; + } + else + { + /* + "Fast" changing of partitioning is not supported. Create + a copy of TABLE::part_info object, so we can modify it safely. + Modifying original TABLE::part_info will cause problems when + we read data from old version of table using this TABLE object + while copying them to new version of table. + */ + if (!(tab_part_info= tab_part_info->get_clone())) + DBUG_RETURN(TRUE); + } DBUG_PRINT("info", ("*fast_alter_table flags: 0x%x", flags)); if ((alter_info->flags & Alter_info::ALTER_ADD_PARTITION) || (alter_info->flags & Alter_info::ALTER_REORGANIZE_PARTITION)) @@ -5577,7 +5597,9 @@ the generated partition syntax in a corr There was no partitioning before and no partitioning defined. Obviously no work needed. */ - if (table->part_info) + partition_info *tab_part_info= table->part_info; + + if (tab_part_info) { if (alter_info->flags & Alter_info::ALTER_REMOVE_PARTITIONING) { @@ -5585,7 +5607,7 @@ the generated partition syntax in a corr if (!(create_info->used_fields & HA_CREATE_USED_ENGINE)) { DBUG_PRINT("info", ("No explicit engine used")); - create_info->db_type= table->part_info->default_engine_type; + create_info->db_type= tab_part_info->default_engine_type; } DBUG_PRINT("info", ("New engine type: %s", ha_resolve_storage_engine_name(create_info->db_type))); @@ -5597,16 +5619,20 @@ the generated partition syntax in a corr /* Retain partitioning but possibly with a new storage engine beneath. + + Create a copy of TABLE::part_info to be able to modify it freely. */ - thd->work_part_info= table->part_info; + if (!(tab_part_info= tab_part_info->get_clone())) + DBUG_RETURN(TRUE); + thd->work_part_info= tab_part_info; if (create_info->used_fields & HA_CREATE_USED_ENGINE && - create_info->db_type != table->part_info->default_engine_type) + create_info->db_type != tab_part_info->default_engine_type) { /* Make sure change of engine happens to all partitions. */ DBUG_PRINT("info", ("partition changed")); - if (table->part_info->is_auto_partitioned) + if (tab_part_info->is_auto_partitioned) { /* If the user originally didn't specify partitioning to be @@ -5634,7 +5660,7 @@ the generated partition syntax in a corr Need to cater for engine types that can handle partition without using the partition handler. */ - if (thd->work_part_info != table->part_info) + if (thd->work_part_info != tab_part_info) { DBUG_PRINT("info", ("partition changed")); *partition_changed= TRUE; @@ -5651,8 +5677,8 @@ the generated partition syntax in a corr part_info->default_engine_type= create_info->db_type; else { - if (table->part_info) - part_info->default_engine_type= table->part_info->default_engine_type; + if (tab_part_info) + part_info->default_engine_type= tab_part_info->default_engine_type; else part_info->default_engine_type= create_info->db_type; } No bundle (reason: useless for push emails).