List:Commits« Previous MessageNext Message »
From:Alexey Kopytov Date:July 2 2009 6:46am
Subject:bzr commit into mysql-5.0-bugteam branch (Alexey.Kopytov:2784)
Bug#45262
View as plain text  
#At file:///data/src/bzr/bugteam/bug45262/my50-bug45262/ based on revid:joro@stripped

 2784 Alexey Kopytov	2009-07-02
      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/field.cc
        Use the new 'truncate' parameter in 
        my_decimal_precision_to_length().
     @ 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.
     @ sql/table.cc
        Use the new 'truncate' parameter in 
        my_decimal_precision_to_length().

    modified:
      mysql-test/r/type_newdecimal.result
      mysql-test/t/type_newdecimal.test
      sql/field.cc
      sql/item.cc
      sql/item_cmpfunc.cc
      sql/item_func.cc
      sql/item_func.h
      sql/item_sum.cc
      sql/my_decimal.h
      sql/sql_select.cc
      sql/table.cc
=== modified file 'mysql-test/r/type_newdecimal.result'
--- a/mysql-test/r/type_newdecimal.result	2008-11-17 15:41:09 +0000
+++ b/mysql-test/r/type_newdecimal.result	2009-07-02 06:46:02 +0000
@@ -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

=== modified file 'mysql-test/t/type_newdecimal.test'
--- a/mysql-test/t/type_newdecimal.test	2008-11-17 15:41:09 +0000
+++ b/mysql-test/t/type_newdecimal.test	2009-07-02 06:46:02 +0000
@@ -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

=== modified file 'sql/field.cc'
--- a/sql/field.cc	2009-06-09 16:11:21 +0000
+++ b/sql/field.cc	2009-07-02 06:46:02 +0000
@@ -8519,7 +8519,7 @@ bool create_field::init(THD *thd, char *
     }
     length=
       my_decimal_precision_to_length(length, decimals,
-                                     fld_type_modifier & UNSIGNED_FLAG);
+                                     fld_type_modifier & UNSIGNED_FLAG, TRUE);
     pack_length=
       my_decimal_get_binary_size(length, decimals);
     break;

=== modified file 'sql/item.cc'
--- a/sql/item.cc	2009-06-17 13:54:01 +0000
+++ b/sql/item.cc	2009-07-02 06:46:02 +0000
@@ -2216,7 +2216,7 @@ Item_decimal::Item_decimal(const char *s
   decimals= (uint8) decimal_value.frac;
   fixed= 1;
   max_length= my_decimal_precision_to_length(decimal_value.intg + decimals,
-                                             decimals, unsigned_flag);
+                                             decimals, unsigned_flag, FALSE);
 }
 
 Item_decimal::Item_decimal(longlong val, bool unsig)
@@ -2225,7 +2225,7 @@ Item_decimal::Item_decimal(longlong val,
   decimals= (uint8) decimal_value.frac;
   fixed= 1;
   max_length= my_decimal_precision_to_length(decimal_value.intg + decimals,
-                                             decimals, unsigned_flag);
+                                             decimals, unsigned_flag, FALSE);
 }
 
 
@@ -2235,7 +2235,7 @@ Item_decimal::Item_decimal(double val, i
   decimals= (uint8) decimal_value.frac;
   fixed= 1;
   max_length= my_decimal_precision_to_length(decimal_value.intg + decimals,
-                                             decimals, unsigned_flag);
+                                             decimals, unsigned_flag, FALSE);
 }
 
 
@@ -2256,7 +2256,7 @@ Item_decimal::Item_decimal(my_decimal *v
   decimals= (uint8) decimal_value.frac;
   fixed= 1;
   max_length= my_decimal_precision_to_length(decimal_value.intg + decimals,
-                                             decimals, unsigned_flag);
+                                             decimals, unsigned_flag, FALSE);
 }
 
 
@@ -2267,7 +2267,7 @@ Item_decimal::Item_decimal(const char *b
   decimals= (uint8) decimal_value.frac;
   fixed= 1;
   max_length= my_decimal_precision_to_length(precision, decimals,
-                                             unsigned_flag);
+                                             unsigned_flag, FALSE);
 }
 
 
