Bug #45261 : Crash, stored procedure + decimal

Bug #48370  Absolutely wrong calculations with GROUP BY and
  decimal fields when using IF

Added the test cases in the above two bugs for regression
testing.
Added additional tests that demonstrate a incomplete fix.
Added a new factory method for Field_new_decimal to 
create a field from an (decimal returning) Item.
In the new method made sure that all the precision and 
length variables are capped in a proper way. 
This is required because Item's can have larger precision
than the decimal fields and thus need to be capped when
creating a field based on an Item type.
Fixed the wrong typecast to Item_decimal.

6 files changed, 50 insertions, 85 deletions

diff --git a/sql/field.cc b/sql/field.cc
index 354c911e1c0..01ccc338782 100644
--- a/sql/field.cc
+++ b/sql/field.cc
@@ -2486,6 +2486,50 @@ Field_new_decimal::Field_new_decimal(uint32 len_arg,
 }
 
 
+Field *Field_new_decimal::create_from_item (Item *item)
+{
+  uint8 dec= item->decimals;
+  uint8 intg= item->decimal_precision() - dec;
+  uint32 len= item->max_length;
+
+  DBUG_ASSERT (item->result_type() == DECIMAL_RESULT);
+
+  /*
+    Trying to put too many digits overall in a DECIMAL(prec,dec)
+    will always throw a warning. We must limit dec to
+    DECIMAL_MAX_SCALE however to prevent an assert() later.
+  */
+
+  if (dec > 0)
+  {
+    signed int overflow;
+
+    dec= min(dec, DECIMAL_MAX_SCALE);
+
+    /*
+      If the value still overflows the field with the corrected dec,
+      we'll throw out decimals rather than integers. This is still
+      bad and of course throws a truncation warning.
+      +1: for decimal point
+      */
+
+    const int required_length=
+      my_decimal_precision_to_length(intg + dec, dec,
+                                     item->unsigned_flag);
+
+    overflow= required_length - len;
+
+    if (overflow > 0)
+      dec= max(0, dec - overflow);            // too long, discard fract
+    else
+      /* Corrected value fits. */
+      len= required_length;
+  }
+  return new Field_new_decimal(len, item->maybe_null, item->name,
+                               dec, item->unsigned_flag);
+}
+
+
 int Field_new_decimal::reset(void)
 {
   store_value(&decimal_zero);
diff --git a/sql/field.h b/sql/field.h
index 784b9133790..ae074cc1a30 100644
--- a/sql/field.h
+++ b/sql/field.h
@@ -807,6 +807,7 @@ public:
   uint is_equal(Create_field *new_field);
   virtual const uchar *unpack(uchar* to, const uchar *from,
                               uint param_data, bool low_byte_first);
+  static Field *create_from_item (Item *);
 };
 
 
