#At file:///home/kgeorge/mysql/work/B44399-5.1-bugteam/ based on revid:ramil@stripped
2899 Georgi Kodinov 2009-05-21
Bug #44399 : crash with statement using TEXT columns, aggregates, GROUP BY, and HAVING
When calculating GROUP BY the server caches some expressions. It does
that by allocating a string slot (Item_copy_string) and assigning the
value of the expression to it. This effectively means that the result
type of the expression can be changed from whatever it was to a string.
As this substitution takes place after the compile-time result type
calculation for IN but before the run-time type calculations,
it causes the type calculations in the IN function done at run time
to get unexpected results different from what was prepared at compile time.
In the CASE ... WHEN ... THEN ... statement there was a similar problem
and it was solved by artificially adding a STRING argument to the matrix
at compile time, so if any of the arguments of the CASE function changes
its type to a string it will still be covered by the information prepared
at compile time.
Fixed by:
1. removing the special case for CASE ... WHEN
2. implemented typed caching in GROUP BY.
@ mysql-test/r/func_in.result
Bug #44399: test case
@ mysql-test/t/func_in.test
Bug #44399: test case
@ sql/item.cc
Bug #44399: Implement typed cashing for GROUP BY
@ sql/item.h
Bug #44399: Implement typed cashing for GROUP BY
@ sql/item_cmpfunc.cc
Bug #44399: remove the special case
@ sql/sql_select.cc
Bug #44399: Implement typed cashing for GROUP BY
modified:
mysql-test/r/func_in.result
mysql-test/t/func_in.test
sql/item.cc
sql/item.h
sql/item_cmpfunc.cc
sql/sql_select.cc
=== modified file 'mysql-test/r/func_in.result'
--- a/mysql-test/r/func_in.result 2009-05-20 11:14:33 +0000
+++ b/mysql-test/r/func_in.result 2009-05-21 13:49:23 +0000
@@ -587,4 +587,22 @@ SELECT CASE c1 WHEN c1 + 1 THEN 1 END, A
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
+DROP TABLE t1;
End of 5.1 tests
=== modified file 'mysql-test/t/func_in.test'
--- a/mysql-test/t/func_in.test 2009-05-20 11:14:33 +0000
+++ b/mysql-test/t/func_in.test 2009-05-21 13:49:23 +0000
@@ -439,4 +439,20 @@ SELECT CASE c1 WHEN c1 + 1 THEN 1 END, A
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);
+DROP TABLE t1;
+
+
--echo End of 5.1 tests
=== modified file 'sql/item.cc'
--- a/sql/item.cc 2009-05-21 08:06:43 +0000
+++ b/sql/item.cc 2009-05-21 13:49:23 +0000
@@ -3268,9 +3268,57 @@ Item_param::set_param_type_and_swap_valu
}
/****************************************************************************
+ 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_real (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);
@@ -3293,12 +3341,163 @@ my_decimal *Item_copy_string::val_decima
{
// 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_real
+****************************************************************************/
+
+String *Item_copy_real::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_real::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_real::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)
*/
@@ -4946,10 +5145,9 @@ int Item_uint::save_in_field(Field *fiel
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();
@@ -4957,6 +5155,12 @@ int Item_int::save_in_field(Field *field
}
+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();
=== modified file 'sql/item.h'
--- a/sql/item.h 2009-04-01 11:10:03 +0000
+++ b/sql/item.h 2009-05-21 13:49:23 +0000
@@ -2443,48 +2443,140 @@ public:
#include "item_xmlfunc.h"
#endif
-class Item_copy_string :public Item
+class Item_copy :public Item
{
+protected:
enum enum_field_types cached_field_type;
-public:
Item *item;
- Item_copy_string(Item *i) :item(i)
+ Item_result cached_result_type;
+ 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:
+ static Item_copy *create (Item *item);
+
+ Item *get_item() { return item; }
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()
+ enum Item_result result_type () const { return cached_result_type; }
+
+ void make_field(Send_field *field) { item->make_field(field); }
+ table_map used_tables() const { return (table_map) 1L; }
+ bool const_item() const { return 0; }
+ bool is_null() { return null_value; }
+
+ 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 void copy() = 0;
+ virtual int save_in_field(Field *field, bool no_conversions) = 0;
+};
+
+
+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()
{
- 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));
+ return null_value ? 0.0 : (double) cached_value;
}
- longlong val_int()
+ virtual 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);
+ 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_real : public Item_copy
+{
+protected:
+ double cached_value;
+public:
+ Item_copy_real (Item *i) : Item_copy(i) {}
+ int save_in_field(Field *field, bool no_conversions);
+
String *val_str(String*);
my_decimal *val_decimal(my_decimal *);
- void make_field(Send_field *field) { item->make_field(field); }
- void copy();
- int save_in_field(Field *field, bool no_conversions)
+ double val_real()
{
- return save_str_value_in_field(field, &str_value);
+ return null_value ? 0.0 : cached_value;
}
- table_map used_tables() const { return (table_map) 1L; }
- bool const_item() const { return 0; }
- bool is_null() { return null_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();
};
=== modified file 'sql/item_cmpfunc.cc'
--- a/sql/item_cmpfunc.cc 2009-05-20 11:14:33 +0000
+++ b/sql/item_cmpfunc.cc 2009-05-21 13:49:23 +0000
@@ -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++)
{
=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc 2009-05-15 13:25:29 +0000
+++ b/sql/sql_select.cc 2009-05-21 13:49:23 +0000
@@ -13855,7 +13855,7 @@ SORT_FIELD *make_unireg_sortorder(ORDER
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_PA
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_PA
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<Item> it(param->copy_funcs);
- Item_copy_string *item;
- while ((item = (Item_copy_string*) it++))
+ Item_copy *item;
+ while ((item = (Item_copy*) it++))
item->copy();
}
Attachment: [text/bzr-bundle] bzr/joro@sun.com-20090521134923-0eo1qssl5scx7mk3.bundle
| Thread |
|---|
| • bzr commit into mysql-5.1-bugteam branch (joro:2899) Bug#44399 | Georgi Kodinov | 21 May |
