[Maria-developers] bzr commit into MariaDB 5.1, with Maria 1.5:maria branch (knielsen:2714)
#At lp:maria 2714 knielsen@knielsen-hq.org 2009-06-18 Fix test cases after merge of XtraDB into MariaDB. Manually merge some InnoDB changes into XtraDB. Fix ALTER TABLE bug in XtraDB with wrong comparison of row type. modified: mysql-test/include/varchar.inc mysql-test/mysql-test-run.pl mysql-test/r/innodb.result mysql-test/t/innodb-use-sys-malloc.test sql/sql_table.cc storage/xtradb/handler/ha_innodb.cc storage/xtradb/include/pars0pars.h storage/xtradb/include/univ.i per-file messages: mysql-test/include/varchar.inc Fix in test case that which of several duplicate keys triggers an error is not deterministic. mysql-test/mysql-test-run.pl InnoDB does not bother to free resources individually during shutdown, but due to using its own memory tracking it nevertheless can free everything at exit. But XtraDB adds an option, on by default, to skip this extra tracking. This causes lots of Valgrind warnings, so needs to be disabled for Valgrind testing. mysql-test/r/innodb.result Fix in test case that which of several duplicate keys triggers an error is not deterministic. mysql-test/t/innodb-use-sys-malloc.test InnoDB does not bother to free resources individually during shutdown, but due to using its own memory tracking it nevertheless can free everything at exit. But XtraDB adds an option, on by default, to skip this extra tracking. This causes lots of Valgrind warnings, so needs to be disabled for Valgrind testing. sql/sql_table.cc Add some useful DBUG while debugging alter table. storage/xtradb/handler/ha_innodb.cc Fix that check_if_incompatible_data did not realise that ROW_TYPE_DEFAULT is identical to the default row format ROW_TYPE_COMPACT, causing excessive table copying in ALTER TABLE Add some useful DBUG while debugging alter table. Manually merge into XtraDB a few small changes for InnoDB from upstream MySQL. storage/xtradb/include/pars0pars.h Manually merge into XtraDB a few small changes for InnoDB from upstream MySQL. storage/xtradb/include/univ.i Manually merge a MariaDB fix in InnoDB into XtraDB. === modified file 'mysql-test/include/varchar.inc' --- a/mysql-test/include/varchar.inc 2008-05-14 06:50:16 +0000 +++ b/mysql-test/include/varchar.inc 2009-06-18 12:39:21 +0000 @@ -86,6 +86,8 @@ explain select count(*) from t1 where v --replace_column 9 # explain select count(*) from t1 where v between 'a' and 'a ' and v between 'a ' and 'b\n'; +# Which duplicate entry triggers error is not deterministic. +--replace_regex /Duplicate entry '[^']+' for key/Duplicate entry '{ ' for key/ --error ER_DUP_ENTRY alter table t1 add unique(v); alter table t1 add key(v); === modified file 'mysql-test/mysql-test-run.pl' --- a/mysql-test/mysql-test-run.pl 2009-06-05 15:35:22 +0000 +++ b/mysql-test/mysql-test-run.pl 2009-06-18 12:39:21 +0000 @@ -1356,6 +1356,18 @@ sub command_line_setup { join(" ", @valgrind_args), "\""); } + # InnoDB does not bother to do individual de-allocations at exit. Instead it + # relies on a custom allocator to track every allocation, and frees all at + # once during exit. + # In XtraDB, an option use-sys-malloc is introduced (and on by default) to + # disable this (for performance). But this exposes Valgrind to all the + # missing de-allocations, so we need to disable it to at least get + # meaningful leak checking for the rest of the server. + if ($opt_valgrind_mysqld) + { + push(@opt_extra_mysqld_opt, "--loose-skip-innodb-use-sys-malloc"); + } + mtr_report("Checking supported features..."); check_ndbcluster_support(\%mysqld_variables); === modified file 'mysql-test/r/innodb.result' --- a/mysql-test/r/innodb.result 2009-06-09 15:08:46 +0000 +++ b/mysql-test/r/innodb.result 2009-06-18 12:39:21 +0000 @@ -1970,7 +1970,7 @@ explain select count(*) from t1 where v id select_type table type possible_keys key key_len ref rows Extra 1 SIMPLE t1 ref v v 13 const # Using where; Using index alter table t1 add unique(v); -ERROR 23000: Duplicate entry 'v' for key 'v_2' +ERROR 23000: Duplicate entry '{ ' for key 'v_2' alter table t1 add key(v); select concat('*',v,'*',c,'*',t,'*') as qq from t1 where v='a'; qq === modified file 'mysql-test/t/innodb-use-sys-malloc.test' --- a/mysql-test/t/innodb-use-sys-malloc.test 2009-06-09 13:19:13 +0000 +++ b/mysql-test/t/innodb-use-sys-malloc.test 2009-06-18 12:39:21 +0000 @@ -1,4 +1,7 @@ --source include/have_innodb.inc +# XtraDB has lots of memory leak warnings at shutdown when +# --innodb-use-sys-malloc +--source include/not_valgrind.inc #display current value of innodb_use_sys_malloc SELECT @@GLOBAL.innodb_use_sys_malloc; === modified file 'sql/sql_table.cc' --- a/sql/sql_table.cc 2009-06-02 09:58:27 +0000 +++ b/sql/sql_table.cc 2009-06-18 12:39:21 +0000 @@ -5333,6 +5333,7 @@ compare_tables(TABLE *table, !table->s->mysql_version || (table->s->frm_version < FRM_VER_TRUE_VARCHAR && varchar)) { + DBUG_PRINT("info", ("Basic checks -> ALTER_TABLE_DATA_CHANGED")); *need_copy_table= ALTER_TABLE_DATA_CHANGED; DBUG_RETURN(0); } @@ -5361,6 +5362,8 @@ compare_tables(TABLE *table, if ((tmp_new_field->flags & NOT_NULL_FLAG) != (uint) (field->flags & NOT_NULL_FLAG)) { + DBUG_PRINT("info", ("NULL behaviour difference in field '%s' -> " + "ALTER_TABLE_DATA_CHANGED", new_field->field_name)); *need_copy_table= ALTER_TABLE_DATA_CHANGED; DBUG_RETURN(0); } @@ -5382,6 +5385,8 @@ compare_tables(TABLE *table, /* Evaluate changes bitmap and send to check_if_incompatible_data() */ if (!(tmp= field->is_equal(tmp_new_field))) { + DBUG_PRINT("info", ("!field_is_equal('%s') -> ALTER_TABLE_DATA_CHANGED", + new_field->field_name)); *need_copy_table= ALTER_TABLE_DATA_CHANGED; DBUG_RETURN(0); } @@ -5515,16 +5520,22 @@ compare_tables(TABLE *table, /* Check if changes are compatible with current handler without a copy */ if (table->file->check_if_incompatible_data(create_info, changes)) { + DBUG_PRINT("info", ("check_if_incompatible_data() -> " + "ALTER_TABLE_DATA_CHANGED")); *need_copy_table= ALTER_TABLE_DATA_CHANGED; DBUG_RETURN(0); } if (*index_drop_count || *index_add_count) { + DBUG_PRINT("info", ("Index dropped=%u added=%u -> " + "ALTER_TABLE_INDEX_CHANGED", + *index_drop_count, *index_add_count)); *need_copy_table= ALTER_TABLE_INDEX_CHANGED; DBUG_RETURN(0); } + DBUG_PRINT("info", (" -> ALTER_TABLE_METADATA_ONLY")); *need_copy_table= ALTER_TABLE_METADATA_ONLY; // Tables are compatible DBUG_RETURN(0); } === modified file 'storage/xtradb/handler/ha_innodb.cc' --- a/storage/xtradb/handler/ha_innodb.cc 2009-06-11 12:53:26 +0000 +++ b/storage/xtradb/handler/ha_innodb.cc 2009-06-18 12:39:21 +0000 @@ -5018,7 +5018,6 @@ convert_search_mode_to_innobase( case HA_READ_MBR_WITHIN: case HA_READ_MBR_DISJOINT: case HA_READ_MBR_EQUAL: - my_error(ER_TABLE_CANT_HANDLE_SPKEYS, MYF(0)); return(PAGE_CUR_UNSUPP); /* do not use "default:" in order to produce a gcc warning: enumeration value '...' not handled in switch @@ -6720,6 +6719,7 @@ innobase_rename_table( int error; char* norm_to; char* norm_from; + DBUG_ENTER("innobase_rename_table"); if (lower_case_table_names) { srv_lower_case_table_names = TRUE; @@ -6747,6 +6747,7 @@ innobase_rename_table( if (error != DB_SUCCESS) { FILE* ef = dict_foreign_err_file; + DBUG_PRINT("info", ("rename failed: %d", error)); fputs("InnoDB: Renaming table ", ef); ut_print_name(ef, trx, TRUE, norm_from); fputs(" to ", ef); @@ -6767,7 +6768,7 @@ innobase_rename_table( my_free(norm_to, MYF(0)); my_free(norm_from, MYF(0)); - return error; + DBUG_RETURN(error); } /************************************************************************* Renames an InnoDB table. */ @@ -6900,7 +6901,7 @@ ha_innobase::records_in_range( mode2); } else { - n_rows = 0; + n_rows = HA_POS_ERROR; } mem_heap_free(heap); @@ -7614,7 +7615,7 @@ ha_innobase::get_foreign_key_list(THD *t f_key_info.referenced_key_name = thd_make_lex_string( thd, f_key_info.referenced_key_name, foreign->referenced_index->name, - strlen(foreign->referenced_index->name), 1); + (uint) strlen(foreign->referenced_index->name), 1); } else f_key_info.referenced_key_name= 0; @@ -8227,7 +8228,7 @@ innodb_show_status( bool result = FALSE; - if (stat_print(thd, innobase_hton_name, strlen(innobase_hton_name), + if (stat_print(thd, innobase_hton_name, (uint) strlen(innobase_hton_name), STRING_WITH_LEN(""), str, flen)) { result= TRUE; } @@ -8258,7 +8259,7 @@ innodb_mutex_show_status( ulint rw_lock_count_os_yield= 0; ulonglong rw_lock_wait_time= 0; #endif /* UNIV_DEBUG */ - uint hton_name_len= strlen(innobase_hton_name), buf1len, buf2len; + uint hton_name_len= (uint) strlen(innobase_hton_name), buf1len, buf2len; DBUG_ENTER("innodb_mutex_show_status"); DBUG_ASSERT(hton == innodb_hton_ptr); @@ -8302,9 +8303,9 @@ innodb_mutex_show_status( rw_lock_wait_time += mutex->lspent_time; } #else /* UNIV_DEBUG */ - buf1len= my_snprintf(buf1, sizeof(buf1), "%s:%lu", + buf1len= (uint) my_snprintf(buf1, sizeof(buf1), "%s:%lu", mutex->cfile_name, (ulong) mutex->cline); - buf2len= my_snprintf(buf2, sizeof(buf2), "os_waits=%lu", + buf2len= (uint) my_snprintf(buf2, sizeof(buf2), "os_waits=%lu", mutex->count_os_wait); if (stat_print(thd, innobase_hton_name, @@ -8860,7 +8861,7 @@ ha_innobase::get_error_message(int error { trx_t* trx = check_trx_exists(ha_thd()); - buf->copy(trx->detailed_error, strlen(trx->detailed_error), + buf->copy(trx->detailed_error, (uint) strlen(trx->detailed_error), system_charset_info); return(FALSE); @@ -9294,31 +9295,49 @@ ha_innobase::check_if_incompatible_data( HA_CREATE_INFO* info, uint table_changes) { + enum row_type row_type, info_row_type; + DBUG_ENTER("ha_innobase::check_if_incompatible_data"); + if (table_changes != IS_EQUAL_YES) { - return(COMPATIBLE_DATA_NO); + DBUG_PRINT("info", ("table_changes != IS_EQUAL_YES " + "-> COMPATIBLE_DATA_NO")); + DBUG_RETURN(COMPATIBLE_DATA_NO); } /* Check that auto_increment value was not changed */ if ((info->used_fields & HA_CREATE_USED_AUTO) && info->auto_increment_value != 0) { - return(COMPATIBLE_DATA_NO); + DBUG_PRINT("info", ("auto_increment_value changed -> " + "COMPATIBLE_DATA_NO")); + DBUG_RETURN(COMPATIBLE_DATA_NO); } /* Check that row format didn't change */ + row_type = get_row_type(); + info_row_type = info->row_type; + /* Default is compact. */ + if (info_row_type == ROW_TYPE_DEFAULT) + info_row_type = ROW_TYPE_COMPACT; if ((info->used_fields & HA_CREATE_USED_ROW_FORMAT) && - get_row_type() != info->row_type) { + row_type != info_row_type) { - return(COMPATIBLE_DATA_NO); + DBUG_PRINT("info", ("get_row_type()=%d != info->row_type=%d -> " + "COMPATIBLE_DATA_NO", + row_type, info->row_type)); + DBUG_RETURN(COMPATIBLE_DATA_NO); } /* Specifying KEY_BLOCK_SIZE requests a rebuild of the table. */ if (info->used_fields & HA_CREATE_USED_KEY_BLOCK_SIZE) { - return(COMPATIBLE_DATA_NO); + DBUG_PRINT("info", ("HA_CREATE_USED_KEY_BLOCK_SIZE -> " + "COMPATIBLE_DATA_NO")); + DBUG_RETURN(COMPATIBLE_DATA_NO); } - return(COMPATIBLE_DATA_YES); + DBUG_PRINT("info", (" -> COMPATIBLE_DATA_YES")); + DBUG_RETURN(COMPATIBLE_DATA_YES); } /**************************************************************** === modified file 'storage/xtradb/include/pars0pars.h' --- a/storage/xtradb/include/pars0pars.h 2009-03-26 06:11:11 +0000 +++ b/storage/xtradb/include/pars0pars.h 2009-06-18 12:39:21 +0000 @@ -700,7 +700,7 @@ struct for_node_struct{ definition */ que_node_t* loop_start_limit;/* initial value of loop variable */ que_node_t* loop_end_limit; /* end value of loop variable */ - int loop_end_value; /* evaluated value for the end value: + lint loop_end_value; /* evaluated value for the end value: it is calculated only when the loop is entered, and will not change within the loop */ === modified file 'storage/xtradb/include/univ.i' --- a/storage/xtradb/include/univ.i 2009-06-11 12:53:26 +0000 +++ b/storage/xtradb/include/univ.i 2009-06-18 12:39:21 +0000 @@ -133,10 +133,10 @@ from Makefile.in->ut0auxconf.h */ # endif /* HAVE_ATOMIC_PTHREAD_T */ #endif /* HAVE_GCC_ATOMIC_BUILTINS */ -/* We only try to do explicit inlining of functions with gcc and -Microsoft Visual C++ */ +/* Enable explicit inlining of functions only for compilers known to +support it. */ -# if !defined(__GNUC__) +# if !defined(__GNUC__) && !defined(__SUNPRO_C) # undef UNIV_MUST_NOT_INLINE /* Remove compiler warning */ # define UNIV_MUST_NOT_INLINE # endif
Hi!
"knielsen" == knielsen <knielsen@knielsen-hq.org> writes:
knielsen> #At lp:maria knielsen> 2714 knielsen@knielsen-hq.org 2009-06-18 knielsen> Fix test cases after merge of XtraDB into MariaDB. knielsen> Manually merge some InnoDB changes into XtraDB. knielsen> Fix ALTER TABLE bug in XtraDB with wrong comparison of row type. knielsen> modified: knielsen> mysql-test/include/varchar.inc knielsen> mysql-test/mysql-test-run.pl knielsen> mysql-test/r/innodb.result knielsen> mysql-test/t/innodb-use-sys-malloc.test knielsen> sql/sql_table.cc knielsen> storage/xtradb/handler/ha_innodb.cc knielsen> storage/xtradb/include/pars0pars.h knielsen> storage/xtradb/include/univ.i knielsen> per-file messages: knielsen> mysql-test/include/varchar.inc knielsen> Fix in test case that which of several duplicate keys triggers an error is not knielsen> deterministic. knielsen> mysql-test/mysql-test-run.pl knielsen> InnoDB does not bother to free resources individually during shutdown, but due to using knielsen> its own memory tracking it nevertheless can free everything at exit. But XtraDB adds an knielsen> option, on by default, to skip this extra tracking. This causes lots of Valgrind knielsen> warnings, so needs to be disabled for Valgrind testing. knielsen> mysql-test/r/innodb.result knielsen> Fix in test case that which of several duplicate keys triggers an error is not knielsen> deterministic. knielsen> mysql-test/t/innodb-use-sys-malloc.test knielsen> InnoDB does not bother to free resources individually during shutdown, but due to using knielsen> its own memory tracking it nevertheless can free everything at exit. But XtraDB adds an knielsen> option, on by default, to skip this extra tracking. This causes lots of Valgrind knielsen> warnings, so needs to be disabled for Valgrind testing. knielsen> sql/sql_table.cc knielsen> Add some useful DBUG while debugging alter table. knielsen> storage/xtradb/handler/ha_innodb.cc knielsen> Fix that check_if_incompatible_data did not realise that ROW_TYPE_DEFAULT is identical knielsen> to the default row format ROW_TYPE_COMPACT, causing excessive table copying in knielsen> ALTER TABLE knielsen> Add some useful DBUG while debugging alter table. knielsen> Manually merge into XtraDB a few small changes for InnoDB from upstream MySQL. knielsen> storage/xtradb/include/pars0pars.h knielsen> Manually merge into XtraDB a few small changes for InnoDB from upstream MySQL. knielsen> storage/xtradb/include/univ.i knielsen> Manually merge a MariaDB fix in InnoDB into XtraDB. <cut> knielsen> === modified file 'mysql-test/r/innodb.result' knielsen> --- a/mysql-test/r/innodb.result 2009-06-09 15:08:46 +0000 knielsen> +++ b/mysql-test/r/innodb.result 2009-06-18 12:39:21 +0000 knielsen> @@ -1970,7 +1970,7 @@ explain select count(*) from t1 where v knielsen> id select_type table type possible_keys key key_len ref rows Extra knielsen> 1 SIMPLE t1 ref v v 13 const # Using where; Using index knielsen> alter table t1 add unique(v); knielsen> -ERROR 23000: Duplicate entry 'v' for key 'v_2' knielsen> +ERROR 23000: Duplicate entry '{ ' for key 'v_2' knielsen> alter table t1 add key(v); knielsen> select concat('*',v,'*',c,'*',t,'*') as qq from t1 where v='a'; knielsen> qq Strange change in test result, but can't comment on it as I don't know the test. <cut> knielsen> === modified file 'storage/xtradb/handler/ha_innodb.cc' <cut> knielsen> @@ -6900,7 +6901,7 @@ ha_innobase::records_in_range( knielsen> mode2); knielsen> } else { knielsen> - n_rows = 0; knielsen> + n_rows = HA_POS_ERROR; knielsen> } Looks correct. Wonder from where the n_rows= 0 came from... <cut> Ok to push, but please check that the duplicate key error is correct (ie, there is two keys with the content '{ ') Regards, Monty
Michael Widenius <monty@askmonty.org> writes:
knielsen> === modified file 'mysql-test/r/innodb.result' knielsen> --- a/mysql-test/r/innodb.result 2009-06-09 15:08:46 +0000 knielsen> +++ b/mysql-test/r/innodb.result 2009-06-18 12:39:21 +0000 knielsen> @@ -1970,7 +1970,7 @@ explain select count(*) from t1 where v knielsen> id select_type table type possible_keys key key_len ref rows Extra knielsen> 1 SIMPLE t1 ref v v 13 const # Using where; Using index knielsen> alter table t1 add unique(v); knielsen> -ERROR 23000: Duplicate entry 'v' for key 'v_2' knielsen> +ERROR 23000: Duplicate entry '{ ' for key 'v_2' knielsen> alter table t1 add key(v); knielsen> select concat('*',v,'*',c,'*',t,'*') as qq from t1 where v='a'; knielsen> qq
Strange change in test result, but can't comment on it as I don't know the test.
Ok to push, but please check that the duplicate key error is correct (ie, there is two keys with the content '{ ')
Actually, this key value is just an arbitrary placeholder, due to this in include/varchar.inc: +# Which duplicate entry triggers error is not deterministic. +--replace_regex /Duplicate entry '[^']+' for key/Duplicate entry '{ ' for key/ The reason I replace with that value is that this is the value detected as duplicate in some other tests (I think it was a maria test). So replacing with this value was to avoid having to change more result files. However, this is perhaps somewhat confusing, so if you prefer I could instead do something like this: +# Which duplicate entry triggers error is not deterministic. +--replace_regex /Duplicate entry '[^']+' for key/Duplicate entry 'DUP' for key/ and update all result files accordingly. In any case, there are multiple duplicates in the statement, so doesn't really make sense to check which one is mentioned in the error message.
knielsen> === modified file 'storage/xtradb/handler/ha_innodb.cc'
<cut>
knielsen> @@ -6900,7 +6901,7 @@ ha_innobase::records_in_range( knielsen> mode2); knielsen> } else {
knielsen> - n_rows = 0; knielsen> + n_rows = HA_POS_ERROR; knielsen> }
Looks correct. Wonder from where the n_rows= 0 came from...
I do not know. This and a few other minor innodb changes are just merges from the MySQL tree. I checked all of the changes in storage/innodb in MySQL. Most of them were already in storage/xtradb; the ones that were not I applied manually. - Kristian.
participants (3)
-
knielsen@knielsen-hq.org
-
Kristian Nielsen
-
Michael Widenius