Bug#45261: Crash, stored procedure + decimal

The problem was that creating a DECIMAL column from a decimal
value could lead to a failed assertion as decimal values can
have a higher precision than those attached to a table. The
assert could be triggered by creating a table from a decimal
with a large (> 30) scale. Also, there was a problem in
calculating the number of digits in the integral and fractional
parts if both exceeded the maximum number of digits permitted
by the new decimal type.

The solution is to ensure that truncation procedure is executed
when deducing a DECIMAL column from a decimal value of higher
precision. If the integer part is equal to or bigger than the
maximum precision for the DECIMAL type (65), the integer part
is truncated to fit and the fractional becomes zero. Otherwise,
the fractional part is truncated to fit into the space left
after the integer part is copied.

This patch borrows code and ideas from Martin Hansson's patch.

mysql-test/r/type_newdecimal.result:
  Add test case result for Bug#45261. Also, update test case to
  reflect that an additive operation increases the precision of
  the resulting type by 1.
mysql-test/t/type_newdecimal.test:
  Add test case for Bug#45261
sql/field.cc:
  Added DBUG_ASSERT to ensure object's invariant is maintained.
  Implement method to create a field to hold a decimal value
  from an item.
sql/field.h:
  Explain member variable. Add method to create a new decimal field.
sql/item.cc:
  The precision should only be capped when storing the value
  on a table. Also, this makes it impossible to calculate the
  integer part if Item::decimals (the scale) is larger than the
  precision.
sql/item.h:
  Simplify calculation of integer part.
sql/item_cmpfunc.cc:
  Do not limit the precision. It will be capped later.
sql/item_func.cc:
  Use new method for allocating a new decimal field.
  Add a specialized method for retrieving the precision
  of a user variable item.
sql/item_func.h:
  Add method to return the precision of a user variable.
sql/item_sum.cc:
  Use new method for allocating a new decimal field.
sql/my_decimal.h:
  The integer part could be improperly calculated for a decimal
  with 31 digits in the fractional part.
sql/sql_select.cc:
  Use new method which truncates the integer or decimal parts
  as needed.

10 files changed, 137 insertions, 102 deletions

