Skip to content

Commit

Permalink
MDEV-8918 Wrong result for CAST(AVG(bigint_column) AS SIGNED)
Browse files Browse the repository at this point in the history 
- Moving Item_xxx_field declarations after Item_sum_xxx declarations,
  so Item_xxx_field constructors can be defined directly in item_sum.h
  rather than item_sum.cc. This removes some duplicate code, e.g.
  initialization of the following members at constructor time:
  name, decimals, max_length, unsigned_flag, field, maybe_null.
- Adding Item_sum_field as a common parent for Item_avg_field and
  Item_variance_field
- Deriving Item_sum_field directly from Item rather that Item_result_field,
  as Item_sum_field descendants do not need anything from Item_result_field.
- Removing hybrid infrastructure from Item_avg_field,
  adding Item_avg_field_decimal and Item_avg_field_double instead,
  as desired result type is already known at constructor time
  (not only at fix_fields time). This simplifies the code.
- Changing Item_avg_field_decimal::val_int() to call val_int_from_decimal()
  instead of doing { return (longlong) rint(val_real()); }
  This is the fix itself.
  • Loading branch information
Alexander Barkov committed Oct 8, 2015
1 parent 174a0b9 commit 7091b78
Show file tree
Hide file tree
Showing 4 changed files with 148 additions and 129 deletions.
17 changes: 17 additions & 0 deletions mysql-test/r/func_group.result
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2283,3 +2283,20 @@ Warnings:
Warning 1292 Truncated incorrect INTEGER value: 'x'
Warning 1292 Truncated incorrect DOUBLE value: 'x'
Warning 1292 Truncated incorrect DECIMAL value: 'x'
#
# MDEV-8918 Wrong result for CAST(AVG(a) AS SIGNED)
#
CREATE TABLE t1 (id INT, a BIGINT);
INSERT INTO t1 VALUES (1,0x7FFFFFFFFFFFFFFF),(2,0x7FFFFFFFFFFFFFFF);
SELECT id, AVG(a) AS avg, CAST(MIN(a) AS SIGNED) AS cast_min FROM t1 GROUP BY id HAVING avg!=123 ORDER BY id;
id avg cast_min
1 9223372036854775807.0000 9223372036854775807
2 9223372036854775807.0000 9223372036854775807
SELECT id, AVG(a) AS avg, CAST(AVG(a) AS SIGNED) AS cast_avg FROM t1 GROUP BY id HAVING avg!=123 ORDER BY id;
id avg cast_avg
1 9223372036854775807.0000 9223372036854775807
2 9223372036854775807.0000 9223372036854775807
DROP TABLE t1;
#
# End of 10.1 tests
#
14 changes: 14 additions & 0 deletions mysql-test/t/func_group.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1570,3 +1570,17 @@ DROP TABLE t1,t2,t3,t4,t5,t6;
--echo # MDEV-8852 Implicit or explicit CAST from MAX(string) to INT,DOUBLE,DECIMAL does not produce warnings
--echo #
SELECT MAX('x') << 1, CAST(MAX('x') AS DOUBLE), CAST(MAX('x') AS DECIMAL);


--echo #
--echo # MDEV-8918 Wrong result for CAST(AVG(a) AS SIGNED)
--echo #
CREATE TABLE t1 (id INT, a BIGINT);
INSERT INTO t1 VALUES (1,0x7FFFFFFFFFFFFFFF),(2,0x7FFFFFFFFFFFFFFF);
SELECT id, AVG(a) AS avg, CAST(MIN(a) AS SIGNED) AS cast_min FROM t1 GROUP BY id HAVING avg!=123 ORDER BY id;
SELECT id, AVG(a) AS avg, CAST(AVG(a) AS SIGNED) AS cast_avg FROM t1 GROUP BY id HAVING avg!=123 ORDER BY id;
DROP TABLE t1;

--echo #
--echo # End of 10.1 tests
--echo #
69 changes: 6 additions & 63 deletions sql/item_sum.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2502,7 +2502,10 @@ void Item_sum_avg::update_field()

Item *Item_sum_avg::result_item(THD *thd, Field *field)
{
return new (thd->mem_root) Item_avg_field(thd, hybrid_type, this);
return
hybrid_type == DECIMAL_RESULT ?
(Item_avg_field*) new (thd->mem_root) Item_avg_field_decimal(thd, this) :
(Item_avg_field*) new (thd->mem_root) Item_avg_field_double(thd, this);
}


Expand Down Expand Up @@ -2620,36 +2623,13 @@ Item_sum_hybrid::min_max_update_decimal_field()
}


