Bug #45262: Bad effects with CREATE TABLE and DECIMAL

 
Using DECIMAL constants with more than 65 digits in CREATE 
TABLE ... SELECT led to bogus errors in release builds or 
assertion failures in debug builds. 
 
The problem was in inconsistency in how DECIMAL constants and 
fields are handled internally. We allow arbitrarily long 
DECIMAL constants, whereas DECIMAL(M,D) columns are limited to 
M<=65 and D<=30. my_decimal_precision_to_length() was used in 
both Item and Field code and truncated precision to 
DECIMAL_MAX_PRECISION when calculating value length without 
adjusting precision and decimals. As a result, a DECIMAL 
constant with more than 65 digits ended up having length less 
than precision or decimals which led to assertion failures. 
 
Fixed by modifying my_decimal_precision_to_length() so that 
precision is truncated to DECIMAL_MAX_PRECISION only for Field 
object which is indicated by the new 'truncate' parameter. 
 
Another inconsistency fixed by this patch is how DECIMAL 
constants and expressions are handled for CREATE ... SELECT. 
create_tmp_field_from_item() (which is used for constants) was 
changed as a part of the bugfix for bug #24907 to handle long 
DECIMAL constants gracefully. Item_func::tmp_table_field() 
(which is used for expressions) on the other hand was still 
using a simplistic approach when creating a Field_new_decimal 
from a DECIMAL expression. 

mysql-test/r/type_newdecimal.result:
  Added a test case for bug #45262.
mysql-test/t/type_newdecimal.test:
  Added a test case for bug #45262.
sql/item.cc:
  Use the new 'truncate' parameter in 
  my_decimal_precision_to_length().
sql/item_cmpfunc.cc:
  Use the new 'truncate' parameter in 
  my_decimal_precision_to_length().
sql/item_func.cc:
  1. Use the new 'truncate' parameter in 
  my_decimal_precision_to_length().
  
  2. Do not truncate decimal precision to DECIMAL_MAX_PRECISION
  for additive expressions involving long DECIMAL constants.
  
  3. Fixed an incosistency in how DECIMAL constants and 
  expressions are handled for CREATE ... SELECT.
sql/item_func.h:
  Use the new 'truncate' parameter in 
  my_decimal_precision_to_length().
sql/item_sum.cc:
  Use the new 'truncate' parameter in 
  my_decimal_precision_to_length().
sql/my_decimal.h:
  Do not truncate precision to DECIMAL_MAX_PRECISION
  when calculating length in 
  my_decimal_precision_to_length() if 'truncate' parameter
  is FALSE.
sql/sql_select.cc:
  1. Use the new 'truncate' parameter in 
  my_decimal_precision_to_length().
  
  2. Use a more correct logic when adjusting value's length.

mysql-test/r/type_newdecimal.result

@@ -1514,10 +1514,10 @@ Warnings:
 Warning	1264	Out of range value adjusted for column 'f1' at row 1
 DESC t1;
 Field	Type	Null	Key	Default	Extra
-f1	decimal(59,30)	NO		0.000000000000000000000000000000	
+f1	decimal(65,30)	NO		0.000000000000000000000000000000	
 SELECT f1 FROM t1;
 f1
-99999999999999999999999999999.999999999999999999999999999999
+99999999999999999999999999999999999.999999999999999999999999999999
 DROP TABLE t1;
 select (1.20396873 * 0.89550000 * 0.68000000 * 1.08721696 * 0.99500000 *
 1.01500000 * 1.01500000 * 0.99500000);
@@ -1539,4 +1539,57 @@ select * from t1;
 5.05 / 0.014
 360.714286
 DROP TABLE t1;
