List:Commits« Previous MessageNext Message »
From:eugene Date:May 7 2006 2:09pm
Subject:bk commit into 4.1 tree (evgen:1.2468) BUG#16377
View as plain text  
Below is the list of changes that have just been committed into a local
4.1 repository of evgen. When evgen does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet
  1.2468 06/05/07 16:09:07 evgen@stripped +8 -0
  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 
  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() and item_cmp_type() functions
  are 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 the first item is a field that can be compared
  as longlong or a function result of which can be compared as longlong. This
  means that this field or functions returns date/time. If so, it tries to coerce
  all constants to INT to make date/time comparison return correct result. In
  order to make coercion the field is needed. In the case when the first item 
  is a function, agg_cmp_type() constructs a temporary field which is deleted 
  when all constants are coerced.  Constants are converted by means of 
  convert_constant_item() item. If all of them are converted successfully the
  aggregated type is changed to INT_RESULT.
  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 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_func class. By
  default it returns FALSE. The TRUE value should be returned for these functions
  the result of which can be compared as int but their result type isn't 
  INT_RESULT. Such functions are all date/time functions.
  
  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.
  
  The val_int() method is implemented for the Item_date_typecast class. 
  
  The item_cmp_type(Item_result, Item_result) function is substituted by 2
  overloaded functions item_cmp_type(Item *, Item *) and 
  item_cmp_type(Item_result, Item *).
  These functions are checking whether the Item parameter is a function and
  call to result_as_longlong() returns TRUE. If so they substitute the result
  type of the item with INT_RESULT. This makes a result of date/time functions
  to be compared as ints.
  In addition, the item_cmp_type(Item *, Item *) function substitutes the result
  type only if another parameter is a constant. This function is used mostly in
  the Arg_comparator to find type to compare given items. The additional check 
  is done because all dates generated by functions are correct and zeros may be
  skipped only by users in user-supplied constants. Also this approach eases 
  comparison of date/time functions result with pure string functions that can
  return a date int the result. In this case they are compared as strings.
  
  

  sql/item.cc
    1.232 06/05/07 16:04:49 evgen@stripped +43 -8
    Fixed bug#16377: result of DATE/TIME functions were compared as strings which
    can lead to a wrong result.
    The item_cmp_type(Item_result, Item_result) function is substituted by 2
    overloaded functions item_cmp_type(Item *, Item *) and
    item_cmp_type(Item_result, Item *).
    These functions are checking whether the Item parameter is a function and
    call to result_as_longlong() returns TRUE. If so they substitute the result
    type of the item with INT_RESULT. This makes a result of date/time functions
    to be compared as ints.
    In addition, the item_cmp_type(Item *, Item *) function substitutes the result
    type only if another parameter is a constant. This function is used mostly in
    the Arg_comparator to find type to compare given items. The additional check
    is done because all dates generated by functions are correct and zeros may be
    skipped only by users in user-supplied constants. Also this approach eases
    comparison of date/time functions result with pure string functions that can
    return a date int the result. In this case they are compared as strings.
    
    The  field_is_equal_to_item() function now takes into account that field can be
date/time field and should be compared as int. Also, this function now uses
item_cmp_type(Item_result, Item *) to find the type for comparison.
    

  sql/item_cmpfunc.cc
    1.208 06/05/07 16:03:08 evgen@stripped +72 -53
    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 the first item is a field that can be compared
    as longlong or a function result of which can be compared as longlong. This
    means that this field or functions returns date/time. If so, it tries to coerce
    all constants to INT to make date/time comparison return correct result. In
    order to make coercion the field is needed. In the case when the first item
    is a function, agg_cmp_type() constructs a temporary field which is deleted
    when all constants are coerced.  Constants are converted by means of
    convert_constant_item() item. If all of them are converted successfully the
    aggregated type is changed to INT_RESULT.
    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 constants to int if possible. Now this is done by
    the agg_cmp_type() function.
    
    

  sql/item.h
    1.193 06/05/07 00:57:45 evgen@stripped +2 -1
    Fixed bug#16377: result of DATE/TIME functions were compared as strings which
    can lead to a wrong result.
    Added prototypes of item_cmp_func(Item_result, Item *) and item_cmp_type(Item *,Item
*).

  sql/item_cmpfunc.h
    1.114 06/05/07 00:56:30 evgen@stripped +4 -5
    Fixed bug#16377: result of DATE/TIME functions were compared as strings which
    can lead to a wrong result.
    All calls to item_cmp_type(Item_result, Item_result) are changed to call
