List:Commits« Previous MessageNext Message »
From:Gleb Shchepa Date:November 20 2008 5:45am
Subject:bzr commit into mysql-5.0-bugteam branch (gshchepa:2704) Bug#36569
View as plain text  
#At file:///work/bzr/5.0-bugteam-36569/ based on revid:gshchepa@stripped

 2704 Gleb Shchepa	2008-11-20
      Bug #36569: UPDATE ... WHERE ... ORDER BY... always does a
                  filesort even if not required
      
      The server uses filesort in single table UPDATE query even
      for already sorted data.
      
      NOTE: this bugfix is disabled by default. To enable it the
      new server command line switch has been introduced:
        --with-bugfix=36569 (or with-bugfix=36569 in my.cnf)
      If this switch is used with unknown bug number, the server
      prints a warning to stderr:
         This version doesn't have ability to switch fix BUG#N,
         see http://bugs.mysql.com/bug.php?id=N
      
      The mysql_update function has been modified to eliminate
      double sorting of data that is already sorted in a proper
      order.
added:
  mysql-test/r/update-bug36569.result
  mysql-test/t/update-bug36569-master.opt
  mysql-test/t/update-bug36569.test
modified:
  sql/mysql_priv.h
  sql/mysqld.cc
  sql/sql_select.cc
  sql/sql_select.h
  sql/sql_update.cc

per-file messages:
  mysql-test/r/update-bug36569.result
    Added test case for bug #36569.
  mysql-test/t/update-bug36569-master.opt
    Added test case for bug #36569.
  mysql-test/t/update-bug36569.test
    Added test case for bug #36569.
  sql/mysql_priv.h
    Bug #36569: UPDATE ... WHERE ... ORDER BY... always does a
                filesort even if not required
    
    Global bugfix_36569 variable has been declared to switch
    this fix on/off.
  sql/mysqld.cc
    Bug #36569: UPDATE ... WHERE ... ORDER BY... always does a
                filesort even if not required
    
    New server command line option has been introduced to
    enable bugfix N:
       --with-bugfix=N, where N is a natural number.
    Currently only N=36569 is supported.
  sql/sql_select.cc
    Bug #36569: UPDATE ... WHERE ... ORDER BY... always does a
                filesort even if not required
    
    1. The test_if_order_by_key function has been made public.
    2. The const_in_where function has been introduced to wrap
    the const_expression_in_where function and its last
    parameter (that actually is an intermediate value).
    3. The const_expression_in_where function has been modified
    to accept Field* arguments like Item*.
  sql/sql_select.h
    Bug #36569: UPDATE ... WHERE ... ORDER BY... always does a
                filesort even if not required
    
    1. The test_if_order_by_key function has been made public.
    2. The wrap_field_or_item class has been introduced to pass
    Field* or Item* values to const_in_where /
    const_expression_in_where functions in a generic way.
    2. The const_in_where function has been added.
  sql/sql_update.cc
    Bug #36569: UPDATE ... WHERE ... ORDER BY... always does a
                filesort even if not required
    
    1. The update_const_key_parts function has been added as a
    simplified version of make_join_statistics() to update
    TABLE::const_key_parts for single table UPDATE query.
    2. The simple_remove_const function has been added to filter
    out parts of the ORDER list that are constants in the WHERE
    clause. This function is a simplified one-table version of
    the remove_const function.
    3. The mysql_update function has been updated to compare
    used_index key parts with a non-constant part of the ORDER
    list and to eliminate unnecessary filesorting if any.
