From faad73550c157388ffb7b1606e3f896ed7b276ac Mon Sep 17 00:00:00 2001
From: unknown <ramil/ram@mysql.com/ramil.myoffice.izhnet.ru>
Date: Wed, 31 Jan 2007 09:51:05 +0400
Subject: [PATCH] fix for bug #19690: ORDER BY eliminates rows from the result

Depending on the queries we use different data processing methods
and can lose some data in case of double (and decimal in 4.1) fields.

The fix consists of two parts:
1. double comparison changed, now double a is equal to double b
if (a-b) is less than 5*0.1^(1 + max(a->decimals, b->decimals)).
For example, if a->decimals==1, b->decimals==2, a==b if (a-b)<0.005
2. if we use a temporary table, store double values there as is
to avoid any data conversion (rounding).


mysql-test/r/type_float.result:
  fix for bug #19690: ORDER BY eliminates rows from the result
    - test result
mysql-test/t/type_float.test:
  fix for bug #19690: ORDER BY eliminates rows from the result
    - test case
sql/field.cc:
  fix for bug #19690: ORDER BY eliminates rows from the result
    - use not_fixed flag instead of dec to check bounds.
sql/field.h:
  fix for bug #19690: ORDER BY eliminates rows from the result
    - Field_Double::not_fixed flag introduced, which is set if dec == NOT_FIXED_DEC
      and is used in the ::store() to check bounds.
    - new constructor introduced (with not_fixed_arg parameter).
sql/init.cc:
  fix for bug #19690: ORDER BY eliminates rows from the result
    - fill log_01[] array with 0.1 powers.
sql/item_cmpfunc.cc:
  fix for bug #19690: ORDER BY eliminates rows from the result
    - compare_real_fixed() and compare_e_real_fixed() introduced,
      they consider double a == double b if a-b is less than 'precision',
      'precision' is set to 5*0.1^(1 + max(a->decimals, b->decimals)),
      for example, if a->decimals==1, b->decimals==2, 'precision' is 0.005
    - use the above functions if both arguments are fixed.
sql/item_cmpfunc.h:
  fix for bug #19690: ORDER BY eliminates rows from the result
    - Arg_comparator::presision introduced.
    - Arg_comparator::compare_real_fixed(), Arg_comparator::compare_e_real_fixed() introduced.
sql/mysql_priv.h:
  fix for bug #19690: ORDER BY eliminates rows from the result
    - log_01 array of 0.1 powers added.
sql/mysqld.cc:
  fix for bug #19690: ORDER BY eliminates rows from the result
    - log_01 array of 0.1 powers added.
sql/sql_select.cc:
  fix for bug #19690: ORDER BY eliminates rows from the result
    - if we create double field in a temporary table, set not_fixed flag
      (use proper constructor) to avoid data conversion
      in the Field_double::store(). Otherwise we can lose some data.
---
 mysql-test/r/type_float.result | 71 ++++++++++++++++++++++++++++++++++
 mysql-test/t/type_float.test   | 25 ++++++++++++
 sql/field.cc                   |  2 +-
 sql/field.h                    | 15 +++++--
 sql/init.cc                    |  6 +++
 sql/item_cmpfunc.cc            | 49 +++++++++++++++++++++++
 sql/item_cmpfunc.h             |  3 ++
 sql/mysql_priv.h               |  1 +
 sql/mysqld.cc                  |  1 +
 sql/sql_select.cc              | 14 ++++---
 10 files changed, 177 insertions(+), 10 deletions(-)

diff --git a/mysql-test/r/type_float.result b/mysql-test/r/type_float.result
index f157bbc602d..3e66fa70dc1 100644
--- a/mysql-test/r/type_float.result
+++ b/mysql-test/r/type_float.result
@@ -278,4 +278,75 @@ select 1e-308, 1.00000001e-300, 100000000e-300;
 select 10e307;
 10e307
 1e+308
