List:Commits« Previous MessageNext Message »
From:Martin Hansson Date:June 3 2010 8:44am
Subject:bzr commit into mysql-5.1-bugteam branch (martin.hansson:3388) Bug#53859
View as plain text  
#At file:///data0/martin/bzr/bug53859/5.1bt-commit/ based on revid:martin.hansson@stripped

 3388 Martin Hansson	2010-06-03
      Bug#53859: Valgrind: opt_sum_query(TABLE_LIST*, List<Item>&,
      Item*) at opt_sum.cc:305
      
      Queries applying MIN/MAX functions to indexed columns are
      optimized to read directly from the index if all key parts
      of the index preceding the aggregated key part are bound to
      constants by the WHERE clause. A prefix length is also
      produced, equal to the total length of the bound key
      parts. If the aggregated column itself is bound to a
      constant, however, it is also included in the prefix.
      
      Such full search keys are read as closed intervals for
      reasons beyond the scope of this bug. However, the procedure
      missed one case where a key part meant for use as range
      endpoint was being overwritten with a NULL value destined
      for equality checking. In this case the key part was
      overwritten but the range flag remained, causing open
      interval reading to be performed.
      
      Bug was fixed by adding more stringent checking to the
      search key building procedure (matching_cond) and never
      allow overwrites of range predicates with non-range
      predicates.
      
      An assertion was added to make sure open intervals are never
      used with full search keys.

    modified:
      mysql-test/r/group_min_max.result
      mysql-test/t/group_min_max.test
      sql/opt_sum.cc
=== modified file 'mysql-test/r/group_min_max.result'
--- a/mysql-test/r/group_min_max.result	2010-03-19 08:23:44 +0000
+++ b/mysql-test/r/group_min_max.result	2010-06-03 08:43:56 +0000
@@ -2767,4 +2767,17 @@ SELECT MIN( a ) FROM t1 WHERE a IS NULL;
 MIN( a )
 NULL
 DROP TABLE t1;
+#
+# Bug#53859: Valgrind: opt_sum_query(TABLE_LIST*, List<Item>&, Item*) at
+# opt_sum.cc:305
+#
+CREATE TABLE t1 ( a INT, KEY (a) );
+INSERT INTO t1 VALUES (1), (2), (3);
+SELECT MIN( a ) AS min_a
+FROM t1
+WHERE a > 1 AND a IS NULL
+ORDER BY min_a;
+min_a
+NULL
+DROP TABLE t1;
 End of 5.1 tests

=== modified file 'mysql-test/t/group_min_max.test'
--- a/mysql-test/t/group_min_max.test	2010-03-19 08:23:44 +0000
+++ b/mysql-test/t/group_min_max.test	2010-06-03 08:43:56 +0000
@@ -1085,4 +1085,19 @@ INSERT INTO t1 VALUES (1), (2), (3);
 --source include/min_null_cond.inc
 DROP TABLE t1;
 
+--echo #
+--echo # Bug#53859: Valgrind: opt_sum_query(TABLE_LIST*, List<Item>&, Item*) at
+--echo # opt_sum.cc:305
+--echo #
+CREATE TABLE t1 ( a INT, KEY (a) );
+INSERT INTO t1 VALUES (1), (2), (3); 
+
+SELECT MIN( a ) AS min_a
+FROM t1
+WHERE a > 1 AND a IS NULL
+ORDER BY min_a;
+
+DROP TABLE t1;
+
+
 --echo End of 5.1 tests

=== modified file 'sql/opt_sum.cc'
--- a/sql/opt_sum.cc	2010-05-27 13:03:08 +0000
+++ b/sql/opt_sum.cc	2010-06-03 08:43:56 +0000
@@ -142,7 +142,10 @@ static int get_index_min_value(TABLE *ta
         1) We have only MIN() and the argument column is nullable, or
         2) there is a > predicate on it, nullability is irrelevant.
         We need to scan the next bigger record first.
+        Open interval is not used if the search key involves the last keypart,
+        and it would not work.
       */