diff --git a/sql/field.cc b/sql/field.cc
index 3bfb54fbd15..426effa57cd 100644
--- a/sql/field.cc
+++ b/sql/field.cc
@@ -2485,12 +2485,97 @@ Field_new_decimal::Field_new_decimal(uint32 len_arg,
 {
   precision= my_decimal_length_to_precision(len_arg, dec_arg, unsigned_arg);
   set_if_smaller(precision, DECIMAL_MAX_PRECISION);
+  DBUG_ASSERT(precision >= dec);
   DBUG_ASSERT((precision <= DECIMAL_MAX_PRECISION) &&
               (dec <= DECIMAL_MAX_SCALE));
   bin_size= my_decimal_get_binary_size(precision, dec);
 }
 
 
+/**
+  Create a field to hold a decimal value from an item.
+
+  @remark The MySQL DECIMAL data type has a characteristic that needs to be
+          taken into account when deducing the type from a Item_decimal.
+
+  But first, let's briefly recap what is the new MySQL DECIMAL type:
+
+  The declaration syntax for a decimal is DECIMAL(M,D), where:
+
+  * M is the maximum number of digits (the precision).
+    It has a range of 1 to 65.
+  * D is the number of digits to the right of the decimal separator (the scale).
+    It has a range of 0 to 30 and must be no larger than M.
+
+  D and M are used to determine the storage requirements for the integer
+  and fractional parts of each value. The integer part is to the left of
+  the decimal separator and to the right is the fractional part. Hence:
+
+  M is the number of digits for the integer and fractional part.
+  D is the number of digits for the fractional part.
+
+  Consequently, M - D is the number of digits for the integer part. For
+  example, a DECIMAL(20,10) column has ten digits on either side of
+  the decimal separator.
+
+  The characteristic that needs to be taken into account is that the
+  backing type for Item_decimal is a my_decimal that has a higher
+  precision (DECIMAL_MAX_POSSIBLE_PRECISION, see my_decimal.h) than
+  DECIMAL.
+
+  Drawing a comparison between my_decimal and DECIMAL:
+
+  * M has a range of 1 to 81.
+  * D has a range of 0 to 81.
+
+  There can be a difference in range if the decimal contains a integer
+  part. This is because the fractional part must always be on a group
+  boundary, leaving at least one group for the integer part. Since each
+  group is 9 (DIG_PER_DEC1) digits and there are 9 (DECIMAL_BUFF_LENGTH)
+  groups, the fractional part is limited to 72 digits if there is at
+  least one digit in the integral part.
+
+  Although the backing type for a DECIMAL is also my_decimal, every
+  time a my_decimal is stored in a DECIMAL field, the precision and
+  scale are explicitly capped at 65 (DECIMAL_MAX_PRECISION) and 30
+  (DECIMAL_MAX_SCALE) digits, following my_decimal truncation procedure
+  (FIX_INTG_FRAC_ERROR).
+*/
+
+Field_new_decimal *
+Field_new_decimal::new_decimal_field(const Item *item)
+{
+  uint32 len;
+  uint intg= item->decimal_int_part(), scale= item->decimals;
+
+  DBUG_ASSERT(item->decimal_precision() >= item->decimals);
+
+  /*
+    Employ a procedure along the lines of the my_decimal truncation process:
+    - If the integer part is equal to or bigger than the maximum precision:
+      Truncate integer part to fit and the fractional becomes zero.
+    - Otherwise:
+      Truncate fractional part to fit.
+  */
+  if (intg >= DECIMAL_MAX_PRECISION)
+  {
+    intg= DECIMAL_MAX_PRECISION;
+    scale= 0;
+  }
+  else
+  {
+    uint room= min(DECIMAL_MAX_PRECISION - intg, DECIMAL_MAX_SCALE);
+    if (scale > room)
+      scale= room;
+  }
+
+  len= my_decimal_precision_to_length(intg + scale, scale, item->unsigned_flag);
+
+  return new Field_new_decimal(len, item->maybe_null, item->name, scale,
+                               item->unsigned_flag);
+}
+
+
 int Field_new_decimal::reset(void)
 {
   store_value(&decimal_zero);
diff --git a/sql/field.h b/sql/field.h
index 5136f760fc1..a9299256f88 100644
--- a/sql/field.h
+++ b/sql/field.h
@@ -608,6 +608,10 @@ protected:
 
 class Field_num :public Field {
 public:
+  /**
+     The scale of the Field's value, i.e. the number of digits to the right
+     of the decimal point.
+  */
   const uint8 dec;
   bool zerofill,unsigned_flag;	// Purify cannot handle bit fields
   Field_num(uchar *ptr_arg,uint32 len_arg, uchar *null_ptr_arg,
@@ -766,6 +770,11 @@ public:
   Field_new_decimal(uint32 len_arg, bool maybe_null_arg,
                     const char *field_name_arg, uint8 dec_arg,
                     bool unsigned_arg);
+  /*
+    Create a field to hold a decimal value from an item.
+    Truncates the precision and/or scale if necessary.
+  */
+  static Field_new_decimal *new_decimal_field(const Item *item);
   enum_field_types type() const { return MYSQL_TYPE_NEWDECIMAL;}
   enum ha_base_keytype key_type() const { return HA_KEYTYPE_BINARY; }
   Item_result result_type () const { return DECIMAL_RESULT; }
diff --git a/sql/item.cc b/sql/item.cc
index 2640b74851b..9551c5feaff 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -435,17 +435,26 @@ Item::Item(THD *thd, Item *item):
 }
 
 
+/**
+  Decimal precision of the item.
+
+  @remark The precision must not be capped as it can be used in conjunction
+          with Item::decimals to determine the size of the integer part when
+          constructing a decimal data type.
+
+  @see Item::decimal_int_part()
+  @see Item::decimals
+*/
+
 uint Item::decimal_precision() const
 {
+  uint precision= max_length;
   Item_result restype= result_type();
 
   if ((restype == DECIMAL_RESULT) || (restype == INT_RESULT))
-  {
-    uint prec= 
-      my_decimal_length_to_precision(max_length, decimals, unsigned_flag);
-    return min(prec, DECIMAL_MAX_PRECISION);
-  }
-  return min(max_length, DECIMAL_MAX_PRECISION);
+    precision= my_decimal_length_to_precision(max_length, decimals, unsigned_flag);
+
+  return precision;
 }
 
 
@@ -4902,9 +4911,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::new_decimal_field(this);
     break;
   case MYSQL_TYPE_TINY:
     field= new Field_tiny((uchar*) 0, max_length, null_ptr, 0, Field::NONE,
diff --git a/sql/item.h b/sql/item.h
index 3dfcd7c2612..464085cb101 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -755,9 +755,10 @@ public:
   virtual cond_result eq_cmp_result() const { return COND_OK; }
   inline uint float_length(uint decimals_par) const
   { return decimals != NOT_FIXED_DEC ? (DBL_DIG+2+decimals_par) : DBL_DIG+8;}
+  /** Returns the uncapped decimal precision of this item. */
   virtual uint decimal_precision() const;
   inline int decimal_int_part() const
-  { return my_decimal_int_part(decimal_precision(), decimals); }
+  { return decimal_precision() - decimals; }
   /* 
     Returns true if this is constant (during query execution, i.e. its value
     will not change until next fix_fields) and its value is known.
diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index 1ff9ca6a419..53feb753844 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -2183,7 +2183,7 @@ uint Item_func_ifnull::decimal_precision() const
   int arg1_int_part= args[1]->decimal_int_part();
   int max_int_part= max(arg0_int_part, arg1_int_part);
   int precision= max_int_part + decimals;
-  return min(precision, DECIMAL_MAX_PRECISION);
+  return precision;
 }
 
 
@@ -2367,7 +2367,7 @@ uint Item_func_if::decimal_precision() const
   int arg1_prec= args[1]->decimal_int_part();
   int arg2_prec= args[2]->decimal_int_part();
   int precision=max(arg1_prec,arg2_prec) + decimals;
-  return min(precision, DECIMAL_MAX_PRECISION);
+  return precision;
 }
 
 
@@ -2775,7 +2775,7 @@ uint Item_func_case::decimal_precision() const
 
   if (else_expr_num != -1) 
     set_if_bigger(max_int_part, args[else_expr_num]->decimal_int_part());
-  return min(max_int_part + decimals, DECIMAL_MAX_PRECISION);
+  return max_int_part + decimals;
 }
 
 
diff --git a/sql/item_func.cc b/sql/item_func.cc
index 55d4b37ddb0..6abc78371db 100644
--- a/sql/item_func.cc
+++ b/sql/item_func.cc
@@ -452,45 +452,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::new_decimal_field(this);
     break;
-  }
   case ROW_RESULT:
   default:
     // This case should never be chosen
@@ -4781,6 +4744,19 @@ void Item_func_get_user_var::fix_length_and_dec()
 }
 
 
+uint Item_func_get_user_var::decimal_precision() const
+{
+  uint precision= max_length;
+  Item_result restype= result_type();
+
+  /* Default to maximum as the precision is unknown a priori. */
+  if ((restype == DECIMAL_RESULT) || (restype == INT_RESULT))
+    precision= DECIMAL_MAX_PRECISION;
+
+  return precision;
+}
+
+
 bool Item_func_get_user_var::const_item() const
 {
   return (!var_entry || current_thd->query_id != var_entry->update_query_id);
diff --git a/sql/item_func.h b/sql/item_func.h
index 025ac12fe07..fdbbff89e60 100644
--- a/sql/item_func.h
+++ b/sql/item_func.h
@@ -1393,6 +1393,7 @@ public:
   table_map used_tables() const
   { return const_item() ? 0 : RAND_TABLE_BIT; }
   bool eq(const Item *item, bool binary_cmp) const;
+  uint decimal_precision() const;
 private:
   bool set_value(THD *thd, sp_rcontext *ctx, Item **it);
 
diff --git a/sql/item_sum.cc b/sql/item_sum.cc
index 38251294053..08a48c6ce2f 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::new_decimal_field(this);
     break;
   case ROW_RESULT:
   default:
diff --git a/sql/my_decimal.h b/sql/my_decimal.h
index 21669e82c44..b1df1395dcd 100644
--- a/sql/my_decimal.h
+++ b/sql/my_decimal.h
@@ -48,10 +48,12 @@ C_MODE_END
   digits * number of decimal digits in one our big digit - number of decimal
   digits in one our big digit decreased by 1 (because we always put decimal
   point on the border of our big digits))
+
+  This value is 65 due to historical reasons partly due to it being used
+  as the maximum allowed precision and not the actual maximum precision.
 */
 #define DECIMAL_MAX_PRECISION (DECIMAL_MAX_POSSIBLE_PRECISION - 8*2)
 #define DECIMAL_MAX_SCALE 30
-#define DECIMAL_NOT_SPECIFIED 31
 
 /**
   maximum length of string representation (number of maximum decimal
@@ -75,12 +77,6 @@ inline uint my_decimal_size(uint precision, uint scale)
 }
 
 
-inline int my_decimal_int_part(uint precision, uint decimals)
-{
-  return precision - ((decimals == DECIMAL_NOT_SPECIFIED) ? 0 : decimals);
-}
-
-
 /**
   my_decimal class limits 'decimal_t' type to what we need in MySQL.
 
@@ -184,7 +180,7 @@ inline uint my_decimal_length_to_precision(uint length, uint scale,
 }
 
 inline uint32 my_decimal_precision_to_length_no_truncation(uint precision,
-                                                           uint8 scale,
+                                                           uint scale,
                                                            bool unsigned_flag)
 {
   /*
@@ -196,7 +192,7 @@ inline uint32 my_decimal_precision_to_length_no_truncation(uint precision,
                   (unsigned_flag || !precision ? 0 : 1));
 }
 
-inline uint32 my_decimal_precision_to_length(uint precision, uint8 scale,
+inline uint32 my_decimal_precision_to_length(uint precision, uint scale,
                                              bool unsigned_flag)
 {
   /*
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index 7f6c5e834a3..5cb0de1ba5c 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -9377,47 +9377,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::new_decimal_field(item);
     break;
-  }
   case ROW_RESULT:
   default:
     // This case should never be choosen