@@ -2323,7 +2323,7 @@ void Item_decimal::set_decimal_value(my_
   decimals= (uint8) decimal_value.frac;
   unsigned_flag= !decimal_value.sign();
   max_length= my_decimal_precision_to_length(decimal_value.intg + decimals,
-                                             decimals, unsigned_flag);
+                                             decimals, unsigned_flag, FALSE);
 }
 
 
@@ -2554,7 +2554,7 @@ void Item_param::set_decimal(const char 
   state= DECIMAL_VALUE;
   decimals= decimal_value.frac;
   max_length= my_decimal_precision_to_length(decimal_value.precision(),
-                                             decimals, unsigned_flag);
+                                             decimals, unsigned_flag, FALSE);
   maybe_null= 0;
   DBUG_VOID_RETURN;
 }
@@ -2713,7 +2713,8 @@ bool Item_param::set_from_user_var(THD *
       state= DECIMAL_VALUE;
       decimals= ent_value->frac;
       max_length= my_decimal_precision_to_length(ent_value->precision(),
-                                                 decimals, unsigned_flag);
+                                                 decimals, unsigned_flag,
+                                                 FALSE);
       item_type= Item::DECIMAL_ITEM;
       break;
     }
@@ -6932,7 +6933,7 @@ bool Item_type_holder::join_types(THD *t
     int precision= min(item_prec, DECIMAL_MAX_PRECISION);
     unsigned_flag&= item->unsigned_flag;
     max_length= my_decimal_precision_to_length(precision, decimals,
-                                               unsigned_flag);
+                                               unsigned_flag, FALSE);
   }
 
   switch (Field::result_merge_type(fld_type))

=== modified file 'sql/item_cmpfunc.cc'
--- a/sql/item_cmpfunc.cc	2009-06-09 16:11:21 +0000
+++ b/sql/item_cmpfunc.cc	2009-07-02 06:46:02 +0000
@@ -2717,7 +2717,7 @@ void Item_func_case::fix_length_and_dec(
     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);
+                                               unsigned_flag, FALSE);
   }
 }
 

=== modified file 'sql/item_func.cc'
--- a/sql/item_func.cc	2009-06-09 16:11:21 +0000
+++ b/sql/item_func.cc	2009-07-02 06:46:02 +0000
@@ -447,11 +447,42 @@ Field *Item_func::tmp_table_field(TABLE 
     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.
+      */
+
+      overflow= my_decimal_precision_to_length(intg + dec, dec,
+                                               unsigned_flag, TRUE) - len;
+
+      if (overflow > 0)
+        dec= max(0, dec - overflow);            // too long, discard fract
+      else
+        len += overflow;            // corrected value fits
+    }
+
+    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
@@ -541,7 +572,7 @@ void Item_func::count_decimal_length()
   }
   int precision= min(max_int_part + decimals, DECIMAL_MAX_PRECISION);
   max_length= my_decimal_precision_to_length(precision, decimals,
-                                             unsigned_flag);
+                                             unsigned_flag, FALSE);
 }
 
 
@@ -1158,8 +1189,7 @@ void Item_func_additive_op::result_preci
   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)
@@ -1167,7 +1197,7 @@ void Item_func_additive_op::result_preci
   else
     unsigned_flag= args[0]->unsigned_flag & args[1]->unsigned_flag;
   max_length= my_decimal_precision_to_length(precision, decimals,
-                                             unsigned_flag);
+                                             unsigned_flag, FALSE);
 }
 
 
@@ -1270,7 +1300,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(precision, decimals,unsigned_flag,
+                                             FALSE);
 }
 
 
@@ -1327,7 +1358,7 @@ void Item_func_div::result_precision()
     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);
+                                             unsigned_flag, FALSE);
 }
 
 
@@ -2014,7 +2045,7 @@ 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);
+                                               unsigned_flag, FALSE);
     break;
   }
   default:
@@ -2250,7 +2281,7 @@ void Item_func_min_max::fix_length_and_d
   }
   else if ((cmp_type == DECIMAL_RESULT) || (cmp_type == INT_RESULT))
     max_length= my_decimal_precision_to_length(max_int_part+decimals, decimals,
-                                            unsigned_flag);
+                                               unsigned_flag, FALSE);
   cached_field_type= agg_field_type(args, arg_count);
 }
 