item_cmp_type(Item *, Item *).

  sql/item_func.cc
    1.260 06/05/06 23:57:15 evgen@stripped +2 -2
    Fixed bug#16377: result of DATE/TIME functions were compared as strings which
    can lead to a wrong result.
    All calls to item_cmp_type(Item_result, Item_result) are changed to call
item_cmp_type(Item_result, Item*) instead.

  sql/item_func.h
    1.130 06/05/06 22:39:16 evgen@stripped +1 -0
    Fixed bug#16377: result of DATE/TIME functions were compared as strings which
    can lead to a wrong result.The new function result_as_longlong() is added to the
Item_func class. By
    default it returns FALSE. The TRUE value should be returned for these functions
    the result of which can be compared as int but their result type isn't 
    INT_RESULT. Such functions are all date/time functions.

  sql/item_timefunc.cc
    1.99 06/05/06 22:38:38 evgen@stripped +8 -0
    Fixed bug#16377: result of DATE/TIME functions were compared as strings which
    can lead to a wrong result.
    The val_int() method is implemented for the Item_date_typecast class.

  sql/item_timefunc.h
    1.57 06/05/06 22:36:13 evgen@stripped +9 -0
    Fixed bug#16377: result of DATE/TIME functions were compared as strings which
    can lead to a wrong resultThe 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.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	evgen
# Host:	moonbone.local
# Root:	/work/16377-bug-4.1-mysql

--- 1.231/sql/item.cc	2006-03-01 16:53:13 +03:00
+++ 1.232/sql/item.cc	2006-05-07 16:04:49 +04:00
@@ -2835,10 +2835,6 @@
   str->append(')');
 }
 
-/*
-  If item is a const function, calculate it and return a const item
-  The original item is freed if not returned
-*/
 
 Item_result item_cmp_type(Item_result a,Item_result b)
 {
@@ -2852,14 +2848,48 @@
 }
 
 