=== added file 'mysql-test/r/update-bug36569.result'
--- a/mysql-test/r/update-bug36569.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/r/update-bug36569.result	2008-11-20 05:45:13 +0000
@@ -0,0 +1,49 @@
+DROP TABLE IF EXISTS t1;
+CREATE TABLE t1 (a INT AUTO_INCREMENT PRIMARY KEY, b INT, c INT, INDEX (b,c));
+INSERT INTO t1 (b, c) VALUES (1, 1), (1, 2), (1,3), (2, 2), (2, 3), (2,4);
+SELECT * FROM t1;
+a	b	c
+1	1	1
+2	1	2
+3	1	3
+4	2	2
+5	2	3
+6	2	4
+UPDATE t1 SET a = a + 10 WHERE b = 1 ORDER BY c LIMIT 2;
+SHOW SESSION STATUS LIKE 'Sort%';
+Variable_name	Value
+Sort_merge_passes	0
+Sort_range	0
+Sort_rows	0
+Sort_scan	0
+SELECT * FROM t1;
+a	b	c
+11	1	1
+12	1	2
+3	1	3
+4	2	2
+5	2	3
+6	2	4
+UPDATE t1 SET a = a + 10 WHERE b = 1 ORDER BY b, c LIMIT 2;
+SHOW SESSION STATUS LIKE 'Sort%';
+Variable_name	Value
+Sort_merge_passes	0
+Sort_range	0
+Sort_rows	0
+Sort_scan	0
+UPDATE t1 SET a = a + 10 WHERE b = 1 ORDER BY b LIMIT 2;
+SHOW SESSION STATUS LIKE 'Sort%';
+Variable_name	Value
+Sort_merge_passes	0
+Sort_range	0
+Sort_rows	0
+Sort_scan	0
+# needs filesort:
+UPDATE t1 SET c = c + 10 WHERE b = 1 ORDER BY c LIMIT 2;
+SHOW SESSION STATUS LIKE 'Sort%';
+Variable_name	Value
+Sort_merge_passes	0
+Sort_range	1
+Sort_rows	2
+Sort_scan	0
+DROP TABLE t1;

=== added file 'mysql-test/t/update-bug36569-master.opt'
--- a/mysql-test/t/update-bug36569-master.opt	1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/update-bug36569-master.opt	2008-11-20 05:45:13 +0000
@@ -0,0 +1 @@
+--with-bugfix=36569

=== added file 'mysql-test/t/update-bug36569.test'
--- a/mysql-test/t/update-bug36569.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/update-bug36569.test	2008-11-20 05:45:13 +0000
@@ -0,0 +1,28 @@
+#
+# Bug #36569: UPDATE ... WHERE ... ORDER BY... always does a filesort even if
+#             not required
+#
+
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+--enable_warnings
+
+CREATE TABLE t1 (a INT AUTO_INCREMENT PRIMARY KEY, b INT, c INT, INDEX (b,c));
+INSERT INTO t1 (b, c) VALUES (1, 1), (1, 2), (1,3), (2, 2), (2, 3), (2,4);
+
+SELECT * FROM t1;
+UPDATE t1 SET a = a + 10 WHERE b = 1 ORDER BY c LIMIT 2;
+SHOW SESSION STATUS LIKE 'Sort%';
+SELECT * FROM t1;
+
+UPDATE t1 SET a = a + 10 WHERE b = 1 ORDER BY b, c LIMIT 2;
+SHOW SESSION STATUS LIKE 'Sort%';
+
+UPDATE t1 SET a = a + 10 WHERE b = 1 ORDER BY b LIMIT 2;
+SHOW SESSION STATUS LIKE 'Sort%';
+
+--echo # needs filesort:
+UPDATE t1 SET c = c + 10 WHERE b = 1 ORDER BY c LIMIT 2;
+SHOW SESSION STATUS LIKE 'Sort%';
+
+DROP TABLE t1;

=== modified file 'sql/mysql_priv.h'
--- a/sql/mysql_priv.h	2008-10-02 11:57:52 +0000
+++ b/sql/mysql_priv.h	2008-11-20 05:45:13 +0000
@@ -1343,6 +1343,7 @@ extern my_bool opt_enable_shared_memory;
 extern char *default_tz_name;
 extern my_bool opt_large_pages;
 extern uint opt_large_page_size;
