bzr commit into mysql-5.1-bugteam branch (martin.hansson:3192) Bug#47925

[Martin Hansson]

#At file:///data0/martin/bzr/bug47925/5.1bt/ based on revid:dao-gang.qu@stripped

 3192 Martin Hansson	2009-10-23
      Bug#47925: regression of range optimizer and date comparison in 5.1.39!
      
      Intermediate patch for discussion.

    modified:
      sql/item.cc
      sql/item.h
      sql/item_cmpfunc.cc
      sql/item_cmpfunc.h
      sql/opt_range.cc
=== modified file 'sql/item.cc'
--- a/sql/item.cc	2009-10-13 04:43:27 +0000
+++ b/sql/item.cc	2009-10-23 11:36:05 +0000
@@ -6866,72 +6866,67 @@ void resolve_const_item(THD *thd, Item *
 }
 
 /**
-  Compare the value stored in field, with the original item.
+  Compare the value stored in field with the expression from the query.
 
-  @param field   field which the item is converted and stored in
-  @param item    original item
+  @param field   Field which the Item is stored in after conversion
+  @param item    Original expression from query
 
-  @return Return an integer greater than, equal to, or less than 0 if
-          the value stored in the field is greater than,  equal to,
-          or less than the original item
+  @return Returns an integer greater than, equal to, or less than 0 if
+          the value stored in the field is greater than, equal to,
+          or less than the original Item. A 0 may also be returned if 
+          out of memory.          
 
   @note We only use this on the range optimizer/partition pruning,
         because in some cases we can't store the value in the field
         without some precision/character loss.
 */
 