+#
+# Bug #45262: Bad effects with CREATE TABLE and DECIMAL
+#
+CREATE TABLE t1 SELECT .123456789123456789123456789123456789123456789123456789123456789123456789123456789 AS my_col;
+Warnings:
+Note	1265	Data truncated for column 'my_col' at row 1
+DESCRIBE t1;
+Field	Type	Null	Key	Default	Extra
+my_col	decimal(30,30)	NO		0.000000000000000000000000000000	
+SELECT my_col FROM t1;
+my_col
+0.123456789123456789123456789123
+DROP TABLE t1;
+CREATE TABLE t1 SELECT 1 + .123456789123456789123456789123456789123456789123456789123456789123456789123456789 AS my_col;
+Warnings:
+Note	1265	Data truncated for column 'my_col' at row 1
+DESCRIBE t1;
+Field	Type	Null	Key	Default	Extra
+my_col	decimal(65,30)	NO		0.000000000000000000000000000000	
+SELECT my_col FROM t1;
+my_col
+1.123456789123456789123456789123
+DROP TABLE t1;
+CREATE TABLE t1 SELECT 1 * .123456789123456789123456789123456789123456789123456789123456789123456789123456789 AS my_col;
+Warnings:
+Note	1265	Data truncated for column 'my_col' at row 1
+DESCRIBE t1;
+Field	Type	Null	Key	Default	Extra
+my_col	decimal(65,30)	NO		0.000000000000000000000000000000	
+SELECT my_col FROM t1;
+my_col
+0.123456789123456789123456789123
+DROP TABLE t1;
+CREATE TABLE t1 SELECT 1 / .123456789123456789123456789123456789123456789123456789123456789123456789123456789 AS my_col;
+Warnings:
+Note	1265	Data truncated for column 'my_col' at row 1
+DESCRIBE t1;
+Field	Type	Null	Key	Default	Extra
+my_col	decimal(65,4)	YES		NULL	
+SELECT my_col FROM t1;
+my_col
+8.1000
+DROP TABLE t1;
+CREATE TABLE t1 SELECT 1 % .123456789123456789123456789123456789123456789123456789123456789123456789123456789 AS my_col;
+Warnings:
+Note	1265	Data truncated for column 'my_col' at row 1
+DESCRIBE t1;
+Field	Type	Null	Key	Default	Extra
+my_col	decimal(65,30)	YES		NULL	
+SELECT my_col FROM t1;
+my_col
+0.012345687012345687012345687012
+DROP TABLE t1;
 End of 5.0 tests

mysql-test/t/type_newdecimal.test

@@ -1235,4 +1235,33 @@ show create table t1;
 select * from t1;
 DROP TABLE t1;
 
+--echo #
+--echo # Bug #45262: Bad effects with CREATE TABLE and DECIMAL
+--echo #
+
+CREATE TABLE t1 SELECT .123456789123456789123456789123456789123456789123456789123456789123456789123456789 AS my_col;
+DESCRIBE t1;
+SELECT my_col FROM t1;
+DROP TABLE t1;
+
+CREATE TABLE t1 SELECT 1 + .123456789123456789123456789123456789123456789123456789123456789123456789123456789 AS my_col;
+DESCRIBE t1;
+SELECT my_col FROM t1;
+DROP TABLE t1;
+
+CREATE TABLE t1 SELECT 1 * .123456789123456789123456789123456789123456789123456789123456789123456789123456789 AS my_col;
+DESCRIBE t1;
+SELECT my_col FROM t1;
+DROP TABLE t1;
+
+CREATE TABLE t1 SELECT 1 / .123456789123456789123456789123456789123456789123456789123456789123456789123456789 AS my_col;
+DESCRIBE t1;
+SELECT my_col FROM t1;
+DROP TABLE t1;
+
+CREATE TABLE t1 SELECT 1 % .123456789123456789123456789123456789123456789123456789123456789123456789123456789 AS my_col;
+DESCRIBE t1;
+SELECT my_col FROM t1;
+DROP TABLE t1;
+
 --echo End of 5.0 tests

sql/item.cc

@@ -2215,8 +2215,10 @@ Item_decimal::Item_decimal(const char *str_arg, uint length,
   name= (char*) str_arg;
   decimals= (uint8) decimal_value.frac;
   fixed= 1;
-  max_length= my_decimal_precision_to_length(decimal_value.intg + decimals,
-                                             decimals, unsigned_flag);
+  max_length= my_decimal_precision_to_length_no_truncation(decimal_value.intg +
+                                                           decimals,
+                                                           decimals,
+                                                           unsigned_flag);
 }
 
 Item_decimal::Item_decimal(longlong val, bool unsig)
@@ -2224,8 +2226,10 @@ Item_decimal::Item_decimal(longlong val, bool unsig)
   int2my_decimal(E_DEC_FATAL_ERROR, val, unsig, &decimal_value);
   decimals= (uint8) decimal_value.frac;
   fixed= 1;
-  max_length= my_decimal_precision_to_length(decimal_value.intg + decimals,
-                                             decimals, unsigned_flag);
+  max_length= my_decimal_precision_to_length_no_truncation(decimal_value.intg +
+                                                           decimals,
+                                                           decimals,
+                                                           unsigned_flag);
 }
 
 