Item_avg_field::Item_avg_field(THD *thd, Item_result res_type,
Item_sum_avg *item):
Item_result_field(thd)
{
name=item->name;
decimals=item->decimals;
max_length= item->max_length;
unsigned_flag= item->unsigned_flag;
field=item->result_field;
maybe_null=1;
hybrid_type= res_type;
prec_increment= item->prec_increment;
if (hybrid_type == DECIMAL_RESULT)
{
f_scale= item->f_scale;
f_precision= item->f_precision;
dec_bin_size= item->dec_bin_size;
}
}

double Item_avg_field::val_real()
double Item_avg_field_double::val_real()
{
// fix_fields() never calls for this Item
double nr;
longlong count;
uchar *res;

if (hybrid_type == DECIMAL_RESULT)
return val_real_from_decimal();

float8get(nr,field->ptr);
res= (field->ptr+sizeof(double));
count= sint8korr(res);
Expand All @@ -2660,18 +2640,9 @@ double Item_avg_field::val_real()
}


longlong Item_avg_field::val_int()
{
return (longlong) rint(val_real());
}


my_decimal *Item_avg_field::val_decimal(my_decimal *dec_buf)
my_decimal *Item_avg_field_decimal::val_decimal(my_decimal *dec_buf)
{
// fix_fields() never calls for this Item
if (hybrid_type == REAL_RESULT)
return val_decimal_from_real(dec_buf);

longlong count= sint8korr(field->ptr + dec_bin_size);
if ((null_value= !count))
return 0;
Expand All @@ -2686,21 +2657,6 @@ my_decimal *Item_avg_field::val_decimal(my_decimal *dec_buf)
}


String *Item_avg_field::val_str(String *str)
{
// fix_fields() never calls for this Item
if (hybrid_type == DECIMAL_RESULT)
return val_string_from_decimal(str);
return val_string_from_real(str);
}


Item_std_field::Item_std_field(THD *thd, Item_sum_std *item):
Item_variance_field(thd, item)
{
}


double Item_std_field::val_real()
{
double nr;
Expand All @@ -2711,19 +2667,6 @@ double Item_std_field::val_real()
}


Item_variance_field::Item_variance_field(THD *thd, Item_sum_variance *item):
Item_result_field(thd)
{
name=item->name;
decimals=item->decimals;
max_length=item->max_length;
unsigned_flag= item->unsigned_flag;
field=item->result_field;
maybe_null=1;
sample= item->sample;
}


double Item_variance_field::val_real()
{
// fix_fields() never calls for this Item
Expand Down
177 changes: 111 additions & 66 deletions sql/item_sum.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -820,37 +820,6 @@ class Item_sum_count :public Item_sum_int
};


/* Item to get the value of a stored sum function */

class Item_sum_avg;

class Item_avg_field :public Item_result_field
{
Field *field;
Item_result hybrid_type;
uint f_precision, f_scale, dec_bin_size;
uint prec_increment;
public:
Item_avg_field(THD *thd, Item_result res_type, Item_sum_avg *item);
enum Type type() const { return FIELD_AVG_ITEM; }
double val_real();
longlong val_int();
my_decimal *val_decimal(my_decimal *);
bool is_null() { update_null_value(); return null_value; }
String *val_str(String*);
enum_field_types field_type() const
{
return hybrid_type == DECIMAL_RESULT ?
MYSQL_TYPE_NEWDECIMAL : MYSQL_TYPE_DOUBLE;
}
enum Item_result result_type () const { return hybrid_type; }
bool check_vcol_func_processor(uchar *int_arg)
{
return trace_unsupported_by_check_vcol_func_processor("avg_field");
}
};


class Item_sum_avg :public Item_sum_sum
{
public:
Expand Down Expand Up @@ -894,31 +863,6 @@ class Item_sum_avg :public Item_sum_sum
}
};

class Item_sum_variance;

class Item_variance_field :public Item_result_field
{
Field *field;
uint sample;
public:
Item_variance_field(THD *thd, Item_sum_variance *item);
enum Type type() const {return FIELD_VARIANCE_ITEM; }
double val_real();
longlong val_int()
{ /* can't be fix_fields()ed */ return (longlong) rint(val_real()); }
String *val_str(String *str)
{ return val_string_from_real(str); }
my_decimal *val_decimal(my_decimal *dec_buf)
{ return val_decimal_from_real(dec_buf); }
bool is_null() { update_null_value(); return null_value; }
enum_field_types field_type() const { return MYSQL_TYPE_DOUBLE; }
enum Item_result result_type () const { return REAL_RESULT; }
bool check_vcol_func_processor(uchar *int_arg)
{
return trace_unsupported_by_check_vcol_func_processor("var_field");
}
};