+      DBUG_ASSERT(prefix_len < ref->key_length);
       error= table->file->index_read_map(table->record[0],
                                          ref->key_buff,
                                          make_prev_keypart_map(ref->key_parts),
@@ -603,18 +606,19 @@ static bool matching_cond(bool max_fl, T
                           key_part_map *key_part_used, uint *range_fl,
                           uint *prefix_len)
 {
+  DBUG_ENTER("matching_cond");
   if (!cond)
-    return 1;
+    DBUG_RETURN(TRUE);
   Field *field= field_part->field;
   if (!(cond->used_tables() & field->table->map))
   {
     /* Condition doesn't restrict the used table */
-    return 1;
+    DBUG_RETURN(TRUE);
   }
   if (cond->type() == Item::COND_ITEM)
   {
     if (((Item_cond*) cond)->functype() == Item_func::COND_OR_FUNC)
-      return 0;
+      DBUG_RETURN(FALSE);
 
     /* AND */
     List_iterator_fast<Item> li(*((Item_cond*) cond)->argument_list());
@@ -623,13 +627,13 @@ static bool matching_cond(bool max_fl, T
     {
       if (!matching_cond(max_fl, ref, keyinfo, field_part, item,
                          key_part_used, range_fl, prefix_len))
-        return 0;
+        DBUG_RETURN(FALSE);
     }
-    return 1;
+    DBUG_RETURN(TRUE);
   }
 
   if (cond->type() != Item::FUNC_ITEM)
-    return 0;                                 // Not operator, can't optimize
+    DBUG_RETURN(FALSE);                                 // Not operator, can't optimize
 
   bool eq_type= 0;                            // =, <=> or IS NULL
   bool is_null_safe_eq= FALSE;                // The operator is NULL safe, e.g. <=> 
@@ -663,7 +667,7 @@ static bool matching_cond(bool max_fl, T
     eq_type= 1;
     break;
   default:
-    return 0;                                        // Can't optimize function
+    DBUG_RETURN(FALSE);                                        // Can't optimize function
   }
   
   Item *args[3];
@@ -671,11 +675,11 @@ static bool matching_cond(bool max_fl, T
 
   /* Test if this is a comparison of a field and constant */
   if (!simple_pred((Item_func*) cond, args, &inv))
-    return 0;
+    DBUG_RETURN(FALSE);
 
   if (!is_null_safe_eq && !is_null &&
       (args[1]->is_null() || (between && args[2]->is_null())))
-    return FALSE;
+    DBUG_RETURN(FALSE);
 
   if (inv && !eq_type)
     less_fl= 1-less_fl;                         // Convert '<' -> '>' (etc)
@@ -687,14 +691,14 @@ static bool matching_cond(bool max_fl, T
 
   {
     if (part > field_part)
-      return 0;                     // Field is beyond the tested parts
+      DBUG_RETURN(FALSE);                     // Field is beyond the tested parts
     if (part->field->eq(((Item_field*) args[0])->field))
       break;                        // Found a part of the key for the field
   }
 
   bool is_field_part= part == field_part;
   if (!(is_field_part || eq_type))
-    return 0;
+    DBUG_RETURN(FALSE);
 
   key_part_map org_key_part_used= *key_part_used;
   if (eq_type || between || max_fl == less_fl)
@@ -714,6 +718,17 @@ static bool matching_cond(bool max_fl, T
     *key_part_used|= (key_part_map) 1 << (part - keyinfo->key_part);
   }
 
+  if (org_key_part_used == *key_part_used &&
+    /*
+      The current search key is not being extended with a new key part.  This
+      means that the a condition is added a key part for which there was a
+      previous condition. We can only overwrite such key parts in some special
+      cases, e.g. a > 2 AND a > 1 (here range_fl must be set to something). In
+      all other cases the WHERE condition is always false anyway.
+    */
+      (eq_type || *range_fl == 0))
+      DBUG_RETURN(FALSE);
+
   if (org_key_part_used != *key_part_used ||
       (is_field_part && 
        (between || eq_type || max_fl == less_fl) && !cond->val_int()))
@@ -759,11 +774,11 @@ static bool matching_cond(bool max_fl, T
   {
     if ((!is_null && !cond->val_int()) ||
         (is_null && !test(part->field->is_null())))
-     return 0;                       // Impossible test
+     DBUG_RETURN(FALSE);                       // Impossible test
   }
   else if (is_field_part)
     *range_fl&= ~(max_fl ? NO_MIN_RANGE : NO_MAX_RANGE);
-  return 1;  
+  DBUG_RETURN(TRUE);  
 }
 
 


Attachment: [text/bzr-bundle] bzr/martin.hansson@sun.com-20100603084356-2lotlh70be7vijre.bundle
Thread
bzr commit into mysql-5.1-bugteam branch (martin.hansson:3388) Bug#53859Martin Hansson3 Jun