# This is a BitKeeper generated diff -Nru style patch. # # ChangeSet # 2008/01/28 15:20:55+01:00 mikael@dator6.(none) # BUG#32943: Fixed buggy lock handling of ALTER TABLE for partitioning # # mysql-test/r/partition_range.result # 2008/01/28 15:20:52+01:00 mikael@dator6.(none) +22 -0 # Added new test cases for lock tables and ALTER TABLE for # partitions, also added a test case with a trigger. # # mysql-test/t/partition_range.test # 2008/01/28 15:20:52+01:00 mikael@dator6.(none) +35 -0 # Added new test cases for lock tables and ALTER TABLE for # partitions, also added a test case with a trigger. # # sql/mysql_priv.h # 2008/01/28 15:20:52+01:00 mikael@dator6.(none) +1 -0 # Added WFRM_KEEP_SHARE for use of code not to be used otherwise # # sql/sql_partition.cc # 2008/01/28 15:20:52+01:00 mikael@dator6.(none) +44 -70 # Removed get_name_lock and release_name_lock, use # close_data_files_and_morph_locks which leaves an # exclusive name lock after completing. # Reopen table after completing if under lock tables # Updated comments # # sql/sql_table.cc # 2008/01/28 15:20:52+01:00 mikael@dator6.(none) +7 -3 # Ensure that code to set partition syntax isn't used other than # when specifically asked to do it. # diff -Nru a/mysql-test/r/partition_range.result b/mysql-test/r/partition_range.result --- a/mysql-test/r/partition_range.result 2008-01-28 16:07:33 +01:00 +++ b/mysql-test/r/partition_range.result 2008-01-28 16:07:33 +01:00 @@ -1,4 +1,26 @@ drop table if exists t1, t2; +create table t1 (a integer) +partition by range (a) +( partition p0 values less than (4), +partition p1 values less than (100)); +create trigger tr1 before insert on t1 +for each row begin +set @a = 1; +end| +alter table t1 drop partition p0; +drop table t1; +create table t1 (a integer) +partition by range (a) +( partition p0 values less than (4), +partition p1 values less than (100)); +LOCK TABLES t1 WRITE; +alter table t1 drop partition p0; +alter table t1 reorganize partition p1 into +( partition p0 values less than (4), +partition p1 values less than (100)); +alter table t1 add partition ( partition p2 values less than (200)); +UNLOCK TABLES; +drop table t1; create table t1 (a int unsigned) partition by range (a) (partition pnull values less than (0), diff -Nru a/mysql-test/t/partition_range.test b/mysql-test/t/partition_range.test --- a/mysql-test/t/partition_range.test 2008-01-28 16:07:33 +01:00 +++ b/mysql-test/t/partition_range.test 2008-01-28 16:07:33 +01:00 @@ -10,6 +10,41 @@ --enable_warnings # +# BUG 32943: +# Locking problems in relation to partitioning and triggers +# Also fixes and test cases of generic lock issues with +# partition change code. +# +create table t1 (a integer) +partition by range (a) +( partition p0 values less than (4), + partition p1 values less than (100)); + +delimiter |; +create trigger tr1 before insert on t1 +for each row begin + set @a = 1; +end| + +delimiter ;| +alter table t1 drop partition p0; + +drop table t1; + +create table t1 (a integer) +partition by range (a) +( partition p0 values less than (4), + partition p1 values less than (100)); +LOCK TABLES t1 WRITE; +alter table t1 drop partition p0; +alter table t1 reorganize partition p1 into +( partition p0 values less than (4), + partition p1 values less than (100)); +alter table t1 add partition ( partition p2 values less than (200)); +UNLOCK TABLES; +drop table t1; + +# # BUG 18198: Various tests for partition functions # #create table t1 (a varchar(10) charset latin1 collate latin1_bin, b int) diff -Nru a/sql/mysql_priv.h b/sql/mysql_priv.h --- a/sql/mysql_priv.h 2008-01-28 16:07:33 +01:00 +++ b/sql/mysql_priv.h 2008-01-28 16:07:33 +01:00 @@ -1635,6 +1635,7 @@ #define WFRM_WRITE_SHADOW 1 #define WFRM_INSTALL_SHADOW 2 #define WFRM_PACK_FRM 4 +#define WFRM_KEEP_SHARE 8 bool mysql_write_frm(ALTER_PARTITION_PARAM_TYPE *lpt, uint flags); int abort_and_upgrade_lock(ALTER_PARTITION_PARAM_TYPE *lpt); void close_open_tables_and_downgrade(ALTER_PARTITION_PARAM_TYPE *lpt); diff -Nru a/sql/sql_partition.cc b/sql/sql_partition.cc --- a/sql/sql_partition.cc 2008-01-28 16:07:33 +01:00 +++ b/sql/sql_partition.cc 2008-01-28 16:07:33 +01:00 @@ -5838,32 +5838,32 @@ /* - Get a lock on table name to avoid that anyone can open the table in - a critical part of the ALTER TABLE. - SYNOPSIS - get_name_lock() + Final part of partition changes to handle things when under + LOCK TABLES. + SYNPOSIS + alter_partition_lock_handling() lpt Struct carrying parameters RETURN VALUES - FALSE Success - TRUE Failure + NONE */ - -static int get_name_lock(ALTER_PARTITION_PARAM_TYPE *lpt) +void alter_partition_lock_handling(ALTER_PARTITION_PARAM_TYPE *lpt) { - int error= 0; - DBUG_ENTER("get_name_lock"); - - bzero(&lpt->table_list, sizeof(lpt->table_list)); - lpt->table_list.db= (char*)lpt->db; - lpt->table_list.table= lpt->table; - lpt->table_list.table_name= (char*)lpt->table_name; - pthread_mutex_lock(&LOCK_open); - error= lock_table_name(lpt->thd, &lpt->table_list, FALSE); - pthread_mutex_unlock(&LOCK_open); - DBUG_RETURN(error); + int err; + if (lpt->thd->locked_tables) + { + pthread_mutex_lock(&LOCK_open); + lpt->thd->in_lock_tables= 1; + err= reopen_tables(lpt->thd, 1, 1); + lpt->thd->in_lock_tables= 0; + pthread_mutex_unlock(&LOCK_open); + if (err) + { + /* Issue a warning since we weren't able to regain the lock again. */ + sql_print_warning("We failed to reacquire LOCKs in ALTER TABLE"); + } + } } - /* Unlock and close table before renaming and dropping partitions SYNOPSIS @@ -5876,35 +5876,16 @@ static int alter_close_tables(ALTER_PARTITION_PARAM_TYPE *lpt) { THD *thd= lpt->thd; - TABLE *table= lpt->table; + const char *db= lpt->db; + const char *table_name= lpt->table_name; DBUG_ENTER("alter_close_tables"); /* We need to also unlock tables and close all handlers. We set lock to zero to ensure we don't do this twice and we set db_stat to zero to ensure we don't close twice. */ - mysql_unlock_tables(thd, thd->lock); - thd->lock= 0; - table->file->close(); - table->db_stat= 0; - DBUG_RETURN(0); -} - - -/* - Release a lock name - SYNOPSIS - release_name_lock() - lpt - RETURN VALUES - 0 -*/ - -static int release_name_lock(ALTER_PARTITION_PARAM_TYPE *lpt) -{ - DBUG_ENTER("release_name_lock"); pthread_mutex_lock(&LOCK_open); - unlock_table_name(lpt->thd, &lpt->table_list); + close_data_files_and_morph_locks(thd, db, table_name); pthread_mutex_unlock(&LOCK_open); DBUG_RETURN(0); } @@ -6202,7 +6183,7 @@ name lock. 5) Close all tables that have already been opened but didn't stumble on the abort locked previously. This is done as part of the - get_name_lock call. + close_data_files_and_morph_locks call. 6) We are now ready to release all locks we got in this thread. 7) Write the bin log Unfortunately the writing of the binlog is not synchronised with @@ -6219,8 +6200,7 @@ 9) Prepare handlers for drop of partitions 10) Drop the partitions 11) Remove entries from ddl log - 12) Release name lock so that all other threads can access the table - again. + 12) Reopen table if under lock tables 13) Complete query We insert Error injections at all places where it could be interesting @@ -6235,23 +6215,21 @@ (not_completed= FALSE) || abort_and_upgrade_lock(lpt) || /* Always returns 0 */ ERROR_INJECT_CRASH("crash_drop_partition_4") || - get_name_lock(lpt) || - ERROR_INJECT_CRASH("crash_drop_partition_5") || alter_close_tables(lpt) || - ERROR_INJECT_CRASH("crash_drop_partition_6") || + ERROR_INJECT_CRASH("crash_drop_partition_5") || ((!thd->lex->no_write_to_binlog) && (write_bin_log(thd, FALSE, thd->query, thd->query_length), FALSE)) || - ERROR_INJECT_CRASH("crash_drop_partition_7") || + ERROR_INJECT_CRASH("crash_drop_partition_6") || ((frm_install= TRUE), FALSE) || mysql_write_frm(lpt, WFRM_INSTALL_SHADOW) || ((frm_install= FALSE), FALSE) || - ERROR_INJECT_CRASH("crash_drop_partition_8") || + ERROR_INJECT_CRASH("crash_drop_partition_7") || mysql_drop_partitions(lpt) || - ERROR_INJECT_CRASH("crash_drop_partition_9") || + ERROR_INJECT_CRASH("crash_drop_partition_8") || (write_log_completed(lpt, FALSE), FALSE) || - ERROR_INJECT_CRASH("crash_drop_partition_10") || - (release_name_lock(lpt), FALSE)) + ERROR_INJECT_CRASH("crash_drop_partition_9") || + (alter_partition_lock_handling(lpt), FALSE)) { handle_alter_part_error(lpt, not_completed, TRUE, frm_install); goto err; @@ -6283,7 +6261,7 @@ name lock. 5) Close all tables that have already been opened but didn't stumble on the abort locked previously. This is done as part of the - get_name_lock call. + close_data_files_and_morph_locks call. 6) Close all table handlers and unlock all handlers but retain name lock 7) Write binlog 8) Now the change is completed except for the installation of the @@ -6293,7 +6271,7 @@ added to the table. 10)Wait until all accesses using the old frm file has completed 11)Remove entries from ddl log - 12)Release name lock + 12)Reopen tables if under lock tables 13)Complete query */ if (write_log_add_change_partition(lpt) || @@ -6303,8 +6281,6 @@ mysql_change_partitions(lpt) || ERROR_INJECT_CRASH("crash_add_partition_3") || abort_and_upgrade_lock(lpt) || /* Always returns 0 */ - ERROR_INJECT_CRASH("crash_add_partition_3") || - get_name_lock(lpt) || ERROR_INJECT_CRASH("crash_add_partition_4") || alter_close_tables(lpt) || ERROR_INJECT_CRASH("crash_add_partition_5") || @@ -6320,7 +6296,7 @@ ERROR_INJECT_CRASH("crash_add_partition_8") || (write_log_completed(lpt, FALSE), FALSE) || ERROR_INJECT_CRASH("crash_add_partition_9") || - (release_name_lock(lpt), FALSE)) + (alter_partition_lock_handling(lpt), FALSE)) { handle_alter_part_error(lpt, not_completed, FALSE, frm_install); goto err; @@ -6374,15 +6350,15 @@ 7) Close all tables opened but not yet locked, after this call we are certain that no other thread is in the lock wait queue or has opened the table. The name lock will ensure that they are blocked - on the open call. This is achieved also by get_name_lock call. + on the open call. + This is achieved also by close_data_files_and_morph_locks call. 8) Close all partitions opened by this thread, but retain name lock. 9) Write bin log 10) Prepare handlers for rename and delete of partitions 11) Rename and drop the reorged partitions such that they are no longer used and rename those added to their real new names. 12) Install the shadow frm file - 13) Release the name lock to enable other threads to start using the - table again. + 13) Reopen the table if under lock tables 14) Complete query */ if (write_log_add_change_partition(lpt) || @@ -6396,24 +6372,22 @@ (not_completed= FALSE) || abort_and_upgrade_lock(lpt) || /* Always returns 0 */ ERROR_INJECT_CRASH("crash_change_partition_5") || - get_name_lock(lpt) || - ERROR_INJECT_CRASH("crash_change_partition_6") || alter_close_tables(lpt) || - ERROR_INJECT_CRASH("crash_change_partition_7") || + ERROR_INJECT_CRASH("crash_change_partition_6") || ((!thd->lex->no_write_to_binlog) && (write_bin_log(thd, FALSE, thd->query, thd->query_length), FALSE)) || - ERROR_INJECT_CRASH("crash_change_partition_8") || + ERROR_INJECT_CRASH("crash_change_partition_7") || mysql_write_frm(lpt, WFRM_INSTALL_SHADOW) || - ERROR_INJECT_CRASH("crash_change_partition_9") || + ERROR_INJECT_CRASH("crash_change_partition_8") || mysql_drop_partitions(lpt) || - ERROR_INJECT_CRASH("crash_change_partition_10") || + ERROR_INJECT_CRASH("crash_change_partition_9") || mysql_rename_partitions(lpt) || ((frm_install= TRUE), FALSE) || - ERROR_INJECT_CRASH("crash_change_partition_11") || + ERROR_INJECT_CRASH("crash_change_partition_10") || (write_log_completed(lpt, FALSE), FALSE) || - ERROR_INJECT_CRASH("crash_change_partition_12") || - (release_name_lock(lpt), FALSE)) + ERROR_INJECT_CRASH("crash_change_partition_11") || + (alter_partition_lock_handling(lpt), FALSE)) { handle_alter_part_error(lpt, not_completed, FALSE, frm_install); goto err; diff -Nru a/sql/sql_table.cc b/sql/sql_table.cc --- a/sql/sql_table.cc 2008-01-28 16:07:33 +01:00 +++ b/sql/sql_table.cc 2008-01-28 16:07:33 +01:00 @@ -1226,8 +1226,12 @@ flags Flags as defined below WFRM_INITIAL_WRITE If set we need to prepare table before creating the frm file - WFRM_CREATE_HANDLER_FILES If set we need to create the handler file as - part of the creation of the frm file + WFRM_INSTALL_SHADOW If set we should install the new frm + WFRM_KEEP_SHARE If set we know that the share is to be + retained and thus we should ensure share + object is correct, if not set we don't + set the new partition syntax string since + we know the share object is destroyed. WFRM_PACK_FRM If set we should pack the frm file and delete the frm file @@ -1370,7 +1374,7 @@ goto err; } #ifdef WITH_PARTITION_STORAGE_ENGINE - if (part_info) + if (part_info && (flags & WFRM_KEEP_SHARE)) { TABLE_SHARE *share= lpt->table->s; char *tmp_part_syntax_str;