+Item_result item_cmp_type(Item_result a, Item *b)
+{
+  Item_result br;
+  if (b->type() == Item::FUNC_ITEM && ((Item_func
*)b)->result_as_longlong())
+    br= INT_RESULT;
+  else
+    br= b->result_type();
+
+  return item_cmp_type(a, br);
+}
+
+
+Item_result item_cmp_type(Item *a, Item *b)
+{
+  Item_result ar, br;
+  if (a->type() == Item::FUNC_ITEM && ((Item_func
*)a)->result_as_longlong() &&
+      b->const_item())
+    ar= INT_RESULT;
+  else
+    ar= a->result_type();
+  if (b->type() == Item::FUNC_ITEM && ((Item_func
*)b)->result_as_longlong() &&
+      a->const_item())
+    br= INT_RESULT;
+  else
+    br= b->result_type();
+
+  return item_cmp_type(ar, br);
+}
+
+
+/*
+  If item is a const function, calculate it and return a const item
+  The original item is freed if not returned
+*/
+
 void resolve_const_item(THD *thd, Item **ref, Item *comp_item)
 {
   Item *item= *ref;
   Item *new_item= NULL;
   if (item->basic_const_item())
     return;                                     // Can't be better
-  Item_result res_type=item_cmp_type(comp_item->result_type(),
-				     item->result_type());
+  Item_result res_type=item_cmp_type(comp_item, item);
   char *name=item->name;			// Alloced by sql_alloc
 
   if (res_type == STRING_RESULT)
@@ -2931,9 +2961,14 @@
 
 bool field_is_equal_to_item(Field *field,Item *item)
 {
+  Item_result fr;
+  /* Check for date/time fields which should be compared as ints */
+  if (field->can_be_compared_as_longlong())
+    fr= INT_RESULT;
+  else
+    fr= field->result_type();
 
-  Item_result res_type=item_cmp_type(field->result_type(),
-				     item->result_type());
+  Item_result res_type=item_cmp_type(fr, item);
   if (res_type == STRING_RESULT)
   {
     char item_buff[MAX_FIELD_WIDTH];

--- 1.192/sql/item.h	2005-10-13 00:27:47 +04:00
+++ 1.193/sql/item.h	2006-05-07 00:57:45 +04:00
@@ -1416,6 +1416,7 @@
 
 
 extern Item_buff *new_Item_buff(THD *thd, Item *item);
-extern Item_result item_cmp_type(Item_result a,Item_result b);
+extern Item_result item_cmp_type(Item_result a,Item *b);
+extern Item_result item_cmp_type(Item *a,Item *b);
 extern void resolve_const_item(THD *thd, Item **ref, Item *cmp_item);
 extern bool field_is_equal_to_item(Field *field,Item *item);

--- 1.207/sql/item_cmpfunc.cc	2006-04-21 10:47:53 +04:00
+++ 1.208/sql/item_cmpfunc.cc	2006-05-07 16:03:08 +04:00
@@ -58,12 +58,79 @@
   }
 }
 
-static void agg_cmp_type(Item_result *type, Item **items, uint nitems)
+
+/*
+  Convert a constant expression or string to an integer.
+  This is done when comparing DATE's of different formats and
+  also when comparing bigint to strings (in which case the string
+  is converted once to a bigint).
+
+  RESULT VALUES
+  0	Can't convert item
+  1	Item was replaced with an integer version of the item
+*/
+
+static bool convert_constant_item(THD *thd, Field *field, Item **item)
+{
+  if ((*item)->const_item())
+  {
+    if (!(*item)->save_in_field(field, 1) && !((*item)->null_value))
+    {
+      Item *tmp=new Item_int_with_ref(field->val_int(), *item);
+      if (tmp)
+        thd->change_item_tree(item, tmp);
+      return 1;					// Item was replaced
+    }
+  }
+  return 0;
+}
+
+
+static void agg_cmp_type(THD *thd, Item_result *type, Item **items, uint nitems)
 {
   uint i;
+  Item::Type res;
+  /*
+    result_as_longlong() returns TRUE only for DATE/TIME functions
+    which can't be longer that 20
+  */
+  char *buff;
+  uchar null;
+  Field *field= NULL;
+  bool all_constant= TRUE;
+
+  /* If the first argument is a FIELD_ITEM, pull out the field. */
+  if ((res= items[0]->real_item()->type()) == Item::FIELD_ITEM)
+  {
+    field= ((Item_field *)items[0]->real_item())->field;
+    field= field->can_be_compared_as_longlong()? field : NULL;
+  }
+  else if (res == Item::FUNC_ITEM &&
+           ((Item_func*)items[0])->result_as_longlong())
+  {
+    field= items[0]->tmp_table_field_from_field_type(0);
+    buff= my_malloc(field->max_length(), MYF(MY_WME));
+    field->move_field(buff, &null, 0);
+  }
+
   type[0]= items[0]->result_type();
   for (i=1 ; i < nitems ; i++)
-    type[0]= item_cmp_type(type[0], items[i]->result_type());
+  {
+    type[0]= item_cmp_type(type[0], items[i]);
+    if (field && !convert_constant_item(thd, field, &items[i]))
+      all_constant= FALSE;
+  }
+  /*
+    If we had a field that can be compared as a longlong, and all constant
+    items, then the aggregate result will be an INT_RESULT.
+  */
+  if (field && all_constant)
+    type[0]= INT_RESULT;
+  if (res == Item::FUNC_ITEM && field)
+  {
+    delete field;
+    my_free(buff, MYF(MY_WME));
+  }
 }
 
 static void my_coll_agg_error(DTCollation &c1, DTCollation &c2,
@@ -184,33 +251,6 @@
 }
 
 
-/*
-  Convert a constant expression or string to an integer.
-  This is done when comparing DATE's of different formats and
-  also when comparing bigint to strings (in which case the string
-  is converted once to a bigint).
-
-  RESULT VALUES
-  0	Can't convert item
-  1	Item was replaced with an integer version of the item
-*/
-
-static bool convert_constant_item(THD *thd, Field *field, Item **item)
-{
-  if ((*item)->const_item())
-  {
-    if (!(*item)->save_in_field(field, 1) && !((*item)->null_value))
-    {
-      Item *tmp=new Item_int_with_ref(field->val_int(), *item);
-      if (tmp)
-        thd->change_item_tree(item, tmp);
-      return 1;					// Item was replaced
-    }
-  }
-  return 0;
-}
-
-
 void Item_bool_func2::fix_length_and_dec()
 {
   max_length= 1;				     // Function returns 0 or 1
@@ -896,31 +936,10 @@
   */
   if (!args[0] || !args[1] || !args[2])
     return;
-  agg_cmp_type(&cmp_type, args, 3);
+  agg_cmp_type(thd, &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.
-    }
-  }
 }
 
 
