MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Evgeny Potemkin Date:July 20 2010 4:22am
Subject:bzr commit into mysql-next-mr-bugfixing branch (epotemkin:3316)
Bug#49771
View as plain text  
#At file:///work/bzrroot/mysql-next-mr-bugfixing/ based on revid:alfranio.correia@stripped

 3316 Evgeny Potemkin	2010-07-20 [merge]
      Auto-merged fix for the bug#49771.

    modified:
      mysql-test/r/group_by.result
      mysql-test/t/group_by.test
      sql/item.cc
      sql/item.h
      sql/item_cmpfunc.cc
      sql/item_cmpfunc.h
      sql/item_sum.cc
      sql/item_sum.h
=== modified file 'mysql-test/r/group_by.result'
--- a/mysql-test/r/group_by.result	2010-05-23 20:41:18 +0000
+++ b/mysql-test/r/group_by.result	2010-07-19 17:11:47 +0000
@@ -1812,3 +1812,32 @@ MAX(t2.a)
 DROP TABLE t1, t2;
 #
 # End of 5.1 tests
+#
+# Bug#49771: Incorrect MIN (date) when minimum value is 0000-00-00
+#
+CREATE TABLE t1 (f1 int, f2 DATE);
+INSERT INTO t1 VALUES (1,'2004-04-19'), (1,'0000-00-00'), (1,'2004-04-18'),
+(2,'2004-05-19'), (2,'0001-01-01'), (3,'2004-04-10');
+SELECT MIN(f2),MAX(f2) FROM t1;
+MIN(f2)	MAX(f2)
+0000-00-00	2004-05-19
+SELECT f1,MIN(f2),MAX(f2) FROM t1 GROUP BY 1;
+f1	MIN(f2)	MAX(f2)
+1	0000-00-00	2004-04-19
+2	0001-01-01	2004-05-19
+3	2004-04-10	2004-04-10
+DROP TABLE t1;
+CREATE TABLE t1 ( f1 int, f2 time);
+INSERT INTO t1 VALUES (1,'01:27:35'), (1,'06:11:01'), (2,'19:53:05'),
+(2,'21:44:25'), (3,'10:55:12'), (3,'05:45:11'), (4,'00:25:00');
+SELECT MIN(f2),MAX(f2) FROM t1;
+MIN(f2)	MAX(f2)
+00:25:00	21:44:25
+SELECT f1,MIN(f2),MAX(f2) FROM t1 GROUP BY 1;
+f1	MIN(f2)	MAX(f2)
+1	01:27:35	06:11:01
+2	19:53:05	21:44:25
+3	05:45:11	10:55:12
+4	00:25:00	00:25:00
+DROP TABLE t1;
+#End of test#49771

=== modified file 'mysql-test/t/group_by.test'
--- a/mysql-test/t/group_by.test	2010-05-23 20:41:18 +0000
+++ b/mysql-test/t/group_by.test	2010-07-19 17:11:47 +0000
@@ -1224,3 +1224,26 @@ DROP TABLE t1, t2;
 --echo #
 
 --echo # End of 5.1 tests
+
+--echo #
+--echo # Bug#49771: Incorrect MIN (date) when minimum value is 0000-00-00
+--echo #
+CREATE TABLE t1 (f1 int, f2 DATE);
+
+INSERT INTO t1 VALUES (1,'2004-04-19'), (1,'0000-00-00'), (1,'2004-04-18'),
+(2,'2004-05-19'), (2,'0001-01-01'), (3,'2004-04-10');
+
+SELECT MIN(f2),MAX(f2) FROM t1;
+SELECT f1,MIN(f2),MAX(f2) FROM t1 GROUP BY 1;
+
+DROP TABLE t1;
+
+CREATE TABLE t1 ( f1 int, f2 time);
+INSERT INTO t1 VALUES (1,'01:27:35'), (1,'06:11:01'), (2,'19:53:05'),
+(2,'21:44:25'), (3,'10:55:12'), (3,'05:45:11'), (4,'00:25:00');
+
+SELECT MIN(f2),MAX(f2) FROM t1;
+SELECT f1,MIN(f2),MAX(f2) FROM t1 GROUP BY 1;
+
+DROP TABLE t1;
+--echo #End of test#49771

