Fixed bug#16377: result of DATE/TIME functions were compared as strings which

can lead to a wrong result.

All date/time functions has the STRING result type thus their results are
compared as strings. The string date representation allows a user to skip 
some of leading zeros. This can lead to wrong comparison result if a date/time 
function result is compared to such a string constant.

The idea behind this bug fix is to compare results of date/time functions
and data/time constants as ints, because that date/time representation is 
more exact. To achieve this the agg_cmp_type() is changed to take in the
account that a date/time field or an date/time item should be compared 
as ints.

This bug fix is partially back ported from 5.0.

The agg_cmp_type() function now accepts THD as one of parameters. 
In addition, it now checks if a date/time field/function is present in the
list. If so, it tries to coerce all constants to INT to make date/time
comparison return correct result. The field for the constant coercion is
taken from the Item_field or constructed from the Item_func. In latter case
the constructed field will be freed after conversion of all constant items.
Otherwise the result is same as before - aggregated with help of the
item_cmp_type() function.

From the Item_func_between::fix_length_and_dec() function removed the part
which was converting date/time constants to int if possible. Now this is 
done by the agg_cmp_type() function.

The new function result_as_longlong() is added to the Item class. 
It indicates that the item is a date/time item and result of it can be
compared as int. Such items are date/time fields/functions.

Correct val_int() methods are implemented for classes Item_date_typecast, 
Item_func_makedate, Item_time_typecast, Item_datetime_typecast. All these
classes are derived from Item_str_func and Item_str_func::val_int() converts
its string value to int without regard to the date/time type of these items.

Arg_comparator::set_compare_func() and Arg_comparator::set_cmp_func()
functions are changed to substitute result type of an item with the INT_RESULT
if the item is a date/time item and another item is a constant. This is done
to get a correct result of comparisons like date_time_function() = string_constant.


mysql-test/r/cast.result:
  Fixed wrong test case result after bug fix#16377.
sql/item_timefunc.h:
  Fixed bug#16377: result of DATE/TIME functions were compared as strings which
  can lead to a wrong result.
  The result_as_longlong() function is set to return TRUE for these classes:
  Item_date, Item_date_func, Item_func_curtime, Item_func_sec_to_time,
  Item_date_typecast, Item_time_typecast, Item_datetime_typecast,
  Item_func_makedate.
sql/item_timefunc.cc:
  Fixed bug#16377: result of DATE/TIME functions were compared as strings which
  can lead to a wrong result.Correct val_int() methods are implemented for classes Item_date_typecast, 
  Item_func_makedate, Item_time_typecast, Item_datetime_typecast.
sql/item_cmpfunc.h:
  Fixed bug#16377: result of DATE/TIME functions were compared as strings which
  can lead to a wrong result.
  Arg_comparator::set_compare_func() and Arg_comparator::set_cmp_func()
  functions are changed to substitute result type of an item with the INT_RESULT
  if the item is a date/time item and another item is a constant.
sql/field.cc:
  Fixed bug#16377: result of DATE/TIME functions were compared as strings which
  can lead to a wrong result.
  Field::set_warning(), Field::set_datetime_warning() now use current_thd to get thd if table isn't set.
sql/item_cmpfunc.cc:
  Fixed bug#16377: result of DATE/TIME functions were compared as strings which
  can lead to a wrong result.
  The agg_cmp_type() function now accepts THD as one of parameters. 
  In addition, it now checks if a date/time field/function is present in the
  list. If so, it tries to coerce all constants to INT to make date/time
  comparison return correct result. The field for the constant coercion is
  taken from the Item_field or constructed from the Item_func. In latter case
  the constructed field will be freed after conversion of all constant items.
  Otherwise the result is same as before - aggregated with help of the
  item_cmp_type() function.
sql/item.h:
  The new function result_as_longlong() is added to the Item class. 
  It indicates that the item is a date/time item and result of it can be
  compared as int. Such items are date/time fields/functions.
mysql-test/t/func_time.test:
  Added test case fot bug#16377: result of DATE/TIME functions were compared as strings which
  can lead to a wrong result.