@@ -2234,8 +2238,10 @@ Item_decimal::Item_decimal(double val, int precision, int scale)
   double2my_decimal(E_DEC_FATAL_ERROR, val, &decimal_value);
   decimals= (uint8) decimal_value.frac;
   fixed= 1;
-  max_length= my_decimal_precision_to_length(decimal_value.intg + decimals,
-                                             decimals, unsigned_flag);
+  max_length= my_decimal_precision_to_length_no_truncation(decimal_value.intg +
+                                                           decimals,
+                                                           decimals,
+                                                           unsigned_flag);
 }
 
 
@@ -2255,8 +2261,10 @@ Item_decimal::Item_decimal(my_decimal *value_par)
   my_decimal2decimal(value_par, &decimal_value);
   decimals= (uint8) decimal_value.frac;
   fixed= 1;
-  max_length= my_decimal_precision_to_length(decimal_value.intg + decimals,
-                                             decimals, unsigned_flag);
+  max_length= my_decimal_precision_to_length_no_truncation(decimal_value.intg +
+                                                           decimals,
+                                                           decimals,
+                                                           unsigned_flag);
 }
 
 
@@ -2266,8 +2274,8 @@ Item_decimal::Item_decimal(const char *bin, int precision, int scale)
                     &decimal_value, precision, scale);
   decimals= (uint8) decimal_value.frac;
   fixed= 1;
-  max_length= my_decimal_precision_to_length(precision, decimals,
-                                             unsigned_flag);
+  max_length= my_decimal_precision_to_length_no_truncation(precision, decimals,
+                                                           unsigned_flag);
 }
 
 
@@ -2322,8 +2330,10 @@ void Item_decimal::set_decimal_value(my_decimal *value_par)
   my_decimal2decimal(value_par, &decimal_value);
   decimals= (uint8) decimal_value.frac;
   unsigned_flag= !decimal_value.sign();
-  max_length= my_decimal_precision_to_length(decimal_value.intg + decimals,
-                                             decimals, unsigned_flag);
+  max_length= my_decimal_precision_to_length_no_truncation(decimal_value.intg +
+                                                           decimals,
+                                                           decimals,
+                                                           unsigned_flag);
 }
 
 
@@ -2553,8 +2563,9 @@ void Item_param::set_decimal(const char *str, ulong length)
   str2my_decimal(E_DEC_FATAL_ERROR, str, &decimal_value, &end);
   state= DECIMAL_VALUE;
   decimals= decimal_value.frac;
-  max_length= my_decimal_precision_to_length(decimal_value.precision(),
-                                             decimals, unsigned_flag);
+  max_length=
+    my_decimal_precision_to_length_no_truncation(decimal_value.precision(),
+                                                 decimals, unsigned_flag);
   maybe_null= 0;
   DBUG_VOID_RETURN;
 }
@@ -2712,8 +2723,9 @@ bool Item_param::set_from_user_var(THD *thd, const user_var_entry *entry)
       my_decimal2decimal(ent_value, &decimal_value);
       state= DECIMAL_VALUE;
       decimals= ent_value->frac;
-      max_length= my_decimal_precision_to_length(ent_value->precision(),
-                                                 decimals, unsigned_flag);
+      max_length=
+        my_decimal_precision_to_length_no_truncation(ent_value->precision(),
+                                                     decimals, unsigned_flag);
       item_type= Item::DECIMAL_ITEM;
       break;
     }
@@ -6931,8 +6943,9 @@ bool Item_type_holder::join_types(THD *thd, Item *item)
     int item_prec = max(prev_decimal_int_part, item_int_part) + decimals;
     int precision= min(item_prec, DECIMAL_MAX_PRECISION);
     unsigned_flag&= item->unsigned_flag;
-    max_length= my_decimal_precision_to_length(precision, decimals,
-                                               unsigned_flag);
+    max_length= my_decimal_precision_to_length_no_truncation(precision,
+                                                             decimals,
+                                                             unsigned_flag);
   }
 
   switch (Field::result_merge_type(fld_type))

sql/item_cmpfunc.cc

@@ -2716,8 +2716,9 @@ void Item_func_case::fix_length_and_dec()
       agg_num_lengths(args[i + 1]);
     if (else_expr_num != -1) 
       agg_num_lengths(args[else_expr_num]);