/*
variance(a) =
Expand Down Expand Up @@ -977,16 +921,6 @@ class Item_sum_variance : public Item_sum_num
}
};

class Item_sum_std;

class Item_std_field :public Item_variance_field
{
public:
Item_std_field(THD *thd, Item_sum_std *item);
enum Type type() const { return FIELD_STD_ITEM; }
double val_real();
};

/*
standard_deviation(a) = sqrt(variance(a))
*/
Expand Down Expand Up @@ -1144,6 +1078,117 @@ class Item_sum_xor :public Item_sum_bit
};


/* Items to get the value of a stored sum function */

class Item_sum_field :public Item
{
protected:
Field *field;
public:
Item_sum_field(THD *thd, Item_sum *item)
:Item(thd), field(item->result_field)
{
name= item->name;
maybe_null= true;
decimals= item->decimals;
max_length= item->max_length;
unsigned_flag= item->unsigned_flag;
}
table_map used_tables() const { return (table_map) 1L; }
Field *get_tmp_table_field() { DBUG_ASSERT(0); return NULL; }
Field *tmp_table_field(TABLE *) { DBUG_ASSERT(0); return NULL; }
void set_result_field(Field *) { DBUG_ASSERT(0); }
void save_in_result_field(bool no_conversions) { DBUG_ASSERT(0); }
};


class Item_avg_field :public Item_sum_field
{
protected:
uint prec_increment;
public:
Item_avg_field(THD *thd, Item_sum_avg *item)
:Item_sum_field(thd, item), prec_increment(item->prec_increment)
{ }
enum Type type() const { return FIELD_AVG_ITEM; }
bool is_null() { update_null_value(); return null_value; }
bool check_vcol_func_processor(uchar *int_arg)
{
return trace_unsupported_by_check_vcol_func_processor("avg_field");
}
};


class Item_avg_field_double :public Item_avg_field
{
public:
Item_avg_field_double(THD *thd, Item_sum_avg *item)
:Item_avg_field(thd, item)
{ }
enum_field_types field_type() const { return MYSQL_TYPE_DOUBLE; }
enum Item_result result_type () const { return REAL_RESULT; }
longlong val_int() { return (longlong) rint(val_real()); }
my_decimal *val_decimal(my_decimal *dec) { return val_decimal_from_real(dec); }
String *val_str(String *str) { return val_string_from_real(str); }
double val_real();
};


class Item_avg_field_decimal :public Item_avg_field
{
uint f_precision, f_scale, dec_bin_size;
public:
Item_avg_field_decimal(THD *thd, Item_sum_avg *item)
:Item_avg_field(thd, item),
f_precision(item->f_precision),
f_scale(item->f_scale),
dec_bin_size(item->dec_bin_size)
{ }
enum_field_types field_type() const { return MYSQL_TYPE_NEWDECIMAL; }
enum Item_result result_type () const { return DECIMAL_RESULT; }
double val_real() { return val_real_from_decimal(); }
longlong val_int() { return val_int_from_decimal(); }
String *val_str(String *str) { return val_string_from_decimal(str); }
my_decimal *val_decimal(my_decimal *);
};


class Item_variance_field :public Item_sum_field
{
uint sample;
public:
Item_variance_field(THD *thd, Item_sum_variance *item)
:Item_sum_field(thd, item), sample(item->sample)
{ }
enum Type type() const {return FIELD_VARIANCE_ITEM; }
double val_real();
longlong val_int()
{ /* can't be fix_fields()ed */ return (longlong) rint(val_real()); }
String *val_str(String *str)
{ return val_string_from_real(str); }
my_decimal *val_decimal(my_decimal *dec_buf)
{ return val_decimal_from_real(dec_buf); }
bool is_null() { update_null_value(); return null_value; }
enum_field_types field_type() const { return MYSQL_TYPE_DOUBLE; }
enum Item_result result_type () const { return REAL_RESULT; }
bool check_vcol_func_processor(uchar *int_arg)
{
return trace_unsupported_by_check_vcol_func_processor("var_field");
}
};


class Item_std_field :public Item_variance_field
{
public:
Item_std_field(THD *thd, Item_sum_std *item)
:Item_variance_field(thd, item)
{ }
enum Type type() const { return FIELD_STD_ITEM; }
double val_real();
};


/*
User defined aggregates
*/
Expand Down

0 comments on commit 7091b78

Please sign in to comment.