+create table t1(a int, b double(8, 2));
+insert into t1 values
+(1, 28.50), (1, 121.85), (1, 157.23), (1, 1351.00), (1, -1965.35), (1, 81.75), 
+(1, 217.08), (1, 7.94), (4, 96.07), (4, 6404.65), (4, -6500.72), (2, 100.00),
+(5, 5.00), (5, -2104.80), (5, 2033.80), (5, 0.07), (5, 65.93),
+(3, -4986.24), (3, 5.00), (3, 4857.34), (3, 123.74), (3,  0.16),
+(6, -1695.31), (6, 1003.77), (6, 499.72), (6, 191.82);
+explain select sum(b) s from t1 group by a;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t1	ALL	NULL	NULL	NULL	NULL	26	Using temporary; Using filesort
+select sum(b) s from t1 group by a;
+s
+0.00
+100.00
+0.00
+-0.00
+-0.00
+0.00
+select sum(b) s from t1 group by a having s <> 0;
+s
+100.00
+select sum(b) s from t1 group by a having s <> 0 order by s;
+s
+100.00
+select sum(b) s from t1 group by a having s <=> 0;
+s
+0.00
+0.00
+-0.00
+-0.00
+0.00
+select sum(b) s from t1 group by a having s <=> 0 order by s;
+s
+-0.00
+-0.00
+0.00
+0.00
+0.00
+alter table t1 add key (a, b);
+explain select sum(b) s from t1 group by a;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t1	index	NULL	a	14	NULL	26	Using index
+select sum(b) s from t1 group by a;
+s
+0.00
+100.00
+0.00
+-0.00
+0.00
+0.00
+select sum(b) s from t1 group by a having s <> 0;
+s
+100.00
+select sum(b) s from t1 group by a having s <> 0 order by s;
+s
+100.00
+select sum(b) s from t1 group by a having s <=> 0;
+s
+0.00
+0.00
+-0.00
+0.00
+0.00
+select sum(b) s from t1 group by a having s <=> 0 order by s;
+s
+-0.00
+0.00
+0.00
+0.00
+0.00
+drop table t1;
 End of 4.1 tests
diff --git a/mysql-test/t/type_float.test b/mysql-test/t/type_float.test
index 8a484f7bcd0..8c22d22ca66 100644
--- a/mysql-test/t/type_float.test
+++ b/mysql-test/t/type_float.test
@@ -188,4 +188,29 @@ select 1e-308, 1.00000001e-300, 100000000e-300;
 # check if overflows are detected correctly
 select 10e307;
 
+#
+# Bug #19690: ORDER BY eliminates rows from the result
+#
+create table t1(a int, b double(8, 2));
+insert into t1 values
+(1, 28.50), (1, 121.85), (1, 157.23), (1, 1351.00), (1, -1965.35), (1, 81.75), 
+(1, 217.08), (1, 7.94), (4, 96.07), (4, 6404.65), (4, -6500.72), (2, 100.00),
+(5, 5.00), (5, -2104.80), (5, 2033.80), (5, 0.07), (5, 65.93),
+(3, -4986.24), (3, 5.00), (3, 4857.34), (3, 123.74), (3,  0.16),
+(6, -1695.31), (6, 1003.77), (6, 499.72), (6, 191.82);
+explain select sum(b) s from t1 group by a;
+select sum(b) s from t1 group by a;
+select sum(b) s from t1 group by a having s <> 0;
+select sum(b) s from t1 group by a having s <> 0 order by s;
+select sum(b) s from t1 group by a having s <=> 0;
+select sum(b) s from t1 group by a having s <=> 0 order by s;
+alter table t1 add key (a, b);
+explain select sum(b) s from t1 group by a;
+select sum(b) s from t1 group by a;
+select sum(b) s from t1 group by a having s <> 0;
+select sum(b) s from t1 group by a having s <> 0 order by s;
+select sum(b) s from t1 group by a having s <=> 0;
+select sum(b) s from t1 group by a having s <=> 0 order by s;
+drop table t1;
+
 --echo End of 4.1 tests
