From bd8657786a185fee54cfbc35b7ef5142e8508073 Mon Sep 17 00:00:00 2001 From: Georgi Kodinov Date: Tue, 9 Dec 2008 19:46:03 +0200 Subject: Bug #37936: ASSERT_COLUMN_MARKED_FOR_WRITE in Field_datetime::store , Field_varstring::store The code that temporary saved the bitmaps of the read set and the write set so that it can set it to all columns for debug purposes was not expecting that the table->read_set and table->write_set can be the same. And was always saving both in sequence. As a result the original value was never restored. Fixed by saving & restoring the original value only once if the two sets are the same (in a special set of functions). mysql-test/r/select.result: Bug #37936: test case mysql-test/t/select.test: Bug #37936: test case sql/item_cmpfunc.cc: Bug #37936: don't save/restore twice if the read and write sets are the same sql/opt_range.cc: Bug #37936: don't save/restore twice if the read and write sets are the same sql/table.h: Bug #37936: Make a designated set of functions that save/restore both the read and the write sets in a single call. --- sql/item_cmpfunc.cc | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) (limited to 'sql/item_cmpfunc.cc') diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index cefa479fea6..564632fbb9b 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -394,19 +394,16 @@ static bool convert_constant_item(THD *thd, Item_field *field_item, TABLE *table= field->table; ulong orig_sql_mode= thd->variables.sql_mode; enum_check_fields orig_count_cuted_fields= thd->count_cuted_fields; - my_bitmap_map *old_write_map; - my_bitmap_map *old_read_map; + my_bitmap_map *old_maps[2]; ulonglong orig_field_val; /* original field value if valid */ - LINT_INIT(old_write_map); - LINT_INIT(old_read_map); + LINT_INIT(old_maps[0]); + LINT_INIT(old_maps[1]); LINT_INIT(orig_field_val); if (table) - { - old_write_map= dbug_tmp_use_all_columns(table, table->write_set); - old_read_map= dbug_tmp_use_all_columns(table, table->read_set); - } + dbug_tmp_use_all_columns(table, old_maps, + table->read_set, table->write_set); /* For comparison purposes allow invalid dates like 2000-01-32 */ thd->variables.sql_mode= (orig_sql_mode & ~MODE_NO_ZERO_DATE) | MODE_INVALID_DATES; @@ -437,10 +434,7 @@ static bool convert_constant_item(THD *thd, Item_field *field_item, thd->variables.sql_mode= orig_sql_mode; thd->count_cuted_fields= orig_count_cuted_fields; if (table) - { - dbug_tmp_restore_column_map(table->write_set, old_write_map); - dbug_tmp_restore_column_map(table->read_set, old_read_map); - } + dbug_tmp_restore_column_maps(table->read_set, table->write_set, old_maps); } return result; } -- cgit v1.2.1 From ffe23f0eb736609915d3c201c3f2556819698dc1 Mon Sep 17 00:00:00 2001 From: Gleb Shchepa Date: Fri, 12 Dec 2008 00:57:32 +0400 Subject: Bug #40761: Assert on sum function on IF(..., CAST(longtext AS UNSIGNED), signed_val) (was: LEFT JOIN on inline view crashes server) Select from a LONGTEXT column wrapped with an expression like "IF(..., CAST(longtext_column AS UNSIGNED), smth_signed)" failed an assertion or crashed the server. IFNULL function was affected too. LONGTEXT column item has a maximum length of 32^2-1 bytes, at the same time this is a maximum possible length of any MySQL item. CAST(longtext_column AS UNSIGNED) returns some unsigned numeric result of length 32^2-1, so the result of IF/IFNULL function of this number and some other signed number will have text length of (32^2-1)+1=32^2 (one byte for the minus sign) - there is integer overflow, and the length is equal to zero. That caused assert/crash. The bug has been fixed by the same solution as in the CASE function implementation. mysql-test/r/func_if.result: Added test case for bug #40761. mysql-test/t/func_if.test: Added test case for bug #40761. sql/item_cmpfunc.cc: Bug #40761: Assert on sum function on IF(..., CAST(longtext AS UNSIGNED), signed_val) 1. Item_func_case::agg_str_lengths method has been moved to the Item_func superclass. 2. Item_func_ifnull/Item_func_if::fix_length_and_dec methods have been updated to calculate max_length, decimals and unsigned flag like Item_func_case. sql/item_cmpfunc.h: Bug #40761: Assert on sum function on IF(..., CAST(longtext AS UNSIGNED), signed_val) Item_func_case::agg_str_lengths method has been moved to the Item_func superclass. sql/item_func.cc: Bug #40761: Assert on sum function on IF(..., CAST(longtext AS UNSIGNED), signed_val) Item_func_case::agg_str_lengths method has been moved to the Item_func superclass. sql/item_func.h: Bug #40761: Assert on sum function on IF(..., CAST(longtext AS UNSIGNED), signed_val) Item_func_case::agg_str_lengths method has been moved to the Item_func superclass. --- sql/item_cmpfunc.cc | 50 ++++++++++++++++++++++---------------------------- 1 file changed, 22 insertions(+), 28 deletions(-) (limited to 'sql/item_cmpfunc.cc') diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 3b1d18b4252..759e912cc82 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -2069,21 +2069,23 @@ Item_func_ifnull::fix_length_and_dec() { agg_result_type(&hybrid_type, args, 2); maybe_null=args[1]->maybe_null; - decimals= max(args[0]->decimals, args[1]->decimals); - unsigned_flag= args[0]->unsigned_flag && args[1]->unsigned_flag; if (hybrid_type == DECIMAL_RESULT || hybrid_type == INT_RESULT) { - int len0= args[0]->max_length - args[0]->decimals - - (args[0]->unsigned_flag ? 0 : 1); - - int len1= args[1]->max_length - args[1]->decimals - - (args[1]->unsigned_flag ? 0 : 1); - - max_length= max(len0, len1) + decimals + (unsigned_flag ? 0 : 1); + max_length= 0; + decimals= 0; + unsigned_flag= TRUE; + agg_num_lengths(args[0]); + agg_num_lengths(args[1]); + max_length= my_decimal_precision_to_length(max_length + decimals, decimals, + unsigned_flag); } else + { max_length= max(args[0]->max_length, args[1]->max_length); + decimals= max(args[0]->decimals, args[1]->decimals); + unsigned_flag=args[0]->unsigned_flag && args[1]->unsigned_flag; + } switch (hybrid_type) { case STRING_RESULT: @@ -2238,8 +2240,6 @@ void Item_func_if::fix_length_and_dec() { maybe_null=args[1]->maybe_null || args[2]->maybe_null; - decimals= max(args[1]->decimals, args[2]->decimals); - unsigned_flag=args[1]->unsigned_flag && args[2]->unsigned_flag; enum Item_result arg1_type=args[1]->result_type(); enum Item_result arg2_type=args[2]->result_type(); @@ -2276,16 +2276,20 @@ Item_func_if::fix_length_and_dec() if ((cached_result_type == DECIMAL_RESULT ) || (cached_result_type == INT_RESULT)) { - int len1= args[1]->max_length - args[1]->decimals - - (args[1]->unsigned_flag ? 0 : 1); - - int len2= args[2]->max_length - args[2]->decimals - - (args[2]->unsigned_flag ? 0 : 1); - - max_length=max(len1, len2) + decimals + (unsigned_flag ? 0 : 1); + max_length= 0; + decimals= 0; + unsigned_flag= TRUE; + agg_num_lengths(args[1]); + agg_num_lengths(args[2]); + max_length= my_decimal_precision_to_length(max_length + decimals, decimals, + unsigned_flag); } else + { max_length= max(args[1]->max_length, args[2]->max_length); + decimals= max(args[1]->decimals, args[2]->decimals); + unsigned_flag=args[1]->unsigned_flag && args[2]->unsigned_flag; + } } @@ -2633,16 +2637,6 @@ void Item_func_case::agg_str_lengths(Item* arg) } -void Item_func_case::agg_num_lengths(Item *arg) -{ - uint len= my_decimal_length_to_precision(arg->max_length, arg->decimals, - arg->unsigned_flag) - arg->decimals; - set_if_bigger(max_length, len); - set_if_bigger(decimals, arg->decimals); - unsigned_flag= unsigned_flag && arg->unsigned_flag; -} - - void Item_func_case::fix_length_and_dec() { Item **agg; -- cgit v1.2.1 From 5ff1bcbc2a3cf3b28ce11673d89a1d2b3d8ad376 Mon Sep 17 00:00:00 2001 From: Gleb Shchepa Date: Fri, 12 Dec 2008 14:59:10 +0400 Subject: rollback of bug #40761 fix --- sql/item_cmpfunc.cc | 50 ++++++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 22 deletions(-) (limited to 'sql/item_cmpfunc.cc') diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 759e912cc82..3b1d18b4252 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -2069,23 +2069,21 @@ Item_func_ifnull::fix_length_and_dec() { agg_result_type(&hybrid_type, args, 2); maybe_null=args[1]->maybe_null; + decimals= max(args[0]->decimals, args[1]->decimals); + unsigned_flag= args[0]->unsigned_flag && args[1]->unsigned_flag; if (hybrid_type == DECIMAL_RESULT || hybrid_type == INT_RESULT) { - max_length= 0; - decimals= 0; - unsigned_flag= TRUE; - agg_num_lengths(args[0]); - agg_num_lengths(args[1]); - max_length= my_decimal_precision_to_length(max_length + decimals, decimals, - unsigned_flag); + int len0= args[0]->max_length - args[0]->decimals + - (args[0]->unsigned_flag ? 0 : 1); + + int len1= args[1]->max_length - args[1]->decimals + - (args[1]->unsigned_flag ? 0 : 1); + + max_length= max(len0, len1) + decimals + (unsigned_flag ? 0 : 1); } else - { max_length= max(args[0]->max_length, args[1]->max_length); - decimals= max(args[0]->decimals, args[1]->decimals); - unsigned_flag=args[0]->unsigned_flag && args[1]->unsigned_flag; - } switch (hybrid_type) { case STRING_RESULT: @@ -2240,6 +2238,8 @@ void Item_func_if::fix_length_and_dec() { maybe_null=args[1]->maybe_null || args[2]->maybe_null; + decimals= max(args[1]->decimals, args[2]->decimals); + unsigned_flag=args[1]->unsigned_flag && args[2]->unsigned_flag; enum Item_result arg1_type=args[1]->result_type(); enum Item_result arg2_type=args[2]->result_type(); @@ -2276,20 +2276,16 @@ Item_func_if::fix_length_and_dec() if ((cached_result_type == DECIMAL_RESULT ) || (cached_result_type == INT_RESULT)) { - max_length= 0; - decimals= 0; - unsigned_flag= TRUE; - agg_num_lengths(args[1]); - agg_num_lengths(args[2]); - max_length= my_decimal_precision_to_length(max_length + decimals, decimals, - unsigned_flag); + int len1= args[1]->max_length - args[1]->decimals + - (args[1]->unsigned_flag ? 0 : 1); + + int len2= args[2]->max_length - args[2]->decimals + - (args[2]->unsigned_flag ? 0 : 1); + + max_length=max(len1, len2) + decimals + (unsigned_flag ? 0 : 1); } else - { max_length= max(args[1]->max_length, args[2]->max_length); - decimals= max(args[1]->decimals, args[2]->decimals); - unsigned_flag=args[1]->unsigned_flag && args[2]->unsigned_flag; - } } @@ -2637,6 +2633,16 @@ void Item_func_case::agg_str_lengths(Item* arg) } +void Item_func_case::agg_num_lengths(Item *arg) +{ + uint len= my_decimal_length_to_precision(arg->max_length, arg->decimals, + arg->unsigned_flag) - arg->decimals; + set_if_bigger(max_length, len); + set_if_bigger(decimals, arg->decimals); + unsigned_flag= unsigned_flag && arg->unsigned_flag; +} + + void Item_func_case::fix_length_and_dec() { Item **agg; -- cgit v1.2.1 From 335e842d24083e8961b7f6b7e9563628dcb8956d Mon Sep 17 00:00:00 2001 From: Gleb Shchepa Date: Wed, 31 Dec 2008 15:55:04 +0400 Subject: Bug #41363: crash of mysqld on windows with aggregate in case Execution of queries containing the CASE function of aggregate function like in "SELECT ... CASE ARGV(...) WHEN ..." crashed the server. The CASE function caches pointers to concrete comparison functions for an each pair of types of CASE-WHERE clause parameters, i.e. for the "CASE INT_RESULT WHERE REAL_RESULT THEN ... WHERE DECIMAL_RESULT ... END" function call it caches comparisons for INT_RESULT with REAL_RESULT and for INT_RESULT with DECIMAL_RESULT. Usually a result type is known after a call to the fix_fields function, however, the setup_copy_fields function call may wrap aggregate items with Item_copy_string that has STRING_RESULT result type, so setup_copy_fields may change argument result types of the CASE function after call to Item_func_case::fix_fields/fix_length_and_dec. Then the Item_func_case::find_item function tries to use comparison function for unexpected pair of the STRING_RESULT and some other type - that caused an assertion failure of server crash. The Item_func_case::fix_length_and_dec function has been modified to take into account possible STRING_RESULT result type in the presence of aggregate arguments of the CASE function. mysql-test/r/func_in.result: Added test case for bug #41363. mysql-test/t/func_in.test: Added test case for bug #41363. sql/item_cmpfunc.cc: Bug #41363: crash of mysqld on windows with aggregate in case The Item_func_case::fix_length_and_dec function has been modified to take into account possible STRING_RESULT result type in the presence of aggregate arguments of the CASE function. --- sql/item_cmpfunc.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'sql/item_cmpfunc.cc') diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index c6b70440b41..813e50e0693 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -2713,6 +2713,16 @@ void Item_func_case::fix_length_and_dec() nagg++; if (!(found_types= collect_cmp_types(agg, nagg))) return; + if (with_sum_func || current_thd->lex->current_select->group_list.elements) + { + /* + See TODO commentary in the setup_copy_fields function: + item in a group may be wrapped with an Item_copy_string item. + That item has a STRING_RESULT result type, so we need + to take this type into account. + */ + found_types |= (1 << item_cmp_type(left_result_type, STRING_RESULT)); + } for (i= 0; i <= (uint)DECIMAL_RESULT; i++) { -- cgit v1.2.1