From aa3233619874c3bfee4d43957148ef2c7904aa6c Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 22 Mar 2007 10:56:47 +0100 Subject: [PATCH 1/3] Bug #24791: Union with AVG-groups generates wrong results The problem in this bug is when we create temporary tables. When temporary tables are created for unions, there is some inferrence being carried out regarding the type of the column. Whenever this column type is inferred to be REAL (i.e. FLOAT or DOUBLE), MySQL will always try to maintain exact precision, and if that is not possible (there are hardware limits, since FLOAT and DOUBLE are stored as approximate values) will switch to using approximate values. The problem here is that at this point the information about number of significant digits is not available. Furthermore, the number of significant digits should be increased for the AVG function, however, this was not properly handled. There are 4 parts to the problem: #1: DOUBLE and FLOAT fields don't display their proper display lengths in max_display_length(). This is hard-coded as 53 for DOUBLE and 24 for FLOAT. Now changed to instead return the field_length. #2: Type holders for temporary tables do not preserve the max_length of the Item's from which they are created, and is instead reverted to the 53 and 24 from above. This causes *all* fields to get non-fixed significant digits. #3: AVG function does not update max_length (display length) when updating number of decimals. #4: The function that switches to non-fixed number of significant digits should use DBL_DIG + 2 or FLT_DIG + 2 as cut-off values (Since fixed precision does not use the 'e' notation) Of these points, #1 is the controversial one, but this change is preferred and has been cleared with Monty. The function causes quite a few unit tests to blow up and they had to b changed, but each one is annotated and motivated. We frequently see the magical 53 and 24 give way to more relevant numbers. mysql-test/r/create.result: bug#24791 changed test result With the changes made for FLOAT and DOUBLE, the original display lengths are now preserved. mysql-test/r/temp_table.result: bug#24791 changed test resullt Test case added mysql-test/r/type_float.result: bug#24791 changed test result delta 1: field was originally declared as DOUBLE with no display length, so the hardware maximum is chosen rather than 53. delta 2: fields exceed the maximum precision and thus switch to non-fixed significant digits delta 3: Same as above, number of decmals and significant digits was not specified when t3 was created. mysql-test/t/temp_table.test: bug#24791 Test case sql/field.h: bug#24791 The method max_display_length is reimplemented as uint32 max_display_length() { return field_length; } in Field_double and Field_float. Since all subclasses of Field_real now have the same implementation of this method, the implementation has been moved up the hierarchy to Field_real. sql/item.cc: bug#24791 We switch to a non-fixed number of significant digits (by setting decimals=NOT_FIXED_DECIMAL) if the calculated display length is greater than the display length of a value with the maximum precision. These values differ for double and float, obviously. sql/item_sum.cc: bug#24791 We must increase the display length accordinly whenever we change number of decimal places. --- mysql-test/r/create.result | 4 ++-- mysql-test/r/temp_table.result | 21 +++++++++++++++++++++ mysql-test/r/type_float.result | 8 ++++---- mysql-test/t/temp_table.test | 16 ++++++++++++++++ sql/field.h | 4 +--- sql/item.cc | 18 +++++++++++------- sql/item_sum.cc | 4 +++- 7 files changed, 58 insertions(+), 17 deletions(-) diff --git a/mysql-test/r/create.result b/mysql-test/r/create.result index 11c1431de7b..0a8fef8d881 100644 --- a/mysql-test/r/create.result +++ b/mysql-test/r/create.result @@ -447,8 +447,8 @@ t2 CREATE TABLE `t2` ( `ifnull(c,c)` mediumint(8) default NULL, `ifnull(d,d)` int(11) default NULL, `ifnull(e,e)` bigint(20) default NULL, - `ifnull(f,f)` float(24,2) default NULL, - `ifnull(g,g)` double(53,3) default NULL, + `ifnull(f,f)` float(3,2) default NULL, + `ifnull(g,g)` double(4,3) default NULL, `ifnull(h,h)` decimal(5,4) default NULL, `ifnull(i,i)` year(4) default NULL, `ifnull(j,j)` date default NULL, diff --git a/mysql-test/r/temp_table.result b/mysql-test/r/temp_table.result index 139a7da77de..d6adf51602b 100644 --- a/mysql-test/r/temp_table.result +++ b/mysql-test/r/temp_table.result @@ -152,3 +152,24 @@ SELECT * FROM t1; i DROP TABLE t1; End of 4.1 tests. +CREATE TABLE t1 ( c FLOAT( 20, 14 ) ); +INSERT INTO t1 VALUES( 12139 ); +CREATE TABLE t2 ( c FLOAT(30,18) ); +INSERT INTO t2 VALUES( 123456 ); +SELECT AVG( c ) FROM t1 UNION SELECT 1; +AVG( c ) +12139 +1 +SELECT 1 UNION SELECT AVG( c ) FROM t1; +1 +1 +12139 +SELECT 1 UNION SELECT * FROM t2 UNION SELECT 1; +1 +1 +123456 +SELECT c/1 FROM t1 UNION SELECT 1; +c/1 +12139 +1 +DROP TABLE t1, t2; diff --git a/mysql-test/r/type_float.result b/mysql-test/r/type_float.result index 0cb77f42caf..188963c5bdf 100644 --- a/mysql-test/r/type_float.result +++ b/mysql-test/r/type_float.result @@ -92,7 +92,7 @@ show create table t2; Table Create Table t2 CREATE TABLE `t2` ( `col1` double default NULL, - `col2` double(53,5) default NULL, + `col2` double(22,5) default NULL, `col3` double default NULL, `col4` double default NULL ) ENGINE=MyISAM DEFAULT CHARSET=latin1 @@ -232,12 +232,12 @@ insert into t2 values ("1.23456780"); create table t3 select * from t2 union select * from t1; select * from t3; d -1.234567800 -100000000.000000000 +1.2345678 +100000000 show create table t3; Table Create Table t3 CREATE TABLE `t3` ( - `d` double(22,9) default NULL + `d` double default NULL ) ENGINE=MyISAM DEFAULT CHARSET=latin1 drop table t1, t2, t3; create table t1 select 105213674794682365.00 + 0.0 x; diff --git a/mysql-test/t/temp_table.test b/mysql-test/t/temp_table.test index 8cb9e34ca08..90f868f5932 100644 --- a/mysql-test/t/temp_table.test +++ b/mysql-test/t/temp_table.test @@ -163,3 +163,19 @@ DROP TABLE t1; --echo End of 4.1 tests. + +# +# Bug #24791: Union with AVG-groups generates wrong results +# +CREATE TABLE t1 ( c FLOAT( 20, 14 ) ); +INSERT INTO t1 VALUES( 12139 ); + +CREATE TABLE t2 ( c FLOAT(30,18) ); +INSERT INTO t2 VALUES( 123456 ); + +SELECT AVG( c ) FROM t1 UNION SELECT 1; +SELECT 1 UNION SELECT AVG( c ) FROM t1; +SELECT 1 UNION SELECT * FROM t2 UNION SELECT 1; +SELECT c/1 FROM t1 UNION SELECT 1; + +DROP TABLE t1, t2; diff --git a/sql/field.h b/sql/field.h index 524380800f3..a00419893e1 100644 --- a/sql/field.h +++ b/sql/field.h @@ -432,6 +432,7 @@ public: int store_decimal(const my_decimal *); my_decimal *val_decimal(my_decimal *); + uint32 max_display_length() { return field_length; } }; @@ -461,7 +462,6 @@ public: void overflow(bool negative); bool zero_pack() const { return 0; } void sql_type(String &str) const; - uint32 max_display_length() { return field_length; } }; @@ -719,7 +719,6 @@ public: void sort_string(char *buff,uint length); uint32 pack_length() const { return sizeof(float); } void sql_type(String &str) const; - uint32 max_display_length() { return 24; } }; @@ -762,7 +761,6 @@ public: void sort_string(char *buff,uint length); uint32 pack_length() const { return sizeof(double); } void sql_type(String &str) const; - uint32 max_display_length() { return 53; } uint size_of() const { return sizeof(*this); } }; diff --git a/sql/item.cc b/sql/item.cc index 11a5039ca19..958de308cc4 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -6348,8 +6348,6 @@ Item_type_holder::Item_type_holder(THD *thd, Item *item) :Item(thd, item), enum_set_typelib(0), fld_type(get_real_type(item)) { DBUG_ASSERT(item->fixed); - - max_length= display_length(item); maybe_null= item->maybe_null; collation.set(item->collation); get_full_info(item); @@ -6521,11 +6519,17 @@ bool Item_type_holder::join_types(THD *thd, Item *item) { int delta1= max_length_orig - decimals_orig; int delta2= item->max_length - item->decimals; - if (fld_type == MYSQL_TYPE_DECIMAL) - max_length= max(delta1, delta2) + decimals; - else - max_length= min(max(delta1, delta2) + decimals, - (fld_type == MYSQL_TYPE_FLOAT) ? FLT_DIG+6 : DBL_DIG+7); + max_length= max(delta1, delta2) + decimals; + if (fld_type == MYSQL_TYPE_FLOAT && max_length > FLT_DIG + 2) + { + max_length= FLT_DIG + 6; + decimals= NOT_FIXED_DEC; + } + if (fld_type == MYSQL_TYPE_DOUBLE && max_length > DBL_DIG + 2) + { + max_length= DBL_DIG + 7; + decimals= NOT_FIXED_DEC; + } } else max_length= (fld_type == MYSQL_TYPE_FLOAT) ? FLT_DIG+6 : DBL_DIG+7; diff --git a/sql/item_sum.cc b/sql/item_sum.cc index 41f0dd6496b..acc2d22c57c 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -1096,8 +1096,10 @@ void Item_sum_avg::fix_length_and_dec() f_scale= args[0]->decimals; dec_bin_size= my_decimal_get_binary_size(f_precision, f_scale); } - else + else { decimals= min(args[0]->decimals + prec_increment, NOT_FIXED_DEC); + max_length= args[0]->max_length + prec_increment; + } } From 33da0f3736fe3b4bc2fed8ea585d5adf74d6b02b Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 22 Mar 2007 14:58:43 +0100 Subject: [PATCH 2/3] Bug#24791: Union with AVG-groups generates wrong results Patch appled after doing a pull from the team tree. Additional tests had to be fixed mysql-test/r/union.result: Bug 24791 The tests for temporary tables have been fixed. Since the call to display_length(Item) was removed from the constructor for Item_type_holder, items in temporary tables keep the original values of the items, rather than the magic numbers supplied by display_length. --- mysql-test/r/union.result | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mysql-test/r/union.result b/mysql-test/r/union.result index 8219d68a681..efdd8195fb5 100644 --- a/mysql-test/r/union.result +++ b/mysql-test/r/union.result @@ -554,7 +554,7 @@ aa show create table t1; Table Create Table t1 CREATE TABLE `t1` ( - `a` varbinary(20) NOT NULL default '' + `a` varbinary(2) NOT NULL default '' ) ENGINE=MyISAM DEFAULT CHARSET=latin1 drop table t1; create table t1 SELECT 12 as a UNION select 12.2 as a; @@ -655,7 +655,7 @@ f show create table t1; Table Create Table t1 CREATE TABLE `t1` ( - `f` varbinary(24) default NULL + `f` varbinary(12) default NULL ) ENGINE=MyISAM DEFAULT CHARSET=latin1 drop table t1; create table t1 SELECT y from t2 UNION select da from t2; From 0a48cd93b4d26056e669b1a9bd5be791b3b79b94 Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 22 Mar 2007 18:44:16 +0200 Subject: [PATCH 3/3] Bug #26207: When making the key image to use in index search MySQL was not explicitly suppressing warnings. And if the context happens to enable warnings (e.g. INSERT .. SELECT) the warnings resulting from converting the data the key is compared to are reported to the client. Fixed by suppressing warnings when converting the data to the same type as the key parts. mysql-test/r/insert_select.result: Bug #26207: test case mysql-test/t/insert_select.test: Bug #26207: test case sql/sql_select.h: Bug #26207: supress warnings when converting data of the same type to key buffer format. --- mysql-test/r/insert_select.result | 29 +++++++++++++++++ mysql-test/t/insert_select.test | 27 +++++++++++++++- sql/sql_select.h | 53 ++++++++++++++++++++++++------- 3 files changed, 96 insertions(+), 13 deletions(-) diff --git a/mysql-test/r/insert_select.result b/mysql-test/r/insert_select.result index 92b3ea0e42b..a96add7eb9a 100644 --- a/mysql-test/r/insert_select.result +++ b/mysql-test/r/insert_select.result @@ -744,3 +744,32 @@ f1 f2 2 2 10 10 DROP TABLE t1, t2; +SET SQL_MODE='STRICT_TRANS_TABLES,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION'; +CREATE TABLE t1 (c VARCHAR(30), INDEX ix_c (c(10))); +CREATE TABLE t2 (d VARCHAR(10)); +INSERT INTO t1 (c) VALUES ('7_chars'), ('13_characters'); +EXPLAIN +SELECT (SELECT SUM(LENGTH(c)) FROM t1 WHERE c='13_characters') FROM t1; +id select_type table type possible_keys key key_len ref rows Extra +1 PRIMARY t1 ALL NULL NULL NULL NULL 2 +2 SUBQUERY t1 ref ix_c ix_c 13 const 1 Using where +SELECT (SELECT SUM(LENGTH(c)) FROM t1 WHERE c='13_characters') FROM t1; +(SELECT SUM(LENGTH(c)) FROM t1 WHERE c='13_characters') +13 +13 +INSERT INTO t2 (d) +SELECT (SELECT SUM(LENGTH(c)) FROM t1 WHERE c='13_characters') FROM t1; +INSERT INTO t2 (d) +SELECT (SELECT SUM(LENGTH(c)) FROM t1 WHERE c='7_chars') FROM t1; +INSERT INTO t2 (d) +SELECT (SELECT SUM(LENGTH(c)) FROM t1 WHERE c IN (SELECT t1.c FROM t1)) +FROM t1; +SELECT * FROM t2; +d +13 +13 +7 +7 +20 +20 +DROP TABLE t1,t2; diff --git a/mysql-test/t/insert_select.test b/mysql-test/t/insert_select.test index 31508b3d6c4..bbc51be6dc9 100644 --- a/mysql-test/t/insert_select.test +++ b/mysql-test/t/insert_select.test @@ -306,4 +306,29 @@ INSERT INTO t2 (f1, f2) SELECT * FROM t2; DROP TABLE t1, t2; - +# +# Bug #26207: inserts don't work with shortened index +# +SET SQL_MODE='STRICT_TRANS_TABLES,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION'; + +CREATE TABLE t1 (c VARCHAR(30), INDEX ix_c (c(10))); +CREATE TABLE t2 (d VARCHAR(10)); +INSERT INTO t1 (c) VALUES ('7_chars'), ('13_characters'); + +EXPLAIN + SELECT (SELECT SUM(LENGTH(c)) FROM t1 WHERE c='13_characters') FROM t1; + +SELECT (SELECT SUM(LENGTH(c)) FROM t1 WHERE c='13_characters') FROM t1; + +INSERT INTO t2 (d) + SELECT (SELECT SUM(LENGTH(c)) FROM t1 WHERE c='13_characters') FROM t1; + +INSERT INTO t2 (d) + SELECT (SELECT SUM(LENGTH(c)) FROM t1 WHERE c='7_chars') FROM t1; + +INSERT INTO t2 (d) + SELECT (SELECT SUM(LENGTH(c)) FROM t1 WHERE c IN (SELECT t1.c FROM t1)) + FROM t1; + +SELECT * FROM t2; +DROP TABLE t1,t2; diff --git a/sql/sql_select.h b/sql/sql_select.h index a17d7fcb362..e70e42da4ae 100644 --- a/sql/sql_select.h +++ b/sql/sql_select.h @@ -488,15 +488,11 @@ extern "C" int refpos_order_cmp(void* arg, const void *a,const void *b); class store_key :public Sql_alloc { - protected: - Field *to_field; // Store data here - char *null_ptr; - char err; public: bool null_key; /* TRUE <=> the value of the key has a null part */ enum store_key_result { STORE_KEY_OK, STORE_KEY_FATAL, STORE_KEY_CONV }; store_key(THD *thd, Field *field_arg, char *ptr, char *null, uint length) - :null_ptr(null), err(0), null_key(0) + :null_key(0), null_ptr(null), err(0) { if (field_arg->type() == FIELD_TYPE_BLOB) { @@ -510,8 +506,35 @@ public: ptr, (uchar*) null, 1); } virtual ~store_key() {} /* Not actually needed */ - virtual enum store_key_result copy()=0; virtual const char *name() const=0; + + /** + @brief sets ignore truncation warnings mode and calls the real copy method + + @details this function makes sure truncation warnings when preparing the + key buffers don't end up as errors (because of an enclosing INSERT/UPDATE). + */ + enum store_key_result copy() + { + enum store_key_result result; + enum_check_fields saved_count_cuted_fields= + to_field->table->in_use->count_cuted_fields; + + to_field->table->in_use->count_cuted_fields= CHECK_FIELD_IGNORE; + + result= copy_inner(); + + to_field->table->in_use->count_cuted_fields= saved_count_cuted_fields; + + return result; + } + + protected: + Field *to_field; // Store data here + char *null_ptr; + char err; + + virtual enum store_key_result copy_inner()=0; }; @@ -531,13 +554,15 @@ class store_key_field: public store_key copy_field.set(to_field,from_field,0); } } - enum store_key_result copy() + const char *name() const { return field_name; } + + protected: + enum store_key_result copy_inner() { copy_field.do_copy(©_field); null_key= to_field->is_null(); return err != 0 ? STORE_KEY_FATAL : STORE_KEY_OK; } - const char *name() const { return field_name; } }; @@ -552,13 +577,15 @@ public: null_ptr_arg ? null_ptr_arg : item_arg->maybe_null ? &err : NullS, length), item(item_arg) {} - enum store_key_result copy() + const char *name() const { return "func"; } + + protected: + enum store_key_result copy_inner() { int res= item->save_in_field(to_field, 1); null_key= to_field->is_null() || item->null_value; return (err != 0 || res > 2 ? STORE_KEY_FATAL : (store_key_result) res); } - const char *name() const { return "func"; } }; @@ -574,7 +601,10 @@ public: &err : NullS, length, item_arg), inited(0) { } - enum store_key_result copy() + const char *name() const { return "const"; } + +protected: + enum store_key_result copy_inner() { int res; if (!inited) @@ -589,7 +619,6 @@ public: null_key= to_field->is_null() || item->null_value; return (err > 2 ? STORE_KEY_FATAL : (store_key_result) err); } - const char *name() const { return "const"; } }; bool cp_buffer_from_ref(THD *thd, TABLE_REF *ref);