List:Commits« Previous MessageNext Message »
From:dlenev Date:September 24 2006 3:37pm
Subject:bk commit into 5.0 tree (dlenev:1.2273) BUG#22338
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of dlenev. When dlenev 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@stripped, 2006-09-24 17:36:59+04:00, dlenev@stripped +1 -0
  Proposed fix for bug#22338 "Valgrind warning: uninitialized variable in
  create_tmp_table()".
  
  The fix for bug 21787 "COUNT(*) + ORDER BY + LIMIT returns wrong result"
  introduced valgrind warnings which occured during execution of
  information_schema.test and sp-prelocking.test. There were no user
  visible effects.
  
  The latter fix made create_tmp_table() dependant on
  THD::lex::current_select value. Valgrind warnings occured when this
  function was executed and THD::lex::current_select member pointed
  to uninitialized SELECT_LEX instance.
  
  The proposed fix tries to remove this dependancy by moving some logic
  outside of create_tmp_table() function.
  
  Question for reviewer is marked by QQ.

  sql/sql_select.cc@stripped, 2006-09-24 17:36:57+04:00, dlenev@stripped +20 -14
    create_tmp_table():
      Moved code which is responsible for determining if optimization
      which pushes down LIMIT clause to temporary table creation is
      applicable out of this function.
      Such move made this function independant of THD::lex::current_select
      value and removed valgrind warnings which occured in cases when this
      member pointed to uninitialized SELECT_LEX object.
      This seems like a better solution than trying to force this pointer 
      always to point to relevant select because: 
      - In some cases when we use create_tmp_table() there are no relevant
        SELECT_LEX object (we use it just to create temporary table/object).
      - There is only one place in code where we call this funciton and
        where this optimization can be enabled. And in this place we
        already have some logic which tries to determine if it is applicable.

# 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:	dlenev
# Host:	mockturtle.local
# Root:	/home/dlenev/src/mysql-5.0-bg22338

--- 1.454/sql/sql_select.cc	2006-09-24 17:37:05 +04:00
+++ 1.455/sql/sql_select.cc	2006-09-24 17:37:05 +04:00
@@ -1134,17 +1134,29 @@ JOIN::optimize()
 
     tmp_table_param.hidden_field_count= (all_fields.elements -
 					 fields_list.elements);
+    ORDER *tmp_group= ((!simple_group && !procedure &&
+                        !(test_flags & TEST_NO_KEY_GROUP)) ? group_list :
+                                                             (ORDER*) 0);
+    /*
+      Pushing LIMIT to the temporary table creation is not applicable
+      when there is ORDER BY or GROUP BY or there is no GROUP BY, but
+      there are aggregate functions, because in all these cases we need
+      all result rows.
+
+      QQ: Should we enable this optimization if tmp_table_param.quick_group==0
+          as it was before this patch?
+    */
+    ha_rows tmp_rows_limit= ((order == 0 || skip_sort_order ||
+                             test(select_options & OPTION_BUFFER_RESULT)) &&
+                             !tmp_group && !sel->with_sum_func) ? select_limit
:
+                                                                  HA_POS_ERROR;
     if (!(exec_tmp_table1 =
 	  create_tmp_table(thd, &tmp_table_param, all_fields,
-			   ((!simple_group && !procedure &&
-			     !(test_flags & TEST_NO_KEY_GROUP)) ?
-			    group_list : (ORDER*) 0),
+                           tmp_group,
 			   group_list ? 0 : select_distinct,
 			   group_list && simple_group,
 			   select_options,
-			   (order == 0 || skip_sort_order || 
-                            test(select_options & OPTION_BUFFER_RESULT)) ? 
-                             select_limit : HA_POS_ERROR,
+                           tmp_rows_limit,
 			   (char *) "")))
       DBUG_RETURN(1);
 
@@ -9229,15 +9241,9 @@ create_tmp_table(THD *thd,TMP_TABLE_PARA
   /*
     Push the LIMIT clause to the temporary table creation, so that we
     materialize only up to 'rows_limit' records instead of all result records.
-    This optimization is not applicable when there is GROUP BY or there is
-    no GROUP BY, but there are aggregate functions, because both must be
-    computed for all result rows.
   */
-  if (!group && !thd->lex->current_select->with_sum_func)
-  {
-    set_if_smaller(table->s->max_rows, rows_limit);
-    param->end_write_records= rows_limit;
-  }
+  set_if_smaller(table->s->max_rows, rows_limit);
+  param->end_write_records= rows_limit;
 
   if (thd->is_fatal_error)				// If end of memory
     goto err;					 /* purecov: inspected */
Thread
bk commit into 5.0 tree (dlenev:1.2273) BUG#22338dlenev24 Sep