diff --git a/sql/field.cc b/sql/field.cc
index e88b8b313e2..acc837c1d37 100644
--- a/sql/field.cc
+++ b/sql/field.cc
@@ -3318,7 +3318,7 @@ int Field_double::store(double nr)
   else 
   {
     double max_value;
-    if (dec >= NOT_FIXED_DEC)
+    if (not_fixed)
     {
       max_value= DBL_MAX;
     }
diff --git a/sql/field.h b/sql/field.h
index e4991ba1961..b10d950163c 100644
--- a/sql/field.h
+++ b/sql/field.h
@@ -616,6 +616,7 @@ class Field_float :public Field_num {
 
 class Field_double :public Field_num {
 public:
+  my_bool not_fixed;
   Field_double(char *ptr_arg, uint32 len_arg, uchar *null_ptr_arg,
 	       uchar null_bit_arg,
 	       enum utype unireg_check_arg, const char *field_name_arg,
@@ -623,12 +624,20 @@ class Field_double :public Field_num {
 	       uint8 dec_arg,bool zero_arg,bool unsigned_arg)
     :Field_num(ptr_arg, len_arg, null_ptr_arg, null_bit_arg,
 	       unireg_check_arg, field_name_arg, table_arg,
-	       dec_arg, zero_arg,unsigned_arg)
+	       dec_arg, zero_arg, unsigned_arg),
+     not_fixed(dec_arg >= NOT_FIXED_DEC)
     {}
   Field_double(uint32 len_arg, bool maybe_null_arg, const char *field_name_arg,
 	       struct st_table *table_arg, uint8 dec_arg)
-    :Field_num((char*) 0, len_arg, maybe_null_arg ? (uchar*) "": 0, (uint) 0,
-	       NONE, field_name_arg, table_arg,dec_arg,0,0)
+    :Field_num((char *) 0, len_arg, maybe_null_arg ? (uchar *) "" : 0, (uint) 0,
+	       NONE, field_name_arg, table_arg,dec_arg, 0, 0),
+     not_fixed(dec_arg >= NOT_FIXED_DEC)
+    {}
+  Field_double(uint32 len_arg, bool maybe_null_arg, const char *field_name_arg,
+	       struct st_table *table_arg, uint8 dec_arg, my_bool not_fixed_srg)
+    :Field_num((char *) 0, len_arg, maybe_null_arg ? (uchar *) "" : 0, (uint) 0,
+	       NONE, field_name_arg, table_arg, dec_arg, 0, 0), 
+    not_fixed(not_fixed_srg)
     {}
   enum_field_types type() const { return FIELD_TYPE_DOUBLE;}
   enum ha_base_keytype key_type() const { return HA_KEYTYPE_DOUBLE; }
diff --git a/sql/init.cc b/sql/init.cc
index 4beb8db0c6f..5e1b6532c75 100644
--- a/sql/init.cc
+++ b/sql/init.cc
@@ -45,6 +45,12 @@ void unireg_init(ulong options)
   {					/* It's used by filesort... */
     log_10[i]= nr ; nr*= 10.0;
   }
+  /* Make a tab of powers of 0.1 */
+  for (i= 0, nr= 0.1; i < array_elements(log_01); i++)
+  {
+    log_01[i]= nr;
+    nr*= 0.1;
+  }
   specialflag|=options;			/* Set options from argv */
   DBUG_VOID_RETURN;
 }
diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index 859b4e0ecc1..65f9b279a18 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -352,6 +352,17 @@ int Arg_comparator::set_compare_func(Item_bool_func2 *item, Item_result type)
         func= &Arg_comparator::compare_e_int_diff_signedness;
     }
   }
+  else if (type == REAL_RESULT)
+  {
+    if ((*a)->decimals < NOT_FIXED_DEC && (*b)->decimals < NOT_FIXED_DEC)
+    {
+      precision= 5 * log_01[max((*a)->decimals, (*b)->decimals)];
+      if (func == &Arg_comparator::compare_real)
+        func= &Arg_comparator::compare_real_fixed;
+      else if (func == &Arg_comparator::compare_e_real)
+        func= &Arg_comparator::compare_e_real_fixed;
+    }
+  }
   return 0;
 }
 
@@ -459,6 +470,44 @@ int Arg_comparator::compare_e_real()
   return test(val1 == val2);
 }
 