mysql-test/r/func_time.result:
  Added test case fot bug#16377: result of DATE/TIME functions were compared as strings which
  can lead to a wrong result.

1 files changed, 122 insertions, 32 deletions

diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index bfbb3355004..e92e1d30ca4 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -25,6 +25,8 @@
 #include <m_ctype.h>
 #include "sql_select.h"
 
+static bool convert_constant_item(THD *thd, Field *field, Item **item);
+
 static Item_result item_store_type(Item_result a,Item_result b)
 {
   if (a == STRING_RESULT || b == STRING_RESULT)
@@ -64,6 +66,7 @@ static void agg_result_type(Item_result *type, Item **items, uint nitems)
 
   SYNOPSIS:
     agg_cmp_type()
+    thd          thread handle
     type   [out] the aggregated type
     items        array of items to aggregate the type from
     nitems       number of items in the array
@@ -78,24 +81,133 @@ static void agg_result_type(Item_result *type, Item **items, uint nitems)
     If all items are constants the type will be aggregated from all items.
     If there are some non-constant items then only types of non-constant
     items will be used for aggregation.
+    If there are DATE/TIME fields/functions in the list and no string
+    fields/functions in the list then:
+      The INT_RESULT type will be used for aggregation instead of orginal
+      result type of any DATE/TIME field/function in the list
+      All constant items in the list will be converted to a DATE/TIME using
+      found field or result field of found function.
+
+    Implementation notes:
+      The code is equvalent to:
+      1. Check the list for presense of a STRING field/function.
+         Collect the is_const flag.
+      2. Get a Field* object to use for type coercion
+      3. Perform type conversion.
+      1 and 2 are implemented in 2 loops. The first searches for a DATE/TIME
+      field/function and checks presense of a STRING field/function.
+      The second loop works only if a DATE/TIME field/function is found.
+      It checks presense of a STRING field/function in the rest of the list.
+
+  TODO
+    1) The current implementation can produce false comparison results for
+    expressions like:
+      date_time_field BETWEEN string_field_with_dates AND string_constant
+    if the string_constant will omit some of leading zeroes.
+    In order to fully implement correct comparison of DATE/TIME the new
+    DATETIME_RESULT result type should be introduced and agg_cmp_type()
+    should return the DATE/TIME field used for the conversion. Later
+    this field can be used by comparison functions like Item_func_between to
+    convert string values to ints on the fly and thus return correct results.
+    This modification will affect functions BETWEEN, IN and CASE.
+
+    2) If in the list a DATE field/function and a DATETIME field/function
+    are present in the list then the first found field/function will be
+    used for conversion. This may lead to wrong results and probably should
+    be fixed.
 */