-    max_length= my_decimal_precision_to_length(max_length + decimals, decimals,
-                                               unsigned_flag);
+    max_length= my_decimal_precision_to_length_no_truncation(max_length +
+                                                             decimals, decimals,
+                                                             unsigned_flag);
   }
 }
 

sql/item_func.cc

@@ -447,11 +447,46 @@ Field *Item_func::tmp_table_field(TABLE *t_arg)
     res= make_string_field(t_arg);
     break;
   case DECIMAL_RESULT:
-    res= new Field_new_decimal(my_decimal_precision_to_length(decimal_precision(),
-                                                              decimals,
-                                                              unsigned_flag),
-                               maybe_null, name, t_arg, decimals, unsigned_flag);
+  {
+    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;
+    }
+
+    res= new Field_new_decimal(len, maybe_null, name,
+                               t_arg, dec, unsigned_flag);
     break;
+  }
   case ROW_RESULT:
   default:
     // This case should never be chosen
@@ -540,8 +575,8 @@ void Item_func::count_decimal_length()
     set_if_smaller(unsigned_flag, args[i]->unsigned_flag);
   }
   int precision= min(max_int_part + decimals, DECIMAL_MAX_PRECISION);
-  max_length= my_decimal_precision_to_length(precision, decimals,
-                                             unsigned_flag);
+  max_length= my_decimal_precision_to_length_no_truncation(precision, decimals,
+                                                           unsigned_flag);
 }
 
 
@@ -1158,16 +1193,15 @@ void Item_func_additive_op::result_precision()
   decimals= max(args[0]->decimals, args[1]->decimals);
   int arg1_int= args[0]->decimal_precision() - args[0]->decimals;
   int arg2_int= args[1]->decimal_precision() - args[1]->decimals;
-  int est_prec= max(arg1_int, arg2_int) + 1 + decimals;
-  int precision= min(est_prec, DECIMAL_MAX_PRECISION);
+  int precision= max(arg1_int, arg2_int) + 1 + decimals;
 
   /* Integer operations keep unsigned_flag if one of arguments is unsigned */
   if (result_type() == INT_RESULT)
     unsigned_flag= args[0]->unsigned_flag | args[1]->unsigned_flag;
   else
     unsigned_flag= args[0]->unsigned_flag & args[1]->unsigned_flag;
-  max_length= my_decimal_precision_to_length(precision, decimals,
-                                             unsigned_flag);
+  max_length= my_decimal_precision_to_length_no_truncation(precision, decimals,
+                                                           unsigned_flag);
 }
 
 
@@ -1270,7 +1304,8 @@ void Item_func_mul::result_precision()
   decimals= min(args[0]->decimals + args[1]->decimals, DECIMAL_MAX_SCALE);
   uint est_prec = args[0]->decimal_precision() + args[1]->decimal_precision();
   uint precision= min(est_prec, DECIMAL_MAX_PRECISION);
-  max_length= my_decimal_precision_to_length(precision, decimals,unsigned_flag);
+  max_length= my_decimal_precision_to_length_no_truncation(precision, decimals,
+                                                           unsigned_flag);
 }
 
 
@@ -1326,8 +1361,8 @@ void Item_func_div::result_precision()
   else
     unsigned_flag= args[0]->unsigned_flag & args[1]->unsigned_flag;
   decimals= min(args[0]->decimals + prec_increment, DECIMAL_MAX_SCALE);
-  max_length= my_decimal_precision_to_length(precision, decimals,
-                                             unsigned_flag);
+  max_length= my_decimal_precision_to_length_no_truncation(precision, decimals,
+                                                           unsigned_flag);
 }
 
 
@@ -2013,8 +2048,9 @@ void Item_func_round::fix_length_and_dec()
 
     precision-= decimals_delta - length_increase;
     decimals= min(decimals_to_set, DECIMAL_MAX_SCALE);
-    max_length= my_decimal_precision_to_length(precision, decimals,
-                                               unsigned_flag);
+    max_length= my_decimal_precision_to_length_no_truncation(precision,
+                                                             decimals,
+                                                             unsigned_flag);
     break;
   }
   default:
@@ -2249,8 +2285,9 @@ void Item_func_min_max::fix_length_and_dec()
     }
   }
   else if ((cmp_type == DECIMAL_RESULT) || (cmp_type == INT_RESULT))