@@ -1444,7 +1463,7 @@
     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;
@@ -1925,7 +1944,7 @@
   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))

--- 1.113/sql/item_cmpfunc.h	2006-01-14 04:55:01 +03:00
+++ 1.114/sql/item_cmpfunc.h	2006-05-07 00:56:30 +04:00
@@ -21,7 +21,8 @@
 #pragma interface			/* gcc class implementation */
 #endif
 
-extern Item_result item_cmp_type(Item_result a,Item_result b);
+extern Item_result item_cmp_type(Item_result a,Item *b);
+extern Item_result item_cmp_type(Item *a,Item *b);
 class Item_bool_func2;
 class Arg_comparator;
 
@@ -43,8 +44,7 @@
   int set_compare_func(Item_bool_func2 *owner, Item_result type);
   inline int set_compare_func(Item_bool_func2 *owner_arg)
   {
-    return set_compare_func(owner_arg, item_cmp_type((*a)->result_type(),
-						     (*b)->result_type()));
+    return set_compare_func(owner_arg, item_cmp_type((*a), (*b)));
   }
   inline int set_cmp_func(Item_bool_func2 *owner_arg,
 			  Item **a1, Item **a2,
@@ -57,8 +57,7 @@
   inline int set_cmp_func(Item_bool_func2 *owner_arg,
 			  Item **a1, Item **a2)
   {
-    return set_cmp_func(owner_arg, a1, a2, item_cmp_type((*a1)->result_type(),
-							 (*a2)->result_type()));
+    return set_cmp_func(owner_arg, a1, a2, item_cmp_type((*a1), (*a2)));
   }
   inline int compare() { return (this->*func)(); }
 

--- 1.259/sql/item_func.cc	2006-04-12 23:02:02 +04:00
+++ 1.260/sql/item_func.cc	2006-05-06 23:57:15 +04:00
@@ -1140,7 +1140,7 @@
       decimals=args[i]->decimals;
     if (!args[i]->maybe_null)
       maybe_null=0;
-    cmp_type=item_cmp_type(cmp_type,args[i]->result_type());
+    cmp_type=item_cmp_type(cmp_type,args[i]);
   }
   if (cmp_type == STRING_RESULT)
     agg_arg_charsets(collation, args, arg_count, MY_COLL_CMP_CONV);
