diff --git a/mysql-test/include/mix1.inc b/mysql-test/include/mix1.inc index d40d78c21b8..303f896cdfe 100644 --- a/mysql-test/include/mix1.inc +++ b/mysql-test/include/mix1.inc @@ -1544,4 +1544,31 @@ SELECT 1 FROM (SELECT COUNT(DISTINCT c1) DROP TABLE t1; +eval +CREATE TABLE t1 (c1 REAL, c2 REAL, c3 REAL, KEY (c3), KEY (c2, c3)) + ENGINE=$engine_type; +INSERT INTO t1 VALUES (1,1,1), (1,1,1), (1,1,2), (1,1,1), (1,1,2); + +SELECT 1 FROM (SELECT COUNT(DISTINCT c1) + FROM t1 WHERE c2 IN (1, 1) AND c3 = 2 GROUP BY c2) x; +EXPLAIN +SELECT 1 FROM (SELECT COUNT(DISTINCT c1) + FROM t1 WHERE c2 IN (1, 1) AND c3 = 2 GROUP BY c2) x; + +DROP TABLE t1; + +eval +CREATE TABLE t1 (c1 DECIMAL(12,2), c2 DECIMAL(12,2), c3 DECIMAL(12,2), + KEY (c3), KEY (c2, c3)) + ENGINE=$engine_type; +INSERT INTO t1 VALUES (1,1,1), (1,1,1), (1,1,2), (1,1,1), (1,1,2); + +SELECT 1 FROM (SELECT COUNT(DISTINCT c1) + FROM t1 WHERE c2 IN (1, 1) AND c3 = 2 GROUP BY c2) x; +EXPLAIN +SELECT 1 FROM (SELECT COUNT(DISTINCT c1) + FROM t1 WHERE c2 IN (1, 1) AND c3 = 2 GROUP BY c2) x; + +DROP TABLE t1; + --echo End of 5.1 tests diff --git a/mysql-test/r/func_in.result b/mysql-test/r/func_in.result index 1e967b668c5..88a822a2fa6 100644 --- a/mysql-test/r/func_in.result +++ b/mysql-test/r/func_in.result @@ -587,4 +587,25 @@ SELECT CASE c1 WHEN c1 + 1 THEN 1 END, ABS(AVG(c0)) FROM t1; CASE c1 WHEN c1 + 1 THEN 1 END ABS(AVG(c0)) NULL 1.0000 DROP TABLE t1; +CREATE TABLE t1(a TEXT, b INT, c INT UNSIGNED, d DECIMAL(12,2), e REAL); +INSERT INTO t1 VALUES('iynfj', 1, 1, 1, 1); +INSERT INTO t1 VALUES('innfj', 2, 2, 2, 2); +SELECT SUM( DISTINCT a ) FROM t1 GROUP BY a HAVING a IN ( AVG( 1 ), 1 + a); +SUM( DISTINCT a ) +SELECT SUM( DISTINCT b ) FROM t1 GROUP BY b HAVING b IN ( AVG( 1 ), 1 + b); +SUM( DISTINCT b ) +1 +SELECT SUM( DISTINCT c ) FROM t1 GROUP BY c HAVING c IN ( AVG( 1 ), 1 + c); +SUM( DISTINCT c ) +1 +SELECT SUM( DISTINCT d ) FROM t1 GROUP BY d HAVING d IN ( AVG( 1 ), 1 + d); +SUM( DISTINCT d ) +1.00 +SELECT SUM( DISTINCT e ) FROM t1 GROUP BY e HAVING e IN ( AVG( 1 ), 1 + e); +SUM( DISTINCT e ) +1 +SELECT SUM( DISTINCT e ) FROM t1 GROUP BY b,c,d HAVING (b,c,d) IN +((AVG( 1 ), 1 + c, 1 + d), (AVG( 1 ), 2 + c, 2 + d)); +SUM( DISTINCT e ) +DROP TABLE t1; End of 5.1 tests diff --git a/mysql-test/r/innodb_mysql.result b/mysql-test/r/innodb_mysql.result index 671009a8a73..83a2a2111d5 100644 --- a/mysql-test/r/innodb_mysql.result +++ b/mysql-test/r/innodb_mysql.result @@ -1718,6 +1718,35 @@ id select_type table type possible_keys key key_len ref rows Extra 1 PRIMARY system NULL NULL NULL NULL 1 2 DERIVED t1 index c3,c2 c2 10 NULL 5 DROP TABLE t1; +CREATE TABLE t1 (c1 REAL, c2 REAL, c3 REAL, KEY (c3), KEY (c2, c3)) +ENGINE=InnoDB; +INSERT INTO t1 VALUES (1,1,1), (1,1,1), (1,1,2), (1,1,1), (1,1,2); +SELECT 1 FROM (SELECT COUNT(DISTINCT c1) +FROM t1 WHERE c2 IN (1, 1) AND c3 = 2 GROUP BY c2) x; +1 +1 +EXPLAIN +SELECT 1 FROM (SELECT COUNT(DISTINCT c1) +FROM t1 WHERE c2 IN (1, 1) AND c3 = 2 GROUP BY c2) x; +id select_type table type possible_keys key key_len ref rows Extra +1 PRIMARY system NULL NULL NULL NULL 1 +2 DERIVED t1 index c3,c2 c2 18 NULL 5 +DROP TABLE t1; +CREATE TABLE t1 (c1 DECIMAL(12,2), c2 DECIMAL(12,2), c3 DECIMAL(12,2), +KEY (c3), KEY (c2, c3)) +ENGINE=InnoDB; +INSERT INTO t1 VALUES (1,1,1), (1,1,1), (1,1,2), (1,1,1), (1,1,2); +SELECT 1 FROM (SELECT COUNT(DISTINCT c1) +FROM t1 WHERE c2 IN (1, 1) AND c3 = 2 GROUP BY c2) x; +1 +1 +EXPLAIN +SELECT 1 FROM (SELECT COUNT(DISTINCT c1) +FROM t1 WHERE c2 IN (1, 1) AND c3 = 2 GROUP BY c2) x; +id select_type table type possible_keys key key_len ref rows Extra +1 PRIMARY system NULL NULL NULL NULL 1 +2 DERIVED t1 index c3,c2 c2 14 NULL 5 +DROP TABLE t1; End of 5.1 tests drop table if exists t1, t2, t3; create table t1(a int); diff --git a/mysql-test/t/func_in.test b/mysql-test/t/func_in.test index 3fc1697f146..adc074259ad 100644 --- a/mysql-test/t/func_in.test +++ b/mysql-test/t/func_in.test @@ -439,4 +439,21 @@ SELECT CASE c1 WHEN c1 + 1 THEN 1 END, ABS(AVG(c0)) FROM t1; DROP TABLE t1; +# +# Bug #44399: crash with statement using TEXT columns, aggregates, GROUP BY, +# and HAVING +# + +CREATE TABLE t1(a TEXT, b INT, c INT UNSIGNED, d DECIMAL(12,2), e REAL); +INSERT INTO t1 VALUES('iynfj', 1, 1, 1, 1); +INSERT INTO t1 VALUES('innfj', 2, 2, 2, 2); +SELECT SUM( DISTINCT a ) FROM t1 GROUP BY a HAVING a IN ( AVG( 1 ), 1 + a); +SELECT SUM( DISTINCT b ) FROM t1 GROUP BY b HAVING b IN ( AVG( 1 ), 1 + b); +SELECT SUM( DISTINCT c ) FROM t1 GROUP BY c HAVING c IN ( AVG( 1 ), 1 + c); +SELECT SUM( DISTINCT d ) FROM t1 GROUP BY d HAVING d IN ( AVG( 1 ), 1 + d); +SELECT SUM( DISTINCT e ) FROM t1 GROUP BY e HAVING e IN ( AVG( 1 ), 1 + e); +SELECT SUM( DISTINCT e ) FROM t1 GROUP BY b,c,d HAVING (b,c,d) IN + ((AVG( 1 ), 1 + c, 1 + d), (AVG( 1 ), 2 + c, 2 + d)); +DROP TABLE t1; + --echo End of 5.1 tests diff --git a/sql/item.cc b/sql/item.cc index 420efb2a4ee..768af47d660 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -3268,10 +3268,58 @@ Item_param::set_param_type_and_swap_value(Item_param *src) str_value_ptr.swap(src->str_value_ptr); } +/**************************************************************************** + Item_copy +****************************************************************************/ +Item_copy *Item_copy::create (Item *item) +{ + switch (item->result_type()) + { + case STRING_RESULT: + return new Item_copy_string (item); + case REAL_RESULT: + return new Item_copy_float (item); + case INT_RESULT: + return item->unsigned_flag ? + new Item_copy_uint (item) : new Item_copy_int (item); + case DECIMAL_RESULT: + return new Item_copy_decimal (item); + + case ROW_RESULT: + DBUG_ASSERT (0); + } + /* should not happen */ + return NULL; +} + /**************************************************************************** Item_copy_string ****************************************************************************/ +double Item_copy_string::val_real() +{ + int err_not_used; + char *end_not_used; + return (null_value ? 0.0 : + my_strntod(str_value.charset(), (char*) str_value.ptr(), + str_value.length(), &end_not_used, &err_not_used)); +} + +longlong Item_copy_string::val_int() +{ + int err; + return null_value ? LL(0) : my_strntoll(str_value.charset(),str_value.ptr(), + str_value.length(),10, (char**) 0, + &err); +} + + +int Item_copy_string::save_in_field(Field *field, bool no_conversions) +{ + return save_str_value_in_field(field, &str_value); +} + + void Item_copy_string::copy() { String *res=item->val_str(&str_value); @@ -3294,12 +3342,163 @@ my_decimal *Item_copy_string::val_decimal(my_decimal *decimal_value) { // Item_copy_string is used without fix_fields call if (null_value) - return 0; + return (my_decimal *) 0; string2my_decimal(E_DEC_FATAL_ERROR, &str_value, decimal_value); return (decimal_value); } +/**************************************************************************** + Item_copy_int +****************************************************************************/ + +void Item_copy_int::copy() +{ + cached_value= item->val_int(); + null_value=item->null_value; +} + +static int save_int_value_in_field (Field *field, longlong nr, + bool null_value, bool unsigned_flag); + +int Item_copy_int::save_in_field(Field *field, bool no_conversions) +{ + return save_int_value_in_field(field, cached_value, + null_value, unsigned_flag); +} + + +String *Item_copy_int::val_str(String *str) +{ + if (null_value) + return (String *) 0; + + str->set(cached_value, &my_charset_bin); + return str; +} + + +my_decimal *Item_copy_int::val_decimal(my_decimal *decimal_value) +{ + if (null_value) + return (my_decimal *) 0; + + int2my_decimal(E_DEC_FATAL_ERROR, cached_value, unsigned_flag, decimal_value); + return decimal_value; +} + + +/**************************************************************************** + Item_copy_uint +****************************************************************************/ + +String *Item_copy_uint::val_str(String *str) +{ + if (null_value) + return (String *) 0; + + str->set((ulonglong) cached_value, &my_charset_bin); + return str; +} + + +/**************************************************************************** + Item_copy_float +****************************************************************************/ + +String *Item_copy_float::val_str(String *str) +{ + if (null_value) + return (String *) 0; + else + { + double nr= val_real(); + str->set_real(nr,decimals, &my_charset_bin); + return str; + } +} + + +my_decimal *Item_copy_float::val_decimal(my_decimal *decimal_value) +{ + if (null_value) + return (my_decimal *) 0; + else + { + double nr= val_real(); + double2my_decimal(E_DEC_FATAL_ERROR, nr, decimal_value); + return decimal_value; + } +} + + +int Item_copy_float::save_in_field(Field *field, bool no_conversions) +{ + if (null_value) + return set_field_to_null(field); + field->set_notnull(); + return field->store(cached_value); +} + + +/**************************************************************************** + Item_copy_decimal +****************************************************************************/ + +int Item_copy_decimal::save_in_field(Field *field, bool no_conversions) +{ + if (null_value) + return set_field_to_null(field); + field->set_notnull(); + return field->store_decimal(&cached_value); +} + + +String *Item_copy_decimal::val_str(String *result) +{ + if (null_value) + return (String *) 0; + result->set_charset(&my_charset_bin); + my_decimal2string(E_DEC_FATAL_ERROR, &cached_value, 0, 0, 0, result); + return result; +} + + +double Item_copy_decimal::val_real() +{ + if (null_value) + return 0.0; + else + { + double result; + my_decimal2double(E_DEC_FATAL_ERROR, &cached_value, &result); + return result; + } +} + + +longlong Item_copy_decimal::val_int() +{ + if (null_value) + return LL(0); + else + { + longlong result; + my_decimal2int(E_DEC_FATAL_ERROR, &cached_value, unsigned_flag, &result); + return result; + } +} + + +void Item_copy_decimal::copy() +{ + my_decimal *nr= item->val_decimal(&cached_value); + if (nr && nr != &cached_value) + memcpy (&cached_value, nr, sizeof (my_decimal)); + null_value= item->null_value; +} + + /* Functions to convert item to field (for send_fields) */ @@ -4947,10 +5146,9 @@ int Item_uint::save_in_field(Field *field, bool no_conversions) return Item_int::save_in_field(field, no_conversions); } - -int Item_int::save_in_field(Field *field, bool no_conversions) +static int save_int_value_in_field (Field *field, longlong nr, + bool null_value, bool unsigned_flag) { - longlong nr=val_int(); if (null_value) return set_field_to_null(field); field->set_notnull(); @@ -4958,6 +5156,12 @@ int Item_int::save_in_field(Field *field, bool no_conversions) } +int Item_int::save_in_field(Field *field, bool no_conversions) +{ + return save_int_value_in_field (field, val_int(), null_value, unsigned_flag); +} + + int Item_decimal::save_in_field(Field *field, bool no_conversions) { field->set_notnull(); diff --git a/sql/item.h b/sql/item.h index 96a4e9f7a31..3dfcd7c2612 100644 --- a/sql/item.h +++ b/sql/item.h @@ -2443,48 +2443,203 @@ public: #include "item_xmlfunc.h" #endif -class Item_copy_string :public Item +/** + Base class to implement typed value caching Item classes + + Item_copy_ classes are very similar to the corresponding Item_ + classes (e.g. Item_copy_int is similar to Item_int) but they add + the following additional functionality to Item_ : + 1. Nullability + 2. Possibility to store the value not only on instantiation time, + but also later. + Item_copy_ classes are a functionality subset of Item_cache_ + classes, as e.g. they don't support comparisons with the original Item + as Item_cache_ classes do. + Item_copy_ classes are used in GROUP BY calculation. + TODO: Item_copy should be made an abstract interface and Item_copy_ + classes should inherit both the respective Item_ class and the interface. + Ideally we should drop Item_copy_ classes altogether and merge + their functionality to Item_cache_ (and these should be made to inherit + from Item_). +*/ + +class Item_copy :public Item { +protected: + + /** + Stores the type of the resulting field that would be used to store the data + in the cache. This is to avoid calls to the original item. + */ enum enum_field_types cached_field_type; -public: + + /** The original item that is copied */ Item *item; - Item_copy_string(Item *i) :item(i) + + /** + Stores the result type of the original item, so it can be returned + without calling the original item's method + */ + Item_result cached_result_type; + + /** + Constructor of the Item_copy class + + stores metadata information about the original class as well as a + pointer to it. + */ + Item_copy(Item *i) { + item= i; null_value=maybe_null=item->maybe_null; decimals=item->decimals; max_length=item->max_length; name=item->name; cached_field_type= item->field_type(); + cached_result_type= item->result_type(); + unsigned_flag= item->unsigned_flag; } + +public: + /** + Factory method to create the appropriate subclass dependent on the type of + the original item. + + @param item the original item. + */ + static Item_copy *create (Item *item); + + /** + Update the cache with the value of the original item + + This is the method that updates the cached value. + It must be explicitly called by the user of this class to store the value + of the orginal item in the cache. + */ + virtual void copy() = 0; + + Item *get_item() { return item; } + /** All of the subclasses should have the same type tag */ enum Type type() const { return COPY_STR_ITEM; } - enum Item_result result_type () const { return STRING_RESULT; } enum_field_types field_type() const { return cached_field_type; } - double val_real() - { - int err_not_used; - char *end_not_used; - return (null_value ? 0.0 : - my_strntod(str_value.charset(), (char*) str_value.ptr(), - str_value.length(), &end_not_used, &err_not_used)); - } - longlong val_int() - { - int err; - return null_value ? LL(0) : my_strntoll(str_value.charset(),str_value.ptr(), - str_value.length(),10, (char**) 0, - &err); - } - String *val_str(String*); - my_decimal *val_decimal(my_decimal *); + enum Item_result result_type () const { return cached_result_type; } + void make_field(Send_field *field) { item->make_field(field); } - void copy(); - int save_in_field(Field *field, bool no_conversions) - { - return save_str_value_in_field(field, &str_value); - } table_map used_tables() const { return (table_map) 1L; } bool const_item() const { return 0; } bool is_null() { return null_value; } + + /* + Override the methods below as pure virtual to make sure all the + sub-classes implement them. + */ + + virtual String *val_str(String*) = 0; + virtual my_decimal *val_decimal(my_decimal *) = 0; + virtual double val_real() = 0; + virtual longlong val_int() = 0; + virtual int save_in_field(Field *field, bool no_conversions) = 0; +}; + +/** + Implementation of a string cache. + + Uses Item::str_value for storage +*/ +class Item_copy_string : public Item_copy +{ +public: + Item_copy_string (Item *item) : Item_copy(item) {} + + String *val_str(String*); + my_decimal *val_decimal(my_decimal *); + double val_real(); + longlong val_int(); + void copy(); + int save_in_field(Field *field, bool no_conversions); +}; + + +class Item_copy_int : public Item_copy +{ +protected: + longlong cached_value; +public: + Item_copy_int (Item *i) : Item_copy(i) {} + int save_in_field(Field *field, bool no_conversions); + + virtual String *val_str(String*); + virtual my_decimal *val_decimal(my_decimal *); + virtual double val_real() + { + return null_value ? 0.0 : (double) cached_value; + } + virtual longlong val_int() + { + return null_value ? LL(0) : cached_value; + } + virtual void copy(); +}; + + +class Item_copy_uint : public Item_copy_int +{ +public: + Item_copy_uint (Item *item) : Item_copy_int(item) + { + unsigned_flag= 1; + } + + String *val_str(String*); + double val_real() + { + return null_value ? 0.0 : (double) (ulonglong) cached_value; + } +}; + + +class Item_copy_float : public Item_copy +{ +protected: + double cached_value; +public: + Item_copy_float (Item *i) : Item_copy(i) {} + int save_in_field(Field *field, bool no_conversions); + + String *val_str(String*); + my_decimal *val_decimal(my_decimal *); + double val_real() + { + return null_value ? 0.0 : cached_value; + } + longlong val_int() + { + return (longlong) rint(val_real()); + } + void copy() + { + cached_value= item->val_real(); + null_value= item->null_value; + } +}; + + +class Item_copy_decimal : public Item_copy +{ +protected: + my_decimal cached_value; +public: + Item_copy_decimal (Item *i) : Item_copy(i) {} + int save_in_field(Field *field, bool no_conversions); + + String *val_str(String*); + my_decimal *val_decimal(my_decimal *) + { + return null_value ? NULL: &cached_value; + } + double val_real(); + longlong val_int(); + void copy(); }; diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index a9bfea1b806..ee823517df8 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -2724,16 +2724,6 @@ 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++) { diff --git a/sql/sql_select.cc b/sql/sql_select.cc index ab9c060c69c..fad533986d6 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -13855,7 +13855,7 @@ SORT_FIELD *make_unireg_sortorder(ORDER *order, uint *length, pos->field= ((Item_sum*) item)->get_tmp_table_field(); else if (item->type() == Item::COPY_STR_ITEM) { // Blob patch - pos->item= ((Item_copy_string*) item)->item; + pos->item= ((Item_copy*) item)->get_item(); } else pos->item= *order->item; @@ -14926,7 +14926,7 @@ setup_copy_fields(THD *thd, TMP_TABLE_PARAM *param, pos= item; if (item->field->flags & BLOB_FLAG) { - if (!(pos= new Item_copy_string(pos))) + if (!(pos= Item_copy::create(pos))) goto err; /* Item_copy_string::copy for function can call @@ -14980,7 +14980,7 @@ setup_copy_fields(THD *thd, TMP_TABLE_PARAM *param, on how the value is to be used: In some cases this may be an argument in a group function, like: IF(ISNULL(col),0,COUNT(*)) */ - if (!(pos=new Item_copy_string(pos))) + if (!(pos= Item_copy::create(pos))) goto err; if (i < border) // HAVING, ORDER and GROUP BY { @@ -15033,8 +15033,8 @@ copy_fields(TMP_TABLE_PARAM *param) (*ptr->do_copy)(ptr); List_iterator_fast it(param->copy_funcs); - Item_copy_string *item; - while ((item = (Item_copy_string*) it++)) + Item_copy *item; + while ((item = (Item_copy*) it++)) item->copy(); }