List:Commits« Previous MessageNext Message »
From:Ole John Aske Date:February 7 2011 9:47am
Subject:bzr commit into mysql-trunk branch (ole.john.aske:3601) Bug#59308
View as plain text  
#At file:///net/fimafeng09/export/home/tmp/oleja/mysql/mysql-trunk/ based on revid:vinay.fisrekar@stripped

 3601 Ole John Aske	2011-02-07 [merge]
      Merge of fix for bug#59308 from mysql-5.5 -> mysql-trunk

    modified:
      mysql-test/include/order_by.inc
      mysql-test/r/order_by_all.result
      mysql-test/r/order_by_icp_mrr.result
      mysql-test/r/order_by_none.result
      sql/sql_select.cc
=== modified file 'mysql-test/include/order_by.inc'
--- a/mysql-test/include/order_by.inc	2011-02-02 13:41:10 +0000
+++ b/mysql-test/include/order_by.inc	2011-02-07 09:46:53 +0000
@@ -1690,6 +1690,23 @@ LIMIT 2;
 DROP TABLE t1, t2;
 
 
+--echo #
+--echo # Bug #59110: Memory leak of QUICK_SELECT_I allocated memory 
+--echo #  and
+--echo # Bug #59308: Incorrect result for 
+--echo               SELECT DISTINCT <col>... ORDER BY <col> DESC 
+--echo
+--echo # Use Valgrind to detect #59110!
+--echo #
+
+CREATE TABLE t1 (a INT,KEY (a));
+INSERT INTO t1 VALUES (1),(2),(3),(4),(5),(6),(7),(8),(9),(10);
+
+EXPLAIN SELECT DISTINCT a,1 FROM t1 WHERE a <> 1 ORDER BY a DESC;
+SELECT DISTINCT a,1 FROM t1 WHERE a <> 1 ORDER BY a DESC;
+
+DROP TABLE t1;
+
 --echo End of 5.1 tests
 
 

=== modified file 'mysql-test/r/order_by_all.result'
--- a/mysql-test/r/order_by_all.result	2011-02-02 13:41:10 +0000
+++ b/mysql-test/r/order_by_all.result	2011-02-07 09:46:53 +0000
@@ -2524,6 +2524,31 @@ id	select_type	table	type	possible_keys	
 1	SIMPLE	t1	index	NULL	a	8	NULL	10	Using index; Using temporary; Using filesort
 1	SIMPLE	t2	eq_ref	PRIMARY	PRIMARY	4	test.t1.b	1	Using where
 DROP TABLE t1, t2;
+#
+# Bug #59110: Memory leak of QUICK_SELECT_I allocated memory 
+#  and
+# Bug #59308: Incorrect result for 
+SELECT DISTINCT <col>... ORDER BY <col> DESC 
+
+# Use Valgrind to detect #59110!
+#
+CREATE TABLE t1 (a INT,KEY (a));
+INSERT INTO t1 VALUES (1),(2),(3),(4),(5),(6),(7),(8),(9),(10);
+EXPLAIN SELECT DISTINCT a,1 FROM t1 WHERE a <> 1 ORDER BY a DESC;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t1	index	a	a	5	NULL	10	Using where; Using index; Using filesort
+SELECT DISTINCT a,1 FROM t1 WHERE a <> 1 ORDER BY a DESC;
+a	1
+10	1
+9	1
+8	1
+7	1
+6	1
+5	1
+4	1
+3	1
+2	1
+DROP TABLE t1;
 End of 5.1 tests
 #
 # Bug #38745: MySQL 5.1 optimizer uses filesort for ORDER BY

=== modified file 'mysql-test/r/order_by_icp_mrr.result'
--- a/mysql-test/r/order_by_icp_mrr.result	2011-02-02 13:41:10 +0000
+++ b/mysql-test/r/order_by_icp_mrr.result	2011-02-07 09:46:53 +0000
@@ -2524,6 +2524,31 @@ id	select_type	table	type	possible_keys	
 1	SIMPLE	t1	index	NULL	a	8	NULL	10	Using index; Using temporary; Using filesort
 1	SIMPLE	t2	eq_ref	PRIMARY	PRIMARY	4	test.t1.b	1	Using where
 DROP TABLE t1, t2;
