MDEV-8793 Wrong result set for SELECT ...

          WHERE COALESCE(time_column)=TIME('00:00:00')
            AND COALESCE(time_column)=DATE('2015-09-11')
MDEV-8814 Wrong result for WHERE datetime_column > TIME('00:00:00')

6 files changed, 164 insertions, 110 deletions

diff --git a/sql/item.cc b/sql/item.cc
index 55159cb9df0..a8641216573 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -8596,7 +8596,8 @@ void resolve_const_item(THD *thd, Item **ref, Item *comp_item)
     bool is_null;
     Item **ref_copy= ref;
     /* the following call creates a constant and puts it in new_item */
-    get_datetime_value(thd, &ref_copy, &new_item, comp_item, &is_null);
+    enum_field_types type= item->field_type_for_temporal_comparison(comp_item);
+    get_datetime_value(thd, &ref_copy, &new_item, type, &is_null);
     if (is_null)
       new_item= new (mem_root) Item_null(thd, name);
     break;
@@ -8936,9 +8937,25 @@ Item_cache_temporal::Item_cache_temporal(THD *thd,
 }
 
 
-longlong Item_cache_temporal::val_temporal_packed()
+longlong Item_cache_temporal::val_datetime_packed()
 {
   DBUG_ASSERT(fixed == 1);
+  if (Item_cache_temporal::field_type() == MYSQL_TYPE_TIME)
+    return Item::val_datetime_packed(); // TIME-to-DATETIME conversion needed
+  if ((!value_cached && !cache_value()) || null_value)
+  {
+    null_value= TRUE;
+    return 0;
+  }
+  return value;
+}
+
+
+longlong Item_cache_temporal::val_time_packed()
+{
+  DBUG_ASSERT(fixed == 1);
+  if (Item_cache_temporal::field_type() != MYSQL_TYPE_TIME)
+    return Item::val_time_packed(); // DATETIME-to-TIME conversion needed
   if ((!value_cached && !cache_value()) || null_value)
   {
     null_value= TRUE;
diff --git a/sql/item.h b/sql/item.h
index 2b845064dce..f727c33326b 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -1221,6 +1221,48 @@ public:
   */
   bool get_time_with_conversion(THD *thd, MYSQL_TIME *ltime,
                                 ulonglong fuzzydate);
+  // Get a DATE or DATETIME value in numeric packed format for comparison
+  virtual longlong val_datetime_packed()
+  {
+    MYSQL_TIME ltime;
+    uint fuzzydate= TIME_FUZZY_DATES | TIME_INVALID_DATES;
+    return get_date_with_conversion(&ltime, fuzzydate) ? 0 : pack_time(&ltime);
+  }
+  // Get a TIME value in numeric packed format for comparison
+  virtual longlong val_time_packed()
+  {
+    MYSQL_TIME ltime;
+    uint fuzzydate= TIME_FUZZY_DATES | TIME_INVALID_DATES | TIME_TIME_ONLY;
+    return get_date(&ltime, fuzzydate) ? 0 : pack_time(&ltime);
+  }
+  // Get a temporal value in packed DATE/DATETIME or TIME format
+  longlong val_temporal_packed(enum_field_types f_type)
+  {
+    return f_type == MYSQL_TYPE_TIME ? val_time_packed() :
+                                       val_datetime_packed();
+  }
+  enum_field_types field_type_for_temporal_comparison(const Item *other) const
+  {
+    if (cmp_type() == TIME_RESULT)
+    {
+      if (other->cmp_type() == TIME_RESULT)
+        return Field::field_type_merge(field_type(), other->field_type());
+      else
+        return field_type();
+    }
+    else
+    {
+      if (other->cmp_type() == TIME_RESULT)
+        return other->field_type();
+      DBUG_ASSERT(0); // Two non-temporal data types, we should not get to here
+      return MYSQL_TYPE_DATETIME;
+    }
+  }
+  // Get a temporal value to compare to another Item
+  longlong val_temporal_packed(const Item *other)
+  {
+    return val_temporal_packed(field_type_for_temporal_comparison(other));
+  }
   bool get_seconds(ulonglong *sec, ulong *sec_part);
   virtual bool get_date_result(MYSQL_TIME *ltime, ulonglong fuzzydate)
   { return get_date(ltime,fuzzydate); }
@@ -4898,7 +4940,8 @@ public:
   String* val_str(String *str);
   my_decimal *val_decimal(my_decimal *);
   longlong val_int();
-  longlong val_temporal_packed();
+  longlong val_datetime_packed();
+  longlong val_time_packed();
   double val_real();
   bool cache_value();
   bool get_date(MYSQL_TIME *ltime, ulonglong fuzzydate);
diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index 05980c4668c..a5369f42a62 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -720,17 +720,16 @@ bool Arg_comparator::agg_arg_charsets_for_comparison()
 */
 
 int Arg_comparator::set_cmp_func(Item_func_or_sum *owner_arg,
-                                        Item **a1, Item **a2,
-                                        Item_result type)
+                                 Item **a1, Item **a2)
 {
   thd= current_thd;
   owner= owner_arg;
   set_null= set_null && owner_arg;
   a= a1;
   b= a2;
-  m_compare_type= type;
+  m_compare_type= item_cmp_type(*a1, *a2);
 
-  if (type == STRING_RESULT &&
+  if (m_compare_type == STRING_RESULT &&
       (*a)->result_type() == STRING_RESULT &&
       (*b)->result_type() == STRING_RESULT)
   {
@@ -741,14 +740,35 @@ int Arg_comparator::set_cmp_func(Item_func_or_sum *owner_arg,
     if (agg_arg_charsets_for_comparison())
       return 1;
   }
-  if (type == INT_RESULT &&
+
+  if (m_compare_type == TIME_RESULT)
+  {
+    enum_field_types f_type= a[0]->field_type_for_temporal_comparison(b[0]);
+    if (f_type == MYSQL_TYPE_TIME)
+    {
+      func= is_owner_equal_func() ? &Arg_comparator::compare_e_time :
+                                    &Arg_comparator::compare_time;
+    }
+    else
+    {
+      func= is_owner_equal_func() ? &Arg_comparator::compare_e_datetime :
+                                    &Arg_comparator::compare_datetime;
+    }
+    return 0;
+  }
+
+  if (m_compare_type == INT_RESULT &&
       (*a)->field_type() == MYSQL_TYPE_YEAR &&
       (*b)->field_type() == MYSQL_TYPE_YEAR)
-    type= TIME_RESULT;
+  {
+    m_compare_type= TIME_RESULT;
+    func= is_owner_equal_func() ? &Arg_comparator::compare_e_datetime :
+                                  &Arg_comparator::compare_datetime;
+  }
 
-  a= cache_converted_constant(thd, a, &a_cache, type);
-  b= cache_converted_constant(thd, b, &b_cache, type);
-  return set_compare_func(owner_arg, type);
+  a= cache_converted_constant(thd, a, &a_cache, m_compare_type);
+  b= cache_converted_constant(thd, b, &b_cache, m_compare_type);
+  return set_compare_func(owner_arg, m_compare_type);
 }
 
 
@@ -791,18 +811,6 @@ Item** Arg_comparator::cache_converted_constant(THD *thd_arg, Item **value,
 }
 
 
-void Arg_comparator::set_datetime_cmp_func(Item_func_or_sum *owner_arg,
-                                           Item **a1, Item **b1)
-{
-  thd= current_thd;
-  owner= owner_arg;
-  a= a1;
-  b= b1;
-  a_cache= 0;
-  b_cache= 0;
-  func= comparator_matrix[TIME_RESULT][is_owner_equal_func()];
-}
-
 /**
   Retrieves correct DATETIME value from given item.
 
@@ -836,34 +844,11 @@ void Arg_comparator::set_datetime_cmp_func(Item_func_or_sum *owner_arg,
 
 longlong
 get_datetime_value(THD *thd, Item ***item_arg, Item **cache_arg,
-                   Item *warn_item, bool *is_null)
+                   enum_field_types f_type, bool *is_null)
 {
   longlong UNINIT_VAR(value);
   Item *item= **item_arg;
-  enum_field_types f_type= item->cmp_type() == TIME_RESULT ?
-                           item->field_type() : warn_item->field_type();
-
-  if (item->result_type() == INT_RESULT &&
-      item->cmp_type() == TIME_RESULT &&
-      item->type() == Item::CACHE_ITEM)
-  {
-    /* it's our Item_cache_temporal, as created below */
-    DBUG_ASSERT(is_temporal_type(((Item_cache *) item)->field_type()));
-    value= ((Item_cache_temporal*) item)->val_temporal_packed();
-  }
-  else
-  {
-    MYSQL_TIME ltime;
-    uint fuzzydate= TIME_FUZZY_DATES | TIME_INVALID_DATES;
-    if ((item->field_type() == MYSQL_TYPE_TIME &&
-        is_temporal_type_with_date(warn_item->field_type())) ?
-        item->get_date_with_conversion(&ltime, fuzzydate) :
-        item->get_date(&ltime, fuzzydate |
-                               (f_type == MYSQL_TYPE_TIME ? TIME_TIME_ONLY : 0)))
-      value= 0; /* invalid date */
-    else
-      value= pack_time(&ltime);
-  }
+  value= item->val_temporal_packed(f_type);
   if ((*is_null= item->null_value))
     return ~(ulonglong) 0;
   if (cache_arg && item->const_item() &&
@@ -900,7 +885,7 @@ get_datetime_value(THD *thd, Item ***item_arg, Item **cache_arg,
        1   a > b
 */
 
-int Arg_comparator::compare_datetime()
+int Arg_comparator::compare_temporal(enum_field_types type)
 {
   bool a_is_null, b_is_null;
   longlong a_value, b_value;
@@ -909,12 +894,12 @@ int Arg_comparator::compare_datetime()
     owner->null_value= 1;
 
   /* Get DATE/DATETIME/TIME value of the 'a' item. */
-  a_value= get_datetime_value(thd, &a, &a_cache, *b, &a_is_null);
+  a_value= get_datetime_value(thd, &a, &a_cache, type, &a_is_null);
   if (a_is_null)
     return -1;
 
   /* Get DATE/DATETIME/TIME value of the 'b' item. */
-  b_value= get_datetime_value(thd, &b, &b_cache, *a, &b_is_null);
+  b_value= get_datetime_value(thd, &b, &b_cache, type, &b_is_null);
   if (b_is_null)
     return -1;
 
@@ -926,16 +911,16 @@ int Arg_comparator::compare_datetime()
   return a_value < b_value ? -1 : a_value > b_value ? 1 : 0;
 }
 
-int Arg_comparator::compare_e_datetime()
+int Arg_comparator::compare_e_temporal(enum_field_types type)
 {
   bool a_is_null, b_is_null;
   longlong a_value, b_value;
 
   /* Get DATE/DATETIME/TIME value of the 'a' item. */
-  a_value= get_datetime_value(thd, &a, &a_cache, *b, &a_is_null);
+  a_value= get_datetime_value(thd, &a, &a_cache, type, &a_is_null);
 
   /* Get DATE/DATETIME/TIME value of the 'b' item. */
-  b_value= get_datetime_value(thd, &b, &b_cache, *a, &b_is_null);
+  b_value= get_datetime_value(thd, &b, &b_cache, type, &b_is_null);
   return a_is_null || b_is_null ? a_is_null == b_is_null
                                 : a_value == b_value;
 }
@@ -2238,8 +2223,8 @@ longlong Item_func_between::val_int()
     bool value_is_null, a_is_null, b_is_null;
 
     ptr= &args[0];
-    value= get_datetime_value(thd, &ptr, &cache, compare_as_dates,
-                              &value_is_null);
+    enum_field_types f_type= field_type_for_temporal_comparison(compare_as_dates);
+    value= get_datetime_value(thd, &ptr, &cache, f_type, &value_is_null);
     if (ptr != &args[0])
       thd->change_item_tree(&args[0], *ptr);
 
@@ -2247,12 +2232,12 @@ longlong Item_func_between::val_int()
       return 0;
 
     ptr= &args[1];
-    a= get_datetime_value(thd, &ptr, &cache, compare_as_dates, &a_is_null);
+    a= get_datetime_value(thd, &ptr, &cache, f_type, &a_is_null);
     if (ptr != &args[1])
       thd->change_item_tree(&args[1], *ptr);
 
     ptr= &args[2];
-    b= get_datetime_value(thd, &ptr, &cache, compare_as_dates, &b_is_null);
+    b= get_datetime_value(thd, &ptr, &cache, f_type, &b_is_null);
     if (ptr != &args[2])
       thd->change_item_tree(&args[2], *ptr);
 
@@ -3700,11 +3685,9 @@ Item *in_longlong::create_item(THD *thd)
 
 void in_datetime::set(uint pos,Item *item)
 {
-  Item **tmp_item= &item;
-  bool is_null;
   struct packed_longlong *buff= &((packed_longlong*) base)[pos];
 
-  buff->val= get_datetime_value(thd, &tmp_item, 0, warn_item, &is_null);
+  buff->val= item->val_temporal_packed(warn_item);
   buff->unsigned_flag= 1L;
 }
 
@@ -3712,7 +3695,9 @@ uchar *in_datetime::get_value(Item *item)
 {
   bool is_null;
   Item **tmp_item= lval_cache ? &lval_cache : &item;
-  tmp.val= get_datetime_value(thd, &tmp_item, &lval_cache, warn_item, &is_null);
+  enum_field_types f_type=
+    tmp_item[0]->field_type_for_temporal_comparison(warn_item);
+  tmp.val= get_datetime_value(thd, &tmp_item, &lval_cache, f_type, &is_null);
   if (item->null_value)
     return 0;
   tmp.unsigned_flag= 1L;
@@ -3969,16 +3954,15 @@ void cmp_item_datetime::store_value(Item *item)
 {
   bool is_null;
   Item **tmp_item= lval_cache ? &lval_cache : &item;
-  value= get_datetime_value(thd, &tmp_item, &lval_cache, warn_item, &is_null);
+  enum_field_types f_type=
+    tmp_item[0]->field_type_for_temporal_comparison(warn_item);
+  value= get_datetime_value(thd, &tmp_item, &lval_cache, f_type, &is_null);
 }
 
 
 int cmp_item_datetime::cmp(Item *arg)
 {
-  bool is_null;
-  Item **tmp_item= &arg;
-  return value !=
-    get_datetime_value(thd, &tmp_item, 0, warn_item, &is_null);
+  return value != arg->val_temporal_packed(warn_item);
 }
 
 
@@ -5914,14 +5898,14 @@ Item *Item_bool_rowready_func2::negated_item(THD *thd)
 
 Item_equal::Item_equal(THD *thd, Item *f1, Item *f2, bool with_const_item):
   Item_bool_func(thd), eval_item(0), cond_false(0), cond_true(0),
-  context_field(NULL), link_equal_fields(FALSE)
+  context_field(NULL), link_equal_fields(FALSE),
+  m_compare_type(item_cmp_type(f1, f2)),
+  m_compare_collation(f2->collation.collation)
 {
   const_item_cache= 0;
   with_const= with_const_item;
   equal_items.push_back(f1, thd->mem_root);
   equal_items.push_back(f2, thd->mem_root);
-  cmp.cmp_collation.set(f2->collation);
-  cmp.set_compare_type(item_cmp_type(f1, f2));
   upper_levels= NULL;
 }
 
@@ -5940,7 +5924,9 @@ Item_equal::Item_equal(THD *thd, Item *f1, Item *f2, bool with_const_item):
 
 Item_equal::Item_equal(THD *thd, Item_equal *item_equal):
   Item_bool_func(thd), eval_item(0), cond_false(0), cond_true(0),
-  context_field(NULL), link_equal_fields(FALSE)
+  context_field(NULL), link_equal_fields(FALSE),
+  m_compare_type(item_equal->m_compare_type),
+  m_compare_collation(item_equal->m_compare_collation)
 {
   const_item_cache= 0;
   List_iterator_fast<Item> li(item_equal->equal_items);
@@ -5950,8 +5936,6 @@ Item_equal::Item_equal(THD *thd, Item_equal *item_equal):
     equal_items.push_back(item, thd->mem_root);
   }
   with_const= item_equal->with_const;
-  cmp.cmp_collation.set(item_equal->cmp.cmp_collation);
-  cmp.set_compare_type(item_equal->cmp.compare_type());
   cond_false= item_equal->cond_false;
   upper_levels= item_equal->upper_levels;
 }
@@ -5984,11 +5968,13 @@ void Item_equal::add_const(THD *thd, Item *c)
     return;
   }
   Item *const_item= get_const();
-  switch (cmp.compare_type()) {
+  switch (Item_equal::compare_type()) {
   case TIME_RESULT:
     {
-      cmp.set_datetime_cmp_func(this, &c, &const_item);
-      cond_false= cmp.compare();
+      enum_field_types f_type= context_field->field_type();
+      longlong value0= c->val_temporal_packed(f_type);
+      longlong value1= const_item->val_temporal_packed(f_type);
+      cond_false= c->null_value || const_item->null_value || value0 != value1;
       break;
     }
   case STRING_RESULT:
@@ -6037,8 +6023,8 @@ void Item_equal::add_const(THD *thd, Item *c)
           SELECT * FROM t1 WHERE a='const' AND a=NULL;
           SELECT * FROM t1 WHERE a='const' AND a=(SELECT MAX(a) FROM t2)
       */
-      cond_false= !(str1= const_item->val_str(&cmp.value1)) ||
-                  !(str2= c->val_str(&cmp.value2)) ||
+      cond_false= !(str1= const_item->val_str(&cmp_value1)) ||
+                  !(str2= c->val_str(&cmp_value2)) ||
                   !str1->eq(str2, compare_collation());
       break;
     }
diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h
index 4ea554059ce..727a7267c57 100644
--- a/sql/item_cmpfunc.h
+++ b/sql/item_cmpfunc.h
@@ -58,12 +58,12 @@ class Arg_comparator: public Sql_alloc
   Item *a_cache, *b_cache;         // Cached values of a and b items
                                    //   when one of arguments is NULL.
   int set_compare_func(Item_func_or_sum *owner, Item_result type);
-  inline int set_compare_func(Item_func_or_sum *owner_arg)
-  {
-    return set_compare_func(owner_arg, item_cmp_type(*a, *b));
-  }
+  int set_cmp_func(Item_func_or_sum *owner_arg, Item **a1, Item **a2);
   bool agg_arg_charsets_for_comparison();
 
+  int compare_temporal(enum_field_types type);
+  int compare_e_temporal(enum_field_types type);
+
 public:
   DTCollation cmp_collation;
   /* Allow owner function to use string buffers. */
@@ -76,20 +76,12 @@ public:
     m_compare_type(STRING_RESULT), set_null(TRUE),
     comparators(0), thd(0), a_cache(0), b_cache(0) {};
 
-private:
-  int set_cmp_func(Item_func_or_sum *owner_arg,
-			  Item **a1, Item **a2,
-			  Item_result type);
 public:
-  void set_compare_type(Item_result type)
-  {
-    m_compare_type= type;
-  }
   inline int set_cmp_func(Item_func_or_sum *owner_arg,
 			  Item **a1, Item **a2, bool set_null_arg)
   {
     set_null= set_null_arg;
-    return set_cmp_func(owner_arg, a1, a2, item_cmp_type(*a1, *a2));
+    return set_cmp_func(owner_arg, a1, a2);
   }
   inline int compare() { return (this->*func)(); }
 
@@ -111,13 +103,13 @@ public:
   int compare_e_row();           // compare args[0] & args[1]
   int compare_real_fixed();
   int compare_e_real_fixed();
-  int compare_datetime();        // compare args[0] & args[1] as DATETIMEs
-  int compare_e_datetime();
+  int compare_datetime()   { return compare_temporal(MYSQL_TYPE_DATETIME); }
+  int compare_e_datetime() { return compare_e_temporal(MYSQL_TYPE_DATETIME); }
+  int compare_time()       { return compare_temporal(MYSQL_TYPE_TIME); }
+  int compare_e_time()     { return compare_e_temporal(MYSQL_TYPE_TIME); }
 
   Item** cache_converted_constant(THD *thd, Item **value, Item **cache,
                                   Item_result type);
-  void set_datetime_cmp_func(Item_func_or_sum *owner_arg,
-                             Item **a1, Item **b1);
   static arg_cmp_func comparator_matrix [6][2];
   inline bool is_owner_equal_func()
   {
@@ -2052,12 +2044,6 @@ class Item_equal: public Item_bool_func
     the equal_items should be ignored.
   */
   bool cond_true;
-  /* 
-    The comparator used to compare constants equal to fields from equal_items
-    as datetimes. The comparator is used only if compare_as_dates=TRUE
-  */
-  Arg_comparator cmp;
- 
   /*
     For Item_equal objects inside an OR clause: one of the fields that were
     used in the original equality.
@@ -2066,6 +2052,9 @@ class Item_equal: public Item_bool_func
 
   bool link_equal_fields;
 
+  Item_result m_compare_type;
+  CHARSET_INFO *m_compare_collation;
+  String cmp_value1, cmp_value2;
 public:
 
   COND_EQUAL *upper_levels;       /* multiple equalities of upper and levels */
@@ -2103,9 +2092,8 @@ public:
   bool walk(Item_processor processor, bool walk_subquery, uchar *arg);
   Item *transform(THD *thd, Item_transformer transformer, uchar *arg);
   virtual void print(String *str, enum_query_type query_type);
-  Item_result compare_type() const { return cmp.compare_type(); }
-  CHARSET_INFO *compare_collation() const
-  { return cmp.cmp_collation.collation; }
+  Item_result compare_type() const { return m_compare_type; }
+  CHARSET_INFO *compare_collation() const { return m_compare_collation; }
 
   void set_context_field(Item_field *ctx_field) { context_field= ctx_field; }
   void set_link_equal_fields(bool flag) { link_equal_fields= flag; }
@@ -2309,7 +2297,7 @@ inline bool is_cond_or(Item *item)
 Item *and_expressions(Item *a, Item *b, Item **org_item);
 
 longlong get_datetime_value(THD *thd, Item ***item_arg, Item **cache_arg,
-                            Item *warn_item, bool *is_null);
+                            enum_field_types f_type, bool *is_null);
 
 
 bool get_mysql_time_from_str(THD *thd, String *str, timestamp_type warn_type,
diff --git a/sql/item_func.cc b/sql/item_func.cc
index e0cfa037780..f224a68ecbe 100644
--- a/sql/item_func.cc
+++ b/sql/item_func.cc
@@ -2910,10 +2910,7 @@ bool Item_func_min_max::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date)
 
   for (uint i=0; i < arg_count ; i++)
   {
-    Item **arg= args + i;
-    bool is_null_tmp;
-    longlong res= get_datetime_value(thd, &arg, 0, compare_as_dates,
-                                     &is_null_tmp);
+    longlong res= args[i]->val_temporal_packed(compare_as_dates);
 
     /* Check if we need to stop (because of error or KILL) and stop the loop */
     if (thd->is_error() || args[i]->null_value)
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index 2dc9f3a1ce2..f245bd89dba 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -13932,6 +13932,29 @@ can_change_cond_ref_to_const(Item_bool_func2 *target,
       target->compare_collation() == source->compare_collation() &&
       target_value->collation.collation == source_const->collation.collation;
   }
+  if (target->compare_type() == TIME_RESULT)
+  {
+    if (target_value->cmp_type() != TIME_RESULT)
+    {
+      /*
+        Can't rewrite:
+          WHERE COALESCE(time_column)='00:00:00'
+            AND COALESCE(time_column)=DATE'2015-09-11'
+        to
+          WHERE DATE'2015-09-11'='00:00:00'
+            AND COALESCE(time_column)=DATE'2015-09-11'
+        because the left part will erroneously try to parse '00:00:00'
+        as DATE, not as TIME.
+
+        TODO: It could still be rewritten to:
+          WHERE DATE'2015-09-11'=TIME'00:00:00'
+            AND COALESCE(time_column)=DATE'2015-09-11'
+        i.e. we need to replace both target_expr and target_value
+        at the same time. This is not supported yet.
+      */
+      return false;
+    }
+  }
   return true; // Non-string comparison
 }