=== modified file 'sql/item_func.h'
--- a/sql/item_func.h	2009-02-24 14:47:12 +0000
+++ b/sql/item_func.h	2009-07-02 06:46:02 +0000
@@ -367,7 +367,7 @@ 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(len, dec, unsigned_flag, FALSE);
   }
   String *val_str(String *str);
   double val_real();

=== modified file 'sql/item_sum.cc'
--- a/sql/item_sum.cc	2009-06-15 15:29:26 +0000
+++ b/sql/item_sum.cc	2009-07-02 06:46:02 +0000
@@ -787,7 +787,7 @@ 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);
+                                               unsigned_flag, FALSE);
     curr_dec_buff= 0;
     hybrid_type= DECIMAL_RESULT;
     my_decimal_set_zero(dec_buffs);
@@ -1218,7 +1218,7 @@ 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);
+                                               unsigned_flag, FALSE);
     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);
@@ -1419,7 +1419,7 @@ void Item_sum_variance::fix_length_and_d
     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);
+                                               unsigned_flag, FALSE);
 
     break;
   }

=== modified file 'sql/my_decimal.h'
--- a/sql/my_decimal.h	2008-04-24 20:39:37 +0000
+++ b/sql/my_decimal.h	2009-07-02 06:46:02 +0000
@@ -171,14 +171,20 @@ inline uint my_decimal_length_to_precisi
 }
 
 inline uint32 my_decimal_precision_to_length(uint precision, uint8 scale,
-                                             bool unsigned_flag)
+                                             bool unsigned_flag, bool truncate)
 {
   /*
     When precision is 0 it means that original length was also 0. Thus
     unsigned_flag is ignored in this case.
   */
   DBUG_ASSERT(precision || !scale);
-  set_if_smaller(precision, DECIMAL_MAX_PRECISION);
+  /*
+    Fields' precision is limited by DECIMAL_MAX_PRECISION, whereas DECIMAL
+    constants and (some) arithmetic expressions on DECIMAL may be arbitrarily
+    long.
+  */
+  if (truncate)
+    set_if_smaller(precision, DECIMAL_MAX_PRECISION);
   return (uint32)(precision + (scale>0 ? 1:0) +
                   (unsigned_flag || !precision ? 0:1));
 }

=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc	2009-06-17 13:54:01 +0000
+++ b/sql/sql_select.cc	2009-07-02 06:46:02 +0000
@@ -9088,12 +9088,12 @@ static Field *create_tmp_field_from_item
       */
 
       overflow= my_decimal_precision_to_length(intg + dec, dec,
-                                               item->unsigned_flag) - len;
+                                               item->unsigned_flag, TRUE) - len;
 
       if (overflow > 0)
         dec= max(0, dec - overflow);            // too long, discard fract
       else
-        len -= item->decimals - dec;            // corrected value fits
+        len += overflow;            // corrected value fits
     }
 
     new_field= new Field_new_decimal(len, maybe_null, item->name,

=== modified file 'sql/table.cc'
--- a/sql/table.cc	2009-06-17 13:54:01 +0000
+++ b/sql/table.cc	2009-07-02 06:46:02 +0000
@@ -614,7 +614,7 @@ int openfrm(THD *thd, const char *name, 
       uint decimals= f_decimals(pack_flag);
       field_length= my_decimal_precision_to_length(field_length,
                                                    decimals,
-                                                   f_is_dec(pack_flag) == 0);
+                                                   f_is_dec(pack_flag) == 0, TRUE);
       sql_print_error("Found incompatible DECIMAL field '%s' in %s; Please do \"ALTER TABLE '%s' FORCE\" to fix it!", share->fieldnames.type_names[i], name, share->table_name);
       push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_ERROR,
                           ER_CRASHED_ON_USAGE,


Attachment: [text/bzr-bundle] bzr/alexey.kopytov@sun.com-20090702064602-z8t5wg6wn1mhmkik.bundle
Thread
bzr commit into mysql-5.0-bugteam branch (Alexey.Kopytov:2784)Bug#45262Alexey Kopytov2 Jul