@@ -1394,7 +1394,7 @@
   maybe_null=0; max_length=3;
   cmp_type= args[0]->result_type();
   for (uint i=1; i < arg_count ; i++)
-    cmp_type= item_cmp_type(cmp_type, args[i]->result_type());
+    cmp_type= item_cmp_type(cmp_type, args[i]);
   if (cmp_type == STRING_RESULT)
     agg_arg_charsets(cmp_collation, args, arg_count, MY_COLL_CMP_CONV);
 }

--- 1.129/sql/item_func.h	2006-02-07 12:49:52 +03:00
+++ 1.130/sql/item_func.h	2006-05-06 22:39:16 +04:00
@@ -130,6 +130,7 @@
   void print_op(String *str);
   void print_args(String *str, uint from);
   void fix_num_length_and_dec();
+  virtual const bool result_as_longlong() { return FALSE; }
   inline bool get_arg0_date(TIME *ltime, uint fuzzy_date)
   {
     return (null_value=args[0]->get_date(ltime, fuzzy_date));

--- 1.98/sql/item_timefunc.cc	2006-04-13 09:55:41 +04:00
+++ 1.99/sql/item_timefunc.cc	2006-05-06 22:38:38 +04:00
@@ -2358,6 +2358,14 @@
   return 0;
 }
 
+longlong Item_date_typecast::val_int()
+{
+  DBUG_ASSERT(fixed == 1);
+  TIME ltime;
+  if (args[0]->get_date(&ltime, TIME_FUZZY_DATE))
+    return 0;
+  return (longlong) (ltime.year * 10000L + ltime.month * 100 + ltime.day);
+}
 
 /*
   MAKEDATE(a,b) is a date function that creates a date value 

--- 1.56/sql/item_timefunc.h	2006-04-13 09:55:41 +04:00
+++ 1.57/sql/item_timefunc.h	2006-05-06 22:36:13 +04:00
@@ -339,6 +339,7 @@
   {
     return (new Field_date(maybe_null, name, t_arg, &my_charset_bin));
   }  
+  virtual const bool result_as_longlong() { return TRUE; }
 };
 
 
@@ -354,6 +355,7 @@
   {
     return (new Field_datetime(maybe_null, name, t_arg, &my_charset_bin));
   }
+  virtual const bool result_as_longlong() { return TRUE; }
 };
 
 
@@ -383,6 +385,7 @@
     TIME representation using UTC-SYSTEM or per-thread time zone.
   */
   virtual void store_now_in_TIME(TIME *now_time)=0;
+  virtual const bool result_as_longlong() { return TRUE; }
 };
 
 
@@ -588,6 +591,7 @@
   {
     return (new Field_time(maybe_null, name, t_arg, &my_charset_bin));
   }
+  virtual const bool result_as_longlong() { return TRUE; }
 };
 
 /*
@@ -715,6 +719,8 @@
     max_length= 10;
     maybe_null= 1;
   }
+  virtual const bool result_as_longlong() { return TRUE; }
+  longlong val_int();
 };
 
 
@@ -731,6 +737,7 @@
   {
     return (new Field_time(maybe_null, name, t_arg, &my_charset_bin));
   }
+  virtual const bool result_as_longlong() { return TRUE; }
 };
 
 
@@ -746,6 +753,7 @@
   {
     return (new Field_datetime(maybe_null, name, t_arg, &my_charset_bin));
   }
+  virtual const bool result_as_longlong() { return TRUE; }
 };
 
 class Item_func_makedate :public Item_str_func
@@ -764,6 +772,7 @@
   {
     return (new Field_date(maybe_null, name, t_arg, &my_charset_bin));
   }
+  virtual const bool result_as_longlong() { return TRUE; }
 };
 
 
Thread
bk commit into 4.1 tree (evgen:1.2468) BUG#16377eugene7 May