=== modified file 'sql/item.cc'
--- a/sql/item.cc	2010-07-16 21:00:50 +0000
+++ b/sql/item.cc	2010-07-20 04:22:14 +0000
@@ -4914,11 +4914,8 @@ Item *Item_field::equal_fields_propagato
     e.g. <bin_col> = <int_col> AND <bin_col> = <hex_string>) since
     Items don't know the context they are in and there are functions like 
     IF (<hex_string>, 'yes', 'no').
-    The same problem occurs when comparing a DATE/TIME field with a
-    DATE/TIME represented as an int and as a string.
   */
-  if (!item ||
-      (cmp_context != (Item_result)-1 && item->cmp_context != cmp_context))
+  if (!item || !has_compatible_context(item))
     item= this;
   else if (field && (field->flags & ZEROFILL_FLAG) && IS_NUM(field->type()))
   {
@@ -4982,8 +4979,7 @@ Item *Item_field::replace_equal_field(uc
     Item *const_item= item_equal->get_const();
     if (const_item)
     {
-      if (cmp_context != (Item_result)-1 &&
-          const_item->cmp_context != cmp_context)
+      if (!has_compatible_context(const_item))
         return this;
       return const_item;
     }
@@ -5053,21 +5049,6 @@ enum_field_types Item::field_type() cons
 }
 
 
-bool Item::is_datetime()
-{
-  switch (field_type())
-  {
-    case MYSQL_TYPE_DATE:
-    case MYSQL_TYPE_DATETIME:
-    case MYSQL_TYPE_TIMESTAMP:
-      return TRUE;
-    default:
-      break;
-  }
-  return FALSE;
-}
-
-
 String *Item::check_well_formed_result(String *str, bool send_error)
 {
   /* Check whether we got a well-formed string */
@@ -7468,6 +7449,8 @@ bool  Item_cache_datetime::cache_value_i
     return FALSE;
 
   value_cached= TRUE;
+  // Mark cached string value obsolete
+  str_value_cached= FALSE;
   /* Assume here that the underlying item will do correct conversion.*/
   int_value= example->val_int_result();
   null_value= example->null_value;
@@ -7480,7 +7463,13 @@ bool  Item_cache_datetime::cache_value()
 {
   if (!example)
     return FALSE;
+
+  if (cmp_context == INT_RESULT)
+    return cache_value_int();
+
   str_value_cached= TRUE;
+  // Mark cached int value obsolete
+  value_cached= FALSE;
   /* Assume here that the underlying item will do correct conversion.*/
   String *res= example->str_result(&str_value);
   if (res && res != &str_value)
@@ -7504,8 +7493,47 @@ void Item_cache_datetime::store(Item *it
 String *Item_cache_datetime::val_str(String *str)
 {
   DBUG_ASSERT(fixed == 1);
-  if (!str_value_cached && !cache_value())
-    return NULL;
+  if (!str_value_cached)
+  {
+    /*
+      When it's possible the Item_cache_datetime uses INT datetime
+      representation due to speed reasons. But still, it always has the STRING
+      result type and thus it can be asked to return a string value. 
+      It is possible that at this time cached item doesn't contain correct
+      string value, thus we have to convert cached int value to string and
+      return it.
+    */
+    if (value_cached)
+    {
+      MYSQL_TIME ltime;
+      if (str_value.alloc(MAX_DATE_STRING_REP_LENGTH))
+        return NULL;
+      if (cached_field_type == MYSQL_TYPE_TIME)
+      {
+        ulonglong time= int_value;
+        DBUG_ASSERT(time < TIME_MAX_VALUE);
+        set_zero_time(&ltime, MYSQL_TIMESTAMP_TIME);
+        ltime.second= time % 100;
+        time/= 100;
+        ltime.minute= time % 100;
+        time/= 100;
+        ltime.hour= time % 100;
+      }
+      else
+      {
+        int was_cut;
+        longlong res;
+        res= number_to_datetime(val_int(), &ltime, TIME_FUZZY_DATE, &was_cut);
+        if (res == -1)
+          return NULL;
+      }
+      str_value.length(my_TIME_to_str(&ltime,
+                                      const_cast<char*>(str_value.ptr())));
+      str_value_cached= TRUE;
+    }
+    else if (!cache_value())
+      return NULL;
+  }
   return &str_value;
 }
 

=== modified file 'sql/item.h'
--- a/sql/item.h	2010-07-07 07:30:01 +0000
+++ b/sql/item.h	2010-07-20 04:22:14 +0000
@@ -1171,7 +1171,40 @@ public:
     representation is more precise than the string one).
   */
   virtual bool result_as_longlong() { return FALSE; }
-  bool is_datetime();
+  inline bool is_datetime() const
+  {
+    switch (field_type())
+    {
+      case MYSQL_TYPE_DATE:
+      case MYSQL_TYPE_DATETIME:
+      case MYSQL_TYPE_TIMESTAMP:
+        return TRUE;
+      default:
+        break;
+    }
+    return FALSE;
+  }
+  /**
+    Check whether this and the given item has compatible comparison context.
+    Used by the equality propagation. See Item_field::equal_fields_propagator.
+
+    @return
+      TRUE  if the context is the same or if fields could be
+            compared as DATETIME values by the Arg_comparator.
+      FALSE otherwise.
+  */
+  inline bool has_compatible_context(Item *item) const
+  {
+    /* Same context. */
+    if (cmp_context == (Item_result)-1 || item->cmp_context == cmp_context)
+      return TRUE;
+    /* DATETIME comparison context. */
+    if (is_datetime())
+      return item->is_datetime() || item->cmp_context == STRING_RESULT;
+    if (item->is_datetime())
+      return is_datetime() || cmp_context == STRING_RESULT;
+    return FALSE;
+  }
   virtual Field::geometry_type get_geometry_type() const
     { return Field::GEOM_GEOMETRY; };
   String *check_well_formed_result(String *str, bool send_error= 0);
@@ -3232,6 +3265,7 @@ public:
   virtual bool cache_value()= 0;
   bool basic_const_item() const
   { return test(example && example->basic_const_item());}
+  virtual void clear() { null_value= TRUE; value_cached= FALSE; }
 };
 
 
@@ -3412,6 +3446,7 @@ public:
   */
   bool cache_value_int();
   bool cache_value();
+  void clear() { Item_cache::clear(); str_value_cached= FALSE; }
 };
 
 

=== modified file 'sql/item_cmpfunc.cc'
--- a/sql/item_cmpfunc.cc	2010-07-16 21:00:50 +0000
+++ b/sql/item_cmpfunc.cc	2010-07-20 04:22:14 +0000
@@ -876,8 +876,10 @@ get_time_value(THD *thd, Item ***item_ar
     Do not cache GET_USER_VAR() function as its const_item() may return TRUE
     for the current thread but it still may change during the execution.
   */
-  if (item->const_item() && cache_arg && (item->type() != Item::FUNC_ITEM ||
-      ((Item_func*)item)->functype() != Item_func::GUSERVAR_FUNC))
+  if (item->const_item() && cache_arg &&
+      item->type() != Item::CACHE_ITEM &&
+      (item->type() != Item::FUNC_ITEM ||
+       ((Item_func*)item)->functype() != Item_func::GUSERVAR_FUNC))
   {
     Item_cache_int *cache= new Item_cache_int();
     /* Mark the cache as non-const to prevent re-caching. */
@@ -937,6 +939,7 @@ int Arg_comparator::set_cmp_func(Item_re
     get_value_a_func= &get_datetime_value;
     get_value_b_func= &get_datetime_value;
     cmp_collation.set(&my_charset_numeric);
+    set_cmp_context_for_datetime();
     return 0;
   }
   else if (type == STRING_RESULT && (*a)->field_type() == MYSQL_TYPE_TIME &&
@@ -949,6 +952,7 @@ int Arg_comparator::set_cmp_func(Item_re
     func= &Arg_comparator::compare_datetime;
     get_value_a_func= &get_time_value;
     get_value_b_func= &get_time_value;
+    set_cmp_context_for_datetime();
     return 0;
   }
   else if (type == STRING_RESULT &&
@@ -1005,6 +1009,7 @@ bool Arg_comparator::try_year_cmp_func(I
 
   is_nulls_eq= is_owner_equal_func();
   func= &Arg_comparator::compare_datetime;
+  set_cmp_context_for_datetime();
 
   return TRUE;
 }
@@ -1058,6 +1063,7 @@ void Arg_comparator::set_datetime_cmp_fu
   func= &Arg_comparator::compare_datetime;
   get_value_a_func= &get_datetime_value;
   get_value_b_func= &get_datetime_value;
+  set_cmp_context_for_datetime();
 }
 
 
@@ -1144,8 +1150,10 @@ get_datetime_value(THD *thd, Item ***ite
     Do not cache GET_USER_VAR() function as its const_item() may return TRUE
     for the current thread but it still may change during the execution.
   */
-  if (item->const_item() && cache_arg && (item->type() != Item::FUNC_ITEM ||
-      ((Item_func*)item)->functype() != Item_func::GUSERVAR_FUNC))
+  if (item->const_item() && cache_arg &&
+      item->type() != Item::CACHE_ITEM &&
+      (item->type() != Item::FUNC_ITEM ||
+       ((Item_func*)item)->functype() != Item_func::GUSERVAR_FUNC))
   {
     Item_cache_int *cache= new Item_cache_int(MYSQL_TYPE_DATETIME);
     /* Mark the cache as non-const to prevent re-caching. */

=== modified file 'sql/item_cmpfunc.h'
--- a/sql/item_cmpfunc.h	2010-07-02 02:58:51 +0000
+++ b/sql/item_cmpfunc.h	2010-07-20 04:22:14 +0000
@@ -123,7 +123,17 @@ public:
     delete [] comparators;
     comparators= 0;
   }
-
+  /*
+    Set correct cmp_context if items would be compared as INTs.
+  */
+  inline void set_cmp_context_for_datetime()
+  {
+    DBUG_ASSERT(func == &Arg_comparator::compare_datetime);
+    if ((*a)->result_as_longlong())
+      (*a)->cmp_context= INT_RESULT;
+    if ((*b)->result_as_longlong())
+      (*b)->cmp_context= INT_RESULT;
+  }
   friend class Item_func;
 };
 

=== modified file 'sql/item_sum.cc'
--- a/sql/item_sum.cc	2010-07-02 02:58:51 +0000
+++ b/sql/item_sum.cc	2010-07-20 04:22:14 +0000
@@ -1217,8 +1217,7 @@ void Item_sum_hybrid::setup_hybrid(Item 
 {
   value= Item_cache::get_cache(item);
   value->setup(item);
-  if (value_arg)
-    value->store(value_arg);
+  value->store(value_arg);
   cmp= new Arg_comparator();
   cmp->set_cmp_func(this, args, (Item**)&value, FALSE);
   collation.set(item->collation);
@@ -1906,7 +1905,7 @@ void Item_sum_variance::update_field()
 
 void Item_sum_hybrid::clear()
 {
-  value->null_value= 1;
+  value->clear();
   null_value= 1;
 }
 

=== modified file 'sql/item_sum.h'
--- a/sql/item_sum.h	2010-07-02 02:58:51 +0000
+++ b/sql/item_sum.h	2010-07-20 04:22:14 +0000
@@ -1008,6 +1008,11 @@ protected:
   void no_rows_in_result();
   Field *create_tmp_field(bool group, TABLE *table,
 			  uint convert_blob_length);
+  /*
+    MIN/MAX uses Item_cache_datetime for storing DATETIME values, thus
+    in this case a correct INT value can be provided.
+  */
+  bool result_as_longlong() { return args[0]->result_as_longlong(); }
 };
 
 


Attachment: [text/bzr-bundle] bzr/epotemkin@mysql.com-20100720042214-39udsg3vt9iutg71.bundle
Thread
bzr commit into mysql-next-mr-bugfixing branch (epotemkin:3316)Bug#49771Evgeny Potemkin20 Jul