+extern my_bool bugfix_36569;
 
 extern char *opt_plugin_dir_ptr;
 extern char opt_plugin_dir[FN_REFLEN];

=== modified file 'sql/mysqld.cc'
--- a/sql/mysqld.cc	2008-08-26 08:32:43 +0000
+++ b/sql/mysqld.cc	2008-11-20 05:45:13 +0000
@@ -375,6 +375,7 @@ my_bool locked_in_memory;
 bool opt_using_transactions, using_update_log;
 bool volatile abort_loop;
 bool volatile shutdown_in_progress;
+my_bool bugfix_36569= FALSE;
 /**
    @brief 'grant_option' is used to indicate if privileges needs
    to be checked, in which case the lock, LOCK_grant, is used
@@ -4965,6 +4966,7 @@ enum options_mysqld
   OPT_EXPIRE_LOGS_DAYS,
   OPT_GROUP_CONCAT_MAX_LEN,
   OPT_DEFAULT_COLLATION,
+  OPT_WITH_BUGFIX,
   OPT_CHARACTER_SET_CLIENT_HANDSHAKE,
   OPT_CHARACTER_SET_FILESYSTEM,
   OPT_LC_TIME_NAMES,
@@ -5081,6 +5083,8 @@ Disable with --skip-bdb (will save memor
   {"bootstrap", OPT_BOOTSTRAP, "Used by mysql installation scripts.", 0, 0, 0,
    GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0},
 #endif
+  {"with-bugfix", OPT_WITH_BUGFIX, "Allow fix for bug N", 0, 0, 0,
+    GET_ULONG, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
   {"character-set-client-handshake", OPT_CHARACTER_SET_CLIENT_HANDSHAKE,
    "Don't ignore client side character set value sent during handshake.",
    (gptr*) &opt_character_set_client_handshake,
@@ -7542,6 +7546,15 @@ get_one_option(int optid, const struct m
     lower_case_table_names= argument ? atoi(argument) : 1;
     lower_case_table_names_used= 1;
     break;
+  case OPT_WITH_BUGFIX:
+    switch(atoi(argument)) {
+      case 36569: bugfix_36569= TRUE; break;
+      default:
+        fprintf(stderr, "This version doesn't have ability to switch fix"
+                        " BUG#%s, see http://bugs.mysql.com/bug.php?id=%s\n",
+            argument, argument);
+    }
+    break;
   }
   return 0;
 }

=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc	2008-10-10 10:13:12 +0000
+++ b/sql/sql_select.cc	2008-11-20 05:45:13 +0000
@@ -109,7 +109,6 @@ static uint build_bitmap_for_nested_join
 static COND *optimize_cond(JOIN *join, COND *conds,
                            List<TABLE_LIST> *join_list,
 			   Item::cond_result *cond_value);
-static bool const_expression_in_where(COND *conds,Item *item, Item **comp_item);
 static bool open_tmp_table(TABLE *table);
 static bool create_myisam_tmp_table(TABLE *table,TMP_TABLE_PARAM *param,
 				    ulonglong options);
@@ -6693,8 +6692,7 @@ remove_const(JOIN *join,ORDER *first_ord
 	*simple_order=0;
       else
       {
-	Item *comp_item=0;
-	if (cond && const_expression_in_where(cond,order->item[0], &comp_item))
+        if (cond && const_in_where(cond, order->item[0]))
 	{
 	  DBUG_PRINT("info",("removing: %s", order->item[0]->full_name()));
 	  continue;
@@ -8780,12 +8778,18 @@ test_if_equality_guarantees_uniqueness(I
         l->collation.collation == r->collation.collation)));
 }
 
-/*
-  Return 1 if the item is a const value in all the WHERE clause
-*/
+/**
+  Test if a field or an item is a constant value in the WHERE clause expression
+
+  @param        cond            WHERE clause expression
+  @param        comp            Item or field to find in WHERE expression
+  @param[out]   const_item      intermediate arg, set to Item pointer to NULL 
 
+  @return TRUE if the field is a constant value in WHERE
+*/
 static bool
