MDEV-8918 Wrong result for CAST(AVG(bigint_column) AS SIGNED)

- 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.

Author: Alexander Barkov

mysql-test/r/func_group.result

@@ -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
+#

mysql-test/t/func_group.test

@@ -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 #

sql/item_sum.cc

@@ -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);
 }
 
 
@@ -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);
@@ -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;
@@ -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;
@@ -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

sql/item_sum.h

@@ -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:
@@ -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) =
@@ -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))
 */
@@ -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
 */