+#
+# Bug #59110: Memory leak of QUICK_SELECT_I allocated memory 
+#  and
+# Bug #59308: Incorrect result for 
+SELECT DISTINCT <col>... ORDER BY <col> DESC 
+
+# Use Valgrind to detect #59110!
+#
+CREATE TABLE t1 (a INT,KEY (a));
+INSERT INTO t1 VALUES (1),(2),(3),(4),(5),(6),(7),(8),(9),(10);
+EXPLAIN SELECT DISTINCT a,1 FROM t1 WHERE a <> 1 ORDER BY a DESC;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t1	index	a	a	5	NULL	10	Using where; Using index; Using filesort
+SELECT DISTINCT a,1 FROM t1 WHERE a <> 1 ORDER BY a DESC;
+a	1
+10	1
+9	1
+8	1
+7	1
+6	1
+5	1
+4	1
+3	1
+2	1
+DROP TABLE t1;
 End of 5.1 tests
 #
 # Bug #38745: MySQL 5.1 optimizer uses filesort for ORDER BY

=== modified file 'mysql-test/r/order_by_none.result'
--- a/mysql-test/r/order_by_none.result	2011-02-02 13:41:10 +0000
+++ b/mysql-test/r/order_by_none.result	2011-02-07 09:46:53 +0000
@@ -2523,6 +2523,31 @@ id	select_type	table	type	possible_keys	
 1	SIMPLE	t1	index	NULL	a	8	NULL	10	Using index; Using temporary; Using filesort
 1	SIMPLE	t2	eq_ref	PRIMARY	PRIMARY	4	test.t1.b	1	Using where
 DROP TABLE t1, t2;
+#
+# Bug #59110: Memory leak of QUICK_SELECT_I allocated memory 
+#  and
+# Bug #59308: Incorrect result for 
+SELECT DISTINCT <col>... ORDER BY <col> DESC 
+
+# Use Valgrind to detect #59110!
+#
+CREATE TABLE t1 (a INT,KEY (a));
+INSERT INTO t1 VALUES (1),(2),(3),(4),(5),(6),(7),(8),(9),(10);
+EXPLAIN SELECT DISTINCT a,1 FROM t1 WHERE a <> 1 ORDER BY a DESC;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t1	index	a	a	5	NULL	10	Using where; Using index; Using filesort
+SELECT DISTINCT a,1 FROM t1 WHERE a <> 1 ORDER BY a DESC;
+a	1
+10	1
+9	1
+8	1
+7	1
+6	1
+5	1
+4	1
+3	1
+2	1
+DROP TABLE t1;
 End of 5.1 tests
 #
 # Bug #38745: MySQL 5.1 optimizer uses filesort for ORDER BY