-static void agg_cmp_type(Item_result *type, Item **items, uint nitems)
+
+static void agg_cmp_type(THD *thd, Item_result *type, Item **items, uint nitems)
 {
   uint i;
-  type[0]= items[0]->result_type();
+  Item::Type res;
+  char *buff= NULL;
+  uchar null_byte;
+  Field *field= NULL;
+
+  /* Search for date/time fields/functions */
+  for (i= 0; i < nitems; i++)
+  {
+    if (!items[i]->result_as_longlong())
+    {
+      /* Do not convert anything if a string field/function is present */
+      if (!items[i]->const_item() && items[i]->result_type() == STRING_RESULT)
+      {
+        i= nitems;
+        break;
+      }
+      continue;
+    }
+    if ((res= items[i]->real_item()->type()) == Item::FIELD_ITEM)
+    {
+      field= ((Item_field *)items[i]->real_item())->field;
+      break;
+    }
+    else if (res == Item::FUNC_ITEM)
+    {
+      field= items[i]->tmp_table_field_from_field_type(0);
+      if (field)
+        buff= alloc_root(thd->mem_root, field->max_length());
+      if (!buff || !field)
+      {
+        if (field)
+          delete field;
+        if (buff)
+          my_free(buff, MYF(MY_WME));
+        field= 0;
+      }
+      else
+        field->move_field(buff, &null_byte, 0);
+      break;
+    }
+  }
+  if (field)
+  {
+    /* Check the rest of the list for presense of a string field/function. */
+    for (i++ ; i < nitems; i++)
+    {
+      if (!items[i]->const_item() && items[i]->result_type() == STRING_RESULT &&
+          !items[i]->result_as_longlong())
+      {
+        field= 0;
+        break;
+      }
+    }
+  }
   /* Reset to 0 on first occurence of non-const item. 1 otherwise */
   bool is_const= items[0]->const_item();
+  /*
+    If the first item is a date/time function then its result should be
+    compared as int
+  */
+  if (field)
+  {
+    /* Suppose we are comparing dates and some non-constant items are present. */
+    type[0]= INT_RESULT;
+    is_const= 0;
+  }
+  else
+    type[0]= items[0]->result_type();
 
-  for (i= 1 ; i < nitems ; i++)
+  for (i= 0; i < nitems ; i++)
   {
     if (!items[i]->const_item())
     {
-      type[0]= is_const ? items[i]->result_type() :
-                 item_cmp_type(type[0], items[i]->result_type());
+      Item_result result= field && items[i]->result_as_longlong() ?
+                            INT_RESULT : items[i]->result_type();
+      type[0]= is_const ? result : item_cmp_type(type[0], result);
       is_const= 0;
     }
     else if (is_const)
       type[0]= item_cmp_type(type[0], items[i]->result_type());
+    else if (field)
+      convert_constant_item(thd, field, &items[i]);
+  }
+
+  if (res == Item::FUNC_ITEM && field)
+  {
+    delete field;
+    my_free(buff, MYF(MY_WME));
   }
 }
 
@@ -929,31 +1041,9 @@ void Item_func_between::fix_length_and_dec()
   */
   if (!args[0] || !args[1] || !args[2])
     return;
-  agg_cmp_type(&cmp_type, args, 3);
-  if (cmp_type == STRING_RESULT &&
-      agg_arg_charsets(cmp_collation, args, 3, MY_COLL_CMP_CONV))
-    return;
-
-  /*
-    Make a special case of compare with date/time and longlong fields.
-    They are compared as integers, so for const item this time-consuming
-    conversion can be done only once, not for every single comparison
-  */
-  if (args[0]->type() == FIELD_ITEM)
-  {
-    Field *field=((Item_field*) args[0])->field;
-    if (field->can_be_compared_as_longlong())
-    {
-      /*
-        The following can't be recoded with || as convert_constant_item
-        changes the argument
-      */
-      if (convert_constant_item(thd, field,&args[1]))
-	cmp_type=INT_RESULT;			// Works for all types.
-      if (convert_constant_item(thd, field,&args[2]))
-	cmp_type=INT_RESULT;			// Works for all types.
-    }
-  }
+  agg_cmp_type(thd, &cmp_type, args, 3);
+  if (cmp_type == STRING_RESULT)
+      agg_arg_charsets(cmp_collation, args, 3, MY_COLL_CMP_CONV);
 }
 
 
@@ -1477,7 +1567,7 @@ void Item_func_case::fix_length_and_dec()
     for (nagg= 0; nagg < ncases/2 ; nagg++)
       agg[nagg+1]= args[nagg*2];
     nagg++;
-    agg_cmp_type(&cmp_type, agg, nagg);
+    agg_cmp_type(current_thd, &cmp_type, agg, nagg);
     if ((cmp_type == STRING_RESULT) &&
         agg_arg_charsets(cmp_collation, agg, nagg, MY_COLL_CMP_CONV))
     return;
@@ -1958,7 +2048,7 @@ void Item_func_in::fix_length_and_dec()
   uint const_itm= 1;
   THD *thd= current_thd;
   
-  agg_cmp_type(&cmp_type, args, arg_count);
+  agg_cmp_type(thd, &cmp_type, args, arg_count);
 
   if (cmp_type == STRING_RESULT &&
       agg_arg_charsets(cmp_collation, args, arg_count, MY_COLL_CMP_CONV))