-const_expression_in_where(COND *cond, Item *comp_item, Item **const_item)
+const_expression_in_where(COND *cond, const wrap_field_or_item &comp,
+                          Item **const_item)
 {
   if (cond->type() == Item::COND_ITEM)
   {
@@ -8795,7 +8799,7 @@ const_expression_in_where(COND *cond, It
     Item *item;
     while ((item=li++))
     {
-      bool res=const_expression_in_where(item, comp_item, const_item);
+      bool res=const_expression_in_where(item, comp, const_item);
       if (res)					// Is a const value
       {
 	if (and_level)
@@ -8814,7 +8818,7 @@ const_expression_in_where(COND *cond, It
       return 0;
     Item *left_item=	((Item_func*) cond)->arguments()[0];
     Item *right_item= ((Item_func*) cond)->arguments()[1];
-    if (left_item->eq(comp_item,1))
+    if (comp.eq(left_item))
     {
       if (test_if_equality_guarantees_uniqueness (left_item, right_item))
       {
@@ -8824,7 +8828,7 @@ const_expression_in_where(COND *cond, It
 	return 1;
       }
     }
-    else if (right_item->eq(comp_item,1))
+    else if (comp.eq(right_item))
     {
       if (test_if_equality_guarantees_uniqueness (right_item, left_item))
       {
@@ -8838,6 +8842,23 @@ const_expression_in_where(COND *cond, It
   return 0;
 }
 
+
+/**
+  Test if a field or an item is a constant value in the WHERE clause expression
+
+  @param        cond            WHERE clause expression
+  @param        comp            Item or field to find in WHERE expression
+
+  @return TRUE if the field is a constant value in WHERE
+*/
+bool
+const_in_where(COND *cond, const wrap_field_or_item &comp)
+{
+  Item *dummy= NULL;
+  return const_expression_in_where(cond, comp, &dummy);
+}
+
+
 /****************************************************************************
   Create internal temporary table
 ****************************************************************************/
@@ -12118,8 +12139,8 @@ part_of_refkey(TABLE *table,Field *field
     @retval -1  Reverse read on the key can be used
 */
 
-static int test_if_order_by_key(ORDER *order, TABLE *table, uint idx,
-				uint *used_key_parts)
+int test_if_order_by_key(ORDER *order, TABLE *table, uint idx,
+                         uint *used_key_parts)
 {
   KEY_PART_INFO *key_part,*key_part_end;
   key_part=table->key_info[idx].key_part;

=== modified file 'sql/sql_select.h'
--- a/sql/sql_select.h	2007-11-07 16:02:12 +0000
+++ b/sql/sql_select.h	2008-11-20 05:45:13 +0000
@@ -660,3 +660,26 @@ bool error_if_full_join(JOIN *join);
 int report_error(TABLE *table, int error);
 int safe_index_read(JOIN_TAB *tab);
 COND *remove_eq_conds(THD *thd, COND *cond, Item::cond_result *cond_value);
+int test_if_order_by_key(ORDER *order, TABLE *table, uint idx,
+                         uint *used_key_parts);
+
+
+/*
+  Generic wrapper to pass Field* or Item* to const_expression_in_where().
+*/
+class wrap_field_or_item {
+  Field *field;
+  Item  *item;
+public:
+  wrap_field_or_item(Field *field_arg) : field(field_arg), item(NULL) {}
+  wrap_field_or_item(Item *item_arg) : field(NULL), item(item_arg) {}
+  bool eq(Item *item_arg) const
+  {
+    return item ? item_arg->eq(item, 1)
+                : (item_arg->type() == Item::FIELD_ITEM &&
+                   ((Item_field *)item_arg)->field->eq(field));
+  }
+};
+
+bool const_in_where(COND *conds, const wrap_field_or_item &expr);
+

=== modified file 'sql/sql_update.cc'
--- a/sql/sql_update.cc	2008-07-15 14:13:21 +0000
+++ b/sql/sql_update.cc	2008-11-20 05:45:13 +0000
@@ -84,6 +84,74 @@ static bool check_fields(THD *thd, List<
 
 
 /*
+  Update TABLE::const_key_parts for single table UPDATE query
+
+  SYNOPSIS
+    update_const_key_parts()
+    conds               WHERE clause expression
+    table               table to UPDATE
+    index               index number
+
+  NOTE
+    This is a simplified version of make_join_statistics() -- unfortunately
+    that function is not suitable for single table UPDATE query preparation.
+*/
+
+static void update_const_key_parts(COND *conds, TABLE *table, uint index)
+{
+  KEY_PART_INFO *keyinfo= table->key_info[index].key_part;
+  KEY_PART_INFO *keyinfo_end= keyinfo + table->key_info[index].key_parts;
+
+  table->const_key_parts[index]= (key_part_map)0;
+
+  for (key_part_map part_map= (key_part_map)1; 
+       keyinfo < keyinfo_end;
+       keyinfo++, part_map<<= 1)
+  {
+    if (const_in_where(conds, keyinfo->field))
+      table->const_key_parts[index]|= part_map;
+  }
+}
+
+
+/*
+  Simplified one-table version of remove_const
+
+  SYNOPSIS
+    simple_remove_const()
+    order               ORDER list
+    where               WHERE clause expression
+
+  RETURNS
+    NULL if order list is constant or a pointer to the first
+    non-constant order item.
+
+  NOTE
+    This function overwrites order list.
+*/
+
+static ORDER * simple_remove_const(ORDER *order, COND *where)
+{
+  ORDER *first= NULL, *prev= NULL;
+  for (ORDER *ord= order; ord; ord= ord->next)
+  {
+    DBUG_ASSERT(!order->item[0]->with_sum_func); // should never happen
+    if (!const_in_where(where, ord->item[0]))
+    {
+      if (!first)
+        first= ord;
+      if (prev)
+        prev->next= ord;
+      prev= ord;
+    }
+  }
+  if (prev)
+    prev->next= NULL;
+  return first;
+}
+
+
+/*
   Process usual UPDATE
 
   SYNOPSIS
@@ -278,6 +346,34 @@ int mysql_update(THD *thd,
     used_index= select->quick->index;
     used_key_is_modified= (!select->quick->unique_key_range() &&
                           select->quick->is_keys_used(&fields));
+    if (bugfix_36569 &&
+        need_sort && !used_key_is_modified && order && used_index != MAX_KEY &&
+        (table->file->index_flags(used_index, 0, 1) & HA_READ_ORDER))
+    {
+      update_const_key_parts(conds, table, used_index);
+      if (used_index != table->s->primary_key &&
+          table->s->primary_key != MAX_KEY &&
+          (table->file->table_flags() & HA_PRIMARY_KEY_IN_READ_INDEX))
+        update_const_key_parts(conds, table, table->s->primary_key);
+
+      if ((order= simple_remove_const(order, conds)))
+      {
+        uint dummy;
+        switch (test_if_order_by_key(order, table, used_index, &dummy))
+        {
+          case -1:
+            if (select->quick->reverse_sorted())
+              need_sort= FALSE;
+            break;
+          case 1:
+            if (!select->quick->reverse_sorted())
+              need_sort= FALSE;
+            break;
+        }
+      }
+      else
+        need_sort= FALSE;
+    }
   }
   else
   {

Thread
bzr commit into mysql-5.0-bugteam branch (gshchepa:2704) Bug#36569Gleb Shchepa20 Nov