=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc	2011-02-04 12:21:31 +0000
+++ b/sql/sql_select.cc	2011-02-07 09:46:53 +0000
@@ -19897,11 +19897,12 @@ test_if_skip_sort_order(JOIN_TAB *tab,OR
 {
   int ref_key;
   uint ref_key_parts;
-  int order_direction;
+  int order_direction= 0;
   uint used_key_parts;
   TABLE *table=tab->table;
   SQL_SELECT *select=tab->select;
   QUICK_SELECT_I *save_quick= 0;
+  int best_key= -1;
   Item *orig_select_cond= 0;
   bool orig_select_cond_saved= false;
   bool changed_key= false;
@@ -20020,6 +20021,7 @@ test_if_skip_sort_order(JOIN_TAB *tab,OR
           key_map new_ref_key_map;  // Force the creation of quick select
           new_ref_key_map.set_bit(new_ref_key); // only for new_ref_key.
 
+          select->quick= 0;
           if (select->test_quick_select(tab->join->thd, new_ref_key_map, 0,
                                         (tab->join->select_options &
                                          OPTION_FOUND_ROWS) ?
@@ -20043,7 +20045,6 @@ test_if_skip_sort_order(JOIN_TAB *tab,OR
     uint best_key_parts= 0;
     uint saved_best_key_parts= 0;
     int best_key_direction= 0;
-    int best_key= -1;
     JOIN *join= tab->join;
     ha_rows table_records= table->file->stats.records;
 
@@ -20067,77 +20068,16 @@ test_if_skip_sort_order(JOIN_TAB *tab,OR
 
     if (best_key >= 0)
     {
-      bool quick_created= FALSE;
       if (table->quick_keys.is_set(best_key) && best_key != ref_key)
       {
         key_map map;           // Force the creation of quick select
         map.set_bit(best_key); // only best_key.
-        quick_created=         
-          select->test_quick_select(join->thd, map, 0,
-                                    join->select_options & OPTION_FOUND_ROWS ?
-                                    HA_POS_ERROR :
-                                    join->unit->select_limit_cnt,
-                                    TRUE, FALSE) > 0;
-      }
-      if (!no_changes)
-      {
-        /* 
-           If ref_key used index tree reading only ('Using index' in EXPLAIN),
-           and best_key doesn't, then revert the decision.
-        */
-        if (!table->covering_keys.is_set(best_key))
-          table->set_keyread(FALSE);
-        if (!quick_created)
-	{
-          tab->index= best_key;
-          tab->read_first_record= best_key_direction > 0 ?
-                                  join_read_first:join_read_last;
-          tab->type=JT_NEXT;           // Read with index_first(), index_next()
-          if (select && select->quick)
-          {
-            delete select->quick;
-            select->quick= 0;
-          }
-          if (table->covering_keys.is_set(best_key))
-            table->set_keyread(TRUE);
-          if (tab->pre_idx_push_select_cond)
-          {
-            tab->set_cond(tab->pre_idx_push_select_cond, __LINE__);
-            /*
-              orig_select_cond is a part of pre_idx_push_select_cond,
-              no need to restore it.
-            */
-            orig_select_cond= 0;
-            orig_select_cond_saved= false;
-          }
-          table->file->ha_index_or_rnd_end();
-          if (join->select_options & SELECT_DESCRIBE)
-          {
-            tab->ref.key= -1;
-            tab->ref.key_parts= 0;
-            if (select_limit < table_records) 
-              tab->limit= select_limit;
-          }
-        }
-        else if (tab->type != JT_ALL)
-        {
-          /*
-            We're about to use a quick access to the table.
-            We need to change the access method so as the quick access
-            method is actually used.
-          */
-          DBUG_ASSERT(tab->select->quick);
-          tab->type=JT_ALL;
-          tab->use_quick=QS_RANGE;
-          tab->ref.key= -1;
-          tab->ref.key_parts=0;		// Don't use ref key.
-          tab->read_first_record= join_init_read_record;
-          if (tab->is_using_loose_index_scan())
-            join->tmp_table_param.precomputed_group_by= TRUE;
-          /*
-            TODO: update the number of records in join->best_positions[tablenr]
-          */
-        }
+        select->quick= 0;
+        select->test_quick_select(join->thd, map, 0,
+                                  join->select_options & OPTION_FOUND_ROWS ?
+                                  HA_POS_ERROR :
+                                  join->unit->select_limit_cnt,
+                                  TRUE, FALSE);
       }
       order_direction= best_key_direction;
       /*
@@ -20155,6 +20095,8 @@ test_if_skip_sort_order(JOIN_TAB *tab,OR
   } 
 
 check_reverse_order:                  
+  DBUG_ASSERT(order_direction != 0);
+
   if (order_direction == -1)		// If ORDER BY ... DESC
   {
     if (select && select->quick)
@@ -20163,9 +20105,10 @@ check_reverse_order:                  
 	Don't reverse the sort order, if it's already done.
         (In some cases test_if_order_by_key() can be called multiple times
       */
-      if (!select->quick->reverse_sorted())
+      if (select->quick->reverse_sorted())
+        goto skipped_filesort;
+      else
       {
-        QUICK_SELECT_I *tmp;
         int quick_type= select->quick->get_type();
         if (quick_type == QUICK_SELECT_I::QS_TYPE_INDEX_MERGE ||
             quick_type == QUICK_SELECT_I::QS_TYPE_ROR_INTERSECT ||
@@ -20173,36 +20116,128 @@ check_reverse_order:                  
             quick_type == QUICK_SELECT_I::QS_TYPE_GROUP_MIN_MAX)
         {
           tab->limit= 0;
-          select->quick= save_quick;
           goto use_filesort;                   // Use filesort
         }
-            
-        /* ORDER BY range_key DESC */
-	tmp= select->quick->make_reverse(used_key_parts);
-	if (!tmp)
-	{
-          select->quick= save_quick;
-          tab->limit= 0;
-	  goto use_filesort;		// Reverse sort not supported
-	}
-	select->set_quick(tmp);
       }
     }
-    else if (tab->type != JT_NEXT && tab->type != JT_REF_OR_NULL &&
-             tab->ref.key >= 0 && tab->ref.key_parts <= used_key_parts)
+  }
+
+  /*
+    Update query plan with access pattern for doing 
+    ordered access according to what we have decided
+    above.
+  */
+  if (!no_changes) // We are allowed to update QEP
+  {
+    if (best_key >= 0)
     {
-      /*
-	SELECT * FROM t1 WHERE a=1 ORDER BY a DESC,b DESC
+      bool quick_created= 
+        (select && select->quick && select->quick!=save_quick);
 
-	Use a traversal function that starts by reading the last row
-	with key part (A) and then traverse the index backwards.
+      /* 
+         If ref_key used index tree reading only ('Using index' in EXPLAIN),
+         and best_key doesn't, then revert the decision.
       */
-      tab->read_first_record= join_read_last_key;
-      tab->read_record.read_record= join_read_prev_same;
+      if (!table->covering_keys.is_set(best_key))
+        table->set_keyread(FALSE);
+      if (!quick_created)
+      {
+        if (select)                  // Throw any existing quick select
+          select->quick= 0;          // Cleanup either reset to save_quick,
+                                     // or 'delete save_quick'
+        tab->index= best_key;
+        tab->read_first_record= order_direction > 0 ?
+                                join_read_first:join_read_last;
+        tab->type=JT_NEXT;           // Read with index_first(), index_next()
+
+        if (table->covering_keys.is_set(best_key))
+          table->set_keyread(TRUE);
+        if (tab->pre_idx_push_select_cond)
+        {
+          tab->set_cond(tab->pre_idx_push_select_cond, __LINE__);
+          /*
+            orig_select_cond is a part of pre_idx_push_select_cond,
+            no need to restore it.
+          */
+          orig_select_cond= 0;
+          orig_select_cond_saved= false;
+        }
+        table->file->ha_index_or_rnd_end();
+        if (tab->join->select_options & SELECT_DESCRIBE)
+        {
+          tab->ref.key= -1;
+          tab->ref.key_parts= 0;
+          if (select_limit < table->file->stats.records) 
+            tab->limit= select_limit;
+        }
+      }
+      else if (tab->type != JT_ALL)
+      {
+        /*
+          We're about to use a quick access to the table.
+          We need to change the access method so as the quick access
+          method is actually used.
+        */
+        DBUG_ASSERT(tab->select->quick);
+        tab->type=JT_ALL;
+        tab->use_quick=QS_RANGE;
+        tab->ref.key= -1;
+        tab->ref.key_parts=0;		// Don't use ref key.
+        tab->read_first_record= join_init_read_record;
+        if (tab->is_using_loose_index_scan())
+          tab->join->tmp_table_param.precomputed_group_by= TRUE;
+        /*
+          TODO: update the number of records in join->best_positions[tablenr]
+        */
+      }
+    } // best_key >= 0
+
+    if (order_direction == -1)		// If ORDER BY ... DESC
+    {
+      if (select && select->quick)
+      {
+        /* ORDER BY range_key DESC */
+        QUICK_SELECT_I *tmp= select->quick->make_reverse(used_key_parts);
+        if (!tmp)
+        {
+          tab->limit= 0;
+          goto use_filesort;            // Reverse sort failed -> filesort
+        }
+        if (select->quick == save_quick)
+          save_quick= 0;                // make_reverse() consumed it
+        select->set_quick(tmp);
+      }
+      else if (tab->type != JT_NEXT && tab->type != JT_REF_OR_NULL &&
+               tab->ref.key >= 0 && tab->ref.key_parts <= used_key_parts)
+      {
+        /*
+          SELECT * FROM t1 WHERE a=1 ORDER BY a DESC,b DESC
+
+          Use a traversal function that starts by reading the last row
+          with key part (A) and then traverse the index backwards.
+        */
+        tab->read_first_record= join_read_last_key;
+        tab->read_record.read_record= join_read_prev_same;
+      }
     }
+    else if (select && select->quick)
+      select->quick->need_sorted_output();
+
+  } // QEP has been modified
+
+  /*
+    Cleanup:
+    We may have both a 'select->quick' and 'save_quick' (original)
+    at this point. Delete the one that we wan't use.
+  */
+
+skipped_filesort:
+  // Keep current (ordered) select->quick 
+  if (select && save_quick != select->quick)
+  {
+    delete save_quick;
+    save_quick= NULL;
   }
-  else if (select && select->quick)
-    select->quick->need_sorted_output();
   /*
     Restore condition only if we didn't chose index different to what we used
     for ICP.
@@ -20212,6 +20247,12 @@ check_reverse_order:                  
   DBUG_RETURN(1);
 
 use_filesort:
+  // Restore original save_quick
+  if (select && select->quick != save_quick)
+  {
+    delete select->quick;
+    select->quick= save_quick;
+  }
   if (orig_select_cond_saved)
     tab->set_cond(orig_select_cond, __LINE__);
   DBUG_RETURN(0);

No bundle (reason: revision is a merge).
Thread
bzr commit into mysql-trunk branch (ole.john.aske:3601) Bug#59308Ole John Aske7 Feb