diff --git a/sql/item.cc b/sql/item.cc
index e1dbc1ba21a..ab70996e541 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -4899,9 +4899,7 @@ Field *Item::tmp_table_field_from_field_type(TABLE *table, bool fixed_length)
   switch (field_type()) {
   case MYSQL_TYPE_DECIMAL:
   case MYSQL_TYPE_NEWDECIMAL:
-    field= new Field_new_decimal((uchar*) 0, max_length, null_ptr, 0,
-                                 Field::NONE, name, decimals, 0,
-                                 unsigned_flag);
+    field= Field_new_decimal::create_from_item(this);
     break;
   case MYSQL_TYPE_TINY:
     field= new Field_tiny((uchar*) 0, max_length, null_ptr, 0, Field::NONE,
diff --git a/sql/item_func.cc b/sql/item_func.cc
index ac52f36474a..977a0de39af 100644
--- a/sql/item_func.cc
+++ b/sql/item_func.cc
@@ -451,45 +451,8 @@ Field *Item_func::tmp_table_field(TABLE *table)
     return make_string_field(table);
     break;
   case DECIMAL_RESULT:
-  {
-    uint8 dec= decimals;
-    uint8 intg= decimal_precision() - dec;
-    uint32 len= max_length;
-
-    /*
-      Trying to put too many digits overall in a DECIMAL(prec,dec)
-      will always throw a warning. We must limit dec to
-      DECIMAL_MAX_SCALE however to prevent an assert() later.
-    */
-
-    if (dec > 0)
-    {
-      int overflow;
-
-      dec= min(dec, DECIMAL_MAX_SCALE);
-
-      /*
-        If the value still overflows the field with the corrected dec,
-        we'll throw out decimals rather than integers. This is still
-        bad and of course throws a truncation warning.
-      */
-
-      const int required_length=
-        my_decimal_precision_to_length(intg + dec, dec,
-                                                     unsigned_flag);
-
-      overflow= required_length - len;
-
-      if (overflow > 0)
-        dec= max(0, dec - overflow);            // too long, discard fract
-      else
-        /* Corrected value fits. */
-        len= required_length;
-    }
-
-    field= new Field_new_decimal(len, maybe_null, name, dec, unsigned_flag);
+    field= Field_new_decimal::create_from_item(this);
     break;
-  }
   case ROW_RESULT:
   default:
     // This case should never be chosen
diff --git a/sql/item_sum.cc b/sql/item_sum.cc
index 38251294053..7bbe167e7ac 100644
--- a/sql/item_sum.cc
+++ b/sql/item_sum.cc
@@ -517,8 +517,7 @@ Field *Item_sum::create_tmp_field(bool group, TABLE *table,
                                name, table->s, collation.collation);
     break;
   case DECIMAL_RESULT:
-    field= new Field_new_decimal(max_length, maybe_null, name,
-                                 decimals, unsigned_flag);
+    field= Field_new_decimal::create_from_item(this);
     break;
   case ROW_RESULT:
   default:
@@ -1270,8 +1269,7 @@ Field *Item_sum_avg::create_tmp_field(bool group, TABLE *table,
                             0, name, &my_charset_bin);
   }
   else if (hybrid_type == DECIMAL_RESULT)
-    field= new Field_new_decimal(max_length, maybe_null, name,
-                                 decimals, unsigned_flag);
+    field= Field_new_decimal::create_from_item(this);
   else
     field= new Field_double(max_length, maybe_null, name, decimals, TRUE);
   if (field)
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index 18fc3b20748..569d8183ab6 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -9466,47 +9466,8 @@ static Field *create_tmp_field_from_item(THD *thd, Item *item, TABLE *table,
     new_field->set_derivation(item->collation.derivation);
     break;
   case DECIMAL_RESULT:
-  {
-    uint8 dec= item->decimals;
-    uint8 intg= ((Item_decimal *) item)->decimal_precision() - dec;
-    uint32 len= item->max_length;
-
-    /*
-      Trying to put too many digits overall in a DECIMAL(prec,dec)
-      will always throw a warning. We must limit dec to
-      DECIMAL_MAX_SCALE however to prevent an assert() later.
-    */
-
-    if (dec > 0)
-    {
-      signed int overflow;
-
-      dec= min(dec, DECIMAL_MAX_SCALE);
-
-      /*
-        If the value still overflows the field with the corrected dec,
-        we'll throw out decimals rather than integers. This is still
-        bad and of course throws a truncation warning.
-        +1: for decimal point
-      */
-
-      const int required_length=
-        my_decimal_precision_to_length(intg + dec, dec,
-                                                     item->unsigned_flag);
-
-      overflow= required_length - len;
-
-      if (overflow > 0)
-        dec= max(0, dec - overflow);            // too long, discard fract
-      else
-        /* Corrected value fits. */
-        len= required_length;
-    }
-
-    new_field= new Field_new_decimal(len, maybe_null, item->name,
-                                     dec, item->unsigned_flag);
+    new_field= Field_new_decimal::create_from_item(item);
     break;
-  }
   case ROW_RESULT:
   default:
     // This case should never be choosen