-    max_length= my_decimal_precision_to_length(max_int_part+decimals, decimals,
-                                            unsigned_flag);
+    max_length= my_decimal_precision_to_length_no_truncation(max_int_part +
+                                                             decimals, decimals,
+                                                             unsigned_flag);
   cached_field_type= agg_field_type(args, arg_count);
 }
 

sql/item_func.h

@@ -367,7 +367,8 @@ public:
   Item_decimal_typecast(Item *a, int len, int dec) :Item_func(a)
   {
     decimals= dec;
-    max_length= my_decimal_precision_to_length(len, dec, unsigned_flag);
+    max_length= my_decimal_precision_to_length_no_truncation(len, dec,
+                                                             unsigned_flag);
   }
   String *val_str(String *str);
   double val_real();

sql/item_sum.cc

@@ -786,8 +786,9 @@ void Item_sum_sum::fix_length_and_dec()
   {
     /* SUM result can't be longer than length(arg) + length(MAX_ROWS) */
     int precision= args[0]->decimal_precision() + DECIMAL_LONGLONG_DIGITS;
-    max_length= my_decimal_precision_to_length(precision, decimals,
-                                               unsigned_flag);
+    max_length= my_decimal_precision_to_length_no_truncation(precision,
+                                                             decimals,
+                                                             unsigned_flag);
     curr_dec_buff= 0;
     hybrid_type= DECIMAL_RESULT;
     my_decimal_set_zero(dec_buffs);
@@ -1217,8 +1218,9 @@ void Item_sum_avg::fix_length_and_dec()
   {
     int precision= args[0]->decimal_precision() + prec_increment;
     decimals= min(args[0]->decimals + prec_increment, DECIMAL_MAX_SCALE);
-    max_length= my_decimal_precision_to_length(precision, decimals,
-                                               unsigned_flag);
+    max_length= my_decimal_precision_to_length_no_truncation(precision,
+                                                             decimals,
+                                                             unsigned_flag);
     f_precision= min(precision+DECIMAL_LONGLONG_DIGITS, DECIMAL_MAX_PRECISION);
     f_scale=  args[0]->decimals;
     dec_bin_size= my_decimal_get_binary_size(f_precision, f_scale);
@@ -1418,8 +1420,9 @@ void Item_sum_variance::fix_length_and_dec()
   {
     int precision= args[0]->decimal_precision()*2 + prec_increment;
     decimals= min(args[0]->decimals + prec_increment, DECIMAL_MAX_SCALE);
-    max_length= my_decimal_precision_to_length(precision, decimals,
-                                               unsigned_flag);
+    max_length= my_decimal_precision_to_length_no_truncation(precision,
+                                                             decimals,
+                                                             unsigned_flag);
 
     break;
   }

sql/my_decimal.h

@@ -170,6 +170,19 @@ inline uint my_decimal_length_to_precision(uint length, uint scale,
                  (unsigned_flag || !length ? 0:1));
 }
 
+inline uint32 my_decimal_precision_to_length_no_truncation(uint precision,
+                                                           uint8 scale,
+                                                           bool unsigned_flag)
+{
+  /*
+    When precision is 0 it means that original length was also 0. Thus
+    unsigned_flag is ignored in this case.
+  */
+  DBUG_ASSERT(precision || !scale);
+  return (uint32)(precision + (scale > 0 ? 1 : 0) +
+                  (unsigned_flag || !precision ? 0 : 1));
+}
+
 inline uint32 my_decimal_precision_to_length(uint precision, uint8 scale,
                                              bool unsigned_flag)
 {
@@ -179,8 +192,8 @@ inline uint32 my_decimal_precision_to_length(uint precision, uint8 scale,
   */
   DBUG_ASSERT(precision || !scale);
   set_if_smaller(precision, DECIMAL_MAX_PRECISION);
-  return (uint32)(precision + (scale>0 ? 1:0) +
-                  (unsigned_flag || !precision ? 0:1));
+  return my_decimal_precision_to_length_no_truncation(precision, scale,
+                                                      unsigned_flag);
 }
 
 inline

sql/sql_select.cc

@@ -9087,13 +9087,17 @@ static Field *create_tmp_field_from_item(THD *thd, Item *item, TABLE *table,
         +1: for decimal point
       */
 
-      overflow= my_decimal_precision_to_length(intg + dec, dec,
-                                               item->unsigned_flag) - len;
+      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
-        len -= item->decimals - dec;            // corrected value fits
+        /* Corrected value fits. */
+        len= required_length;
     }
 
     new_field= new Field_new_decimal(len, maybe_null, item->name,