-int stored_field_cmp_to_item(Field *field, Item *item)
+int stored_field_cmp_to_item(THD *thd, Field *field, Item *item)
 {
-
   Item_result res_type=item_cmp_type(field->result_type(),
 				     item->result_type());
   if (res_type == STRING_RESULT)
   {
     char item_buff[MAX_FIELD_WIDTH];
     char field_buff[MAX_FIELD_WIDTH];
-    String item_tmp(item_buff,sizeof(item_buff),&my_charset_bin),*item_result;
+    
+    String item_tmp(item_buff,sizeof(item_buff),&my_charset_bin);
     String field_tmp(field_buff,sizeof(field_buff),&my_charset_bin);
-    enum_field_types field_type;
-    item_result=item->val_str(&item_tmp);
+    String *item_result= item->val_str(&item_tmp);
+    /*
+      Some implementations of Item::val_str(String*) actually modify
+      the field Item::null_value, hence we can't check it earlier.
+    */
     if (item->null_value)
       return 0;
-    field->val_str(&field_tmp);
+    String *field_result= field->val_str(&field_tmp);
 
-    /*
-      If comparing DATE with DATETIME, append the time-part to the DATE.
-      So that the strings are equally formatted.
-      A DATE converted to string is 10 (MAX_DATE_WIDTH) characters, 
-      and a DATETIME converted to string is 19 (MAX_DATETIME_WIDTH) characters.
-    */
-    field_type= field->type();
-    uint32 item_length= item_result->length();
-    if (field_type == MYSQL_TYPE_DATE &&
-        item_length == MAX_DATETIME_WIDTH)
-      field_tmp.append(" 00:00:00");
-    else if (field_type == MYSQL_TYPE_DATETIME)
+    if (field->type() == MYSQL_TYPE_DATE ||
+        field->type() == MYSQL_TYPE_DATETIME)
     {
-      if (item_length == MAX_DATE_WIDTH)
-        item_result->append(" 00:00:00");
-      else if (item_length > MAX_DATETIME_WIDTH)
-      {
-        /*
-          We don't store microsecond part of DATETIME in field
-          but item_result contains it. As we compare DATETIMEs as strings
-          we must trim trailing 0's in item_result's microsecond part
-          to ensure "YYYY-MM-DD HH:MM:SS" == "YYYY-MM-DD HH:MM:SS.0000"
-        */
-        char *end= (char *) item_result->ptr() + item_length - 1;
-        /* Trim trailing 0's */
-        while (*end == '0')
-          end--;
-        /* Trim '.' if no microseconds */
-        if (*end == '.')
-          end--;
-        DBUG_ASSERT(end - item_result->ptr() + 1 >= MAX_DATETIME_WIDTH);
-        item_result->length(end - item_result->ptr() + 1);
-      }
+      enum_mysql_timestamp_type type= MYSQL_TIMESTAMP_ERROR;
+
+      if (field->type() == MYSQL_TYPE_DATE)
+        type= MYSQL_TIMESTAMP_DATE;
+
+      if (field->type() == MYSQL_TYPE_DATETIME)
+        type= MYSQL_TIMESTAMP_DATETIME;
+        
+      bool ignored_error;
+      const char *field_name= field->field_name;
+      ulonglong field_long_value= 
+        get_date_from_str(thd, field_result, type, field_name, &ignored_error);
+
+      ulonglong item_long_value= 
+        get_date_from_str(thd, item_result, type, field_name, &ignored_error);
+
+      if (field_long_value > item_long_value)
+        return 1;
+      if (field_long_value < item_long_value)
+        return -1;
+      return 0;
     }
-    return stringcmp(&field_tmp,item_result);
+    return stringcmp(field_result, item_result);
   }
   if (res_type == INT_RESULT)
     return 0;					// Both are of type int

=== modified file 'sql/item.h'
--- a/sql/item.h	2009-08-28 10:55:59 +0000
+++ b/sql/item.h	2009-10-23 11:36:05 +0000
@@ -3125,4 +3125,4 @@ void mark_select_range_as_dependent(THD 
 extern Cached_item *new_Cached_item(THD *thd, Item *item);
 extern Item_result item_cmp_type(Item_result a,Item_result b);
 extern void resolve_const_item(THD *thd, Item **ref, Item *cmp_item);
-extern int stored_field_cmp_to_item(Field *field, Item *item);
+extern int stored_field_cmp_to_item(THD *thd, Field *field, Item *item);

=== modified file 'sql/item_cmpfunc.cc'
--- a/sql/item_cmpfunc.cc	2009-10-05 05:27:36 +0000
+++ b/sql/item_cmpfunc.cc	2009-10-23 11:36:05 +0000
@@ -659,9 +659,8 @@ int Arg_comparator::set_compare_func(Ite
     converted value. 0 on error and on zero-dates -- check 'failure'
 */
 
-static ulonglong
-get_date_from_str(THD *thd, String *str, timestamp_type warn_type,
-                  char *warn_name, bool *error_arg)
+ulonglong get_date_from_str(THD *thd, String *str, timestamp_type warn_type,
+                            const char *warn_name, bool *error_arg)
 {
   ulonglong value= 0;
   int error;

=== modified file 'sql/item_cmpfunc.h'
--- a/sql/item_cmpfunc.h	2008-12-12 11:13:11 +0000
+++ b/sql/item_cmpfunc.h	2009-10-23 11:36:05 +0000
@@ -1721,3 +1721,6 @@ inline Item *and_conds(Item *a, Item *b)
 }
 
 Item *and_expressions(Item *a, Item *b, Item **org_item);
+
+ulonglong get_date_from_str(THD *thd, String *str, timestamp_type warn_type,
+                            const char *warn_name, bool *error_arg)

=== modified file 'sql/opt_range.cc'
--- a/sql/opt_range.cc	2009-10-16 20:19:51 +0000
+++ b/sql/opt_range.cc	2009-10-23 11:36:05 +0000
@@ -5947,7 +5947,7 @@ get_mm_leaf(RANGE_OPT_PARAM *param, COND
 
   switch (type) {
   case Item_func::LT_FUNC:
-    if (stored_field_cmp_to_item(field,value) == 0)
+    if (stored_field_cmp_to_item(param->thd, field, value) == 0)
       tree->max_flag=NEAR_MAX;
     /* fall through */
   case Item_func::LE_FUNC:
@@ -5962,14 +5962,14 @@ get_mm_leaf(RANGE_OPT_PARAM *param, COND
   case Item_func::GT_FUNC:
     /* Don't use open ranges for partial key_segments */
     if ((!(key_part->flag & HA_PART_KEY_SEG)) &&
-        (stored_field_cmp_to_item(field, value) <= 0))
+        (stored_field_cmp_to_item(param->thd, field, value) <= 0))
       tree->min_flag=NEAR_MIN;
     tree->max_flag= NO_MAX_RANGE;
     break;
   case Item_func::GE_FUNC:
     /* Don't use open ranges for partial key_segments */
     if ((!(key_part->flag & HA_PART_KEY_SEG)) &&
-        (stored_field_cmp_to_item(field,value) < 0))
+        (stored_field_cmp_to_item(param->thd, field, value) < 0))
       tree->min_flag= NEAR_MIN;
     tree->max_flag=NO_MAX_RANGE;
     break;


Attachment: [text/bzr-bundle] bzr/[e-mail]