+
+int Arg_comparator::compare_real_fixed()
+{
+  /*
+    Fix yet another manifestation of Bug#2338. 'Volatile' will instruct
+    gcc to flush double values out of 80-bit Intel FPU registers before
+    performing the comparison.
+  */
+  volatile double val1, val2;
+  val1= (*a)->val();
+  if (!(*a)->null_value)
+  {
+    val2= (*b)->val();
+    if (!(*b)->null_value)
+    {
+      owner->null_value= 0;
+      if (val1 == val2 || fabs(val1 - val2) < precision)
+        return 0;
+      if (val1 < val2)
+        return -1;
+      return 1;
+    }
+  }
+  owner->null_value= 1;
+  return -1;
+}
+
+
+int Arg_comparator::compare_e_real_fixed()
+{
+  double val1= (*a)->val();
+  double val2= (*b)->val();
+  if ((*a)->null_value || (*b)->null_value)
+    return test((*a)->null_value && (*b)->null_value);
+  return test(val1 == val2 || fabs(val1 - val2) < precision);
+}
+
+
 int Arg_comparator::compare_int_signed()
 {
   longlong val1= (*a)->val_int();
diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h
index 0e157fd412c..3dc09f0789a 100644
--- a/sql/item_cmpfunc.h
+++ b/sql/item_cmpfunc.h
@@ -33,6 +33,7 @@ class Arg_comparator: public Sql_alloc
   arg_cmp_func func;
   Item_bool_func2 *owner;
   Arg_comparator *comparators;   // used only for compare_row()
+  double precision;
 
 public:
   DTCollation cmp_collation;
@@ -77,6 +78,8 @@ class Arg_comparator: public Sql_alloc
   int compare_e_int();           // compare args[0] & args[1]
   int compare_e_int_diff_signedness();
   int compare_e_row();           // compare args[0] & args[1]
+  int compare_real_fixed();
+  int compare_e_real_fixed();
 
   static arg_cmp_func comparator_matrix [4][2];
 
diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h
index 3a240612cfa..40b7b9bcb1a 100644
--- a/sql/mysql_priv.h
+++ b/sql/mysql_priv.h
@@ -907,6 +907,7 @@ extern char glob_hostname[FN_REFLEN], mysql_home[FN_REFLEN];
 extern char pidfile_name[FN_REFLEN], system_time_zone[30], *opt_init_file;
 extern char log_error_file[FN_REFLEN];
 extern double log_10[32];
+extern double log_01[32];
 extern ulonglong log_10_int[20];
 extern ulonglong keybuff_size;
 extern ulong refresh_version,flush_version, thread_id,query_id,opened_tables;
diff --git a/sql/mysqld.cc b/sql/mysqld.cc
index 024e4682a22..d31cafd9a33 100644
--- a/sql/mysqld.cc
+++ b/sql/mysqld.cc
@@ -361,6 +361,7 @@ ulong my_bind_addr;			/* the address we bind to */
 volatile ulong cached_thread_count= 0;
 
 double log_10[32];			/* 10 potences */
+double log_01[32];
 time_t start_time;
 
 char mysql_home[FN_REFLEN], pidfile_name[FN_REFLEN], system_time_zone[30];
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index 0254e8f56dc..a6881337d83 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -5006,6 +5006,8 @@ static Field* create_tmp_field_from_field(THD *thd, Field* org_field,
       new_field->flags&= ~NOT_NULL_FLAG;	// Because of outer join
     if (org_field->type() == FIELD_TYPE_VAR_STRING)
       table->db_create_options|= HA_OPTION_PACK_RECORD;
+    else if (org_field->type() == FIELD_TYPE_DOUBLE)
+      ((Field_double *) new_field)->not_fixed= TRUE;
   }
   return new_field;
 }
@@ -5045,7 +5047,7 @@ static Field* create_tmp_field_from_item(THD *thd, Item *item, TABLE *table,
   switch (item->result_type()) {
   case REAL_RESULT:
     new_field=new Field_double(item->max_length, maybe_null,
-			       item->name, table, item->decimals);
+			       item->name, table, item->decimals, TRUE);
     break;
   case INT_RESULT:
     new_field=new Field_longlong(item->max_length, maybe_null,
@@ -5136,8 +5138,8 @@ Field *create_tmp_field(THD *thd, TABLE *table,Item *item, Item::Type type,
 	return new Field_string(sizeof(double)+sizeof(longlong),
 				0, item->name,table,&my_charset_bin);
       else
-	return new Field_double(item_sum->max_length,maybe_null,
-				item->name, table, item_sum->decimals);
+	return new Field_double(item_sum->max_length, maybe_null,
+				item->name, table, item_sum->decimals, TRUE);
     case Item_sum::VARIANCE_FUNC:		/* Place for sum & count */
     case Item_sum::STD_FUNC:
       if (group)
@@ -5145,7 +5147,7 @@ Field *create_tmp_field(THD *thd, TABLE *table,Item *item, Item::Type type,
 				 0, item->name,table,&my_charset_bin);
       else
 	return new Field_double(item_sum->max_length, maybe_null,
-				item->name,table,item_sum->decimals);
+				item->name, table, item_sum->decimals, TRUE);
     case Item_sum::UNIQUE_USERS_FUNC:
       return new Field_long(9,maybe_null,item->name,table,1);
     case Item_sum::MIN_FUNC:
@@ -5160,8 +5162,8 @@ Field *create_tmp_field(THD *thd, TABLE *table,Item *item, Item::Type type,
     default:
       switch (item_sum->result_type()) {
       case REAL_RESULT:
-	return new Field_double(item_sum->max_length,maybe_null,
-				item->name,table,item_sum->decimals);
+	return new Field_double(item_sum->max_length, maybe_null,
+				item->name, table, item_sum->decimals, TRUE);
       case INT_RESULT:
 	return new Field_longlong(item_sum->max_length,maybe_null,
 				  item->name,table,item->unsigned_flag);
-- 
2.30.9