List:Commits« Previous MessageNext Message »
From:Gleb Shchepa Date:October 27 2008 4:58pm
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/

 2704 Gleb Shchepa	2008-10-27
      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.
      
      The mysql_update function has been modified to eliminate double sorting
      of data that is already sorted in a proper order.
modified:
  mysql-test/r/update.result
  mysql-test/t/update.test
  sql/sql_select.cc
  sql/sql_select.h
  sql/sql_update.cc

per-file messages:
  mysql-test/r/update.result
    Added test case for bug #36569.
  mysql-test/t/update.test
    Added test case for bug #36569.
  sql/sql_select.cc
    Bug #36569: UPDATE ... WHERE ... ORDER BY... always does a filesort even
                if not required
    
    1. The const_expression_in_where and the test_if_order_by_key functions
       have been made public.
    2. The const_field_in_where function has been added as a variety
       of the const_expression_in_where for the Field argument instead of
       the Item argument.
  sql/sql_select.h
    Bug #36569: UPDATE ... WHERE ... ORDER BY... always does a filesort even
                if not required
    
    1. The const_expression_in_where and the test_if_order_by_key functions
       have been made public.
    2. The const_field_in_where function has been added as a variety
       of the const_expression_in_where for the Field argument instead of
       the Item argument.
  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 mysql_update function has been updated to compare used_index key
       parts with a non-constant tail of the ORDER list and to eliminate
       unnecessary filesorting if any.
=== modified file 'mysql-test/r/update.result'
--- a/mysql-test/r/update.result	2007-04-23 14:22:33 +0000
+++ b/mysql-test/r/update.result	2008-10-27 16:57:51 +0000
@@ -491,4 +491,52 @@ update t1 join t2 on (t1.a=t2.a) set t1.
 affected rows: 127
 info: Rows matched: 128  Changed: 127  Warnings: 0
 drop table t1,t2;
+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;
 End of 5.0 tests

=== modified file 'mysql-test/t/update.test'
--- a/mysql-test/t/update.test	2007-04-23 14:22:33 +0000
+++ b/mysql-test/t/update.test	2008-10-27 16:57:51 +0000
@@ -430,4 +430,29 @@ drop table t1,t2;
 connection default;
 disconnect con1;
 
+#
+# Bug #36569: UPDATE ... WHERE ... ORDER BY... always does a filesort even if
+#             not required
+#
+
+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;
+
 --echo End of 5.0 tests

=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc	2008-10-10 10:13:12 +0000
+++ b/sql/sql_select.cc	2008-10-27 16:57:51 +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);
@@ -8784,7 +8783,7 @@ test_if_equality_guarantees_uniqueness(I
   Return 1 if the item is a const value in all the WHERE clause
 */
 
-static bool
+bool
 const_expression_in_where(COND *cond, Item *comp_item, Item **const_item)
 {
   if (cond->type() == Item::COND_ITEM)
@@ -8838,6 +8837,77 @@ const_expression_in_where(COND *cond, It
   return 0;
 }
 
+
+/**
+  Test if a field is a constant value in the WHERE clause expression
+
+  This is a clone of the const_expression_in_where function for the
+  Field arguments.
+
+  @param        cond            WHERE clause expression
+  @param        field           Field to find in WHERE expression
+  @param[out]   const_item      intermediate arg, set to Item pointer to NULL 
+
+  @return indication if the field is a constant value in WHERE
+*/
+
+bool
+const_field_in_where(COND *cond, Field *field, Item **const_item)
+{
+  if (cond->type() == Item::COND_ITEM)
+  {
+    bool and_level= (((Item_cond*) cond)->functype()
+		     == Item_func::COND_AND_FUNC);
+    List_iterator_fast<Item> li(*((Item_cond*) cond)->argument_list());
+    Item *item;
+    while ((item=li++))
+    {
+      bool res=const_field_in_where(item, field, const_item);
+      if (res)					// Is a const value
+      {
+	if (and_level)
+	  return 1;
+      }
+      else if (!and_level)
+	return 0;
+    }
+    return and_level ? 0 : 1;
+  }
+  else if (cond->eq_cmp_result() != Item::COND_OK)
+  {						// boolan compare function
+    Item_func* func= (Item_func*) cond;
+    if (func->functype() != Item_func::EQUAL_FUNC &&
+	func->functype() != Item_func::EQ_FUNC)
+      return 0;
+    Item *left_item=	((Item_func*) cond)->arguments()[0];
+    Item *right_item= ((Item_func*) cond)->arguments()[1];
+    if (left_item->type() == Item::FIELD_ITEM &&
+        ((Item_field *)left_item)->field->eq(field))
+    {
+      if (test_if_equality_guarantees_uniqueness (left_item, right_item))
+      {
+	if (*const_item)
+	  return right_item->eq(*const_item, 1);
+	*const_item=right_item;
+	return 1;
+      }
+    }
+    else if (right_item->type() == Item::FIELD_ITEM &&
+        ((Item_field *)right_item)->field->eq(field))
+    {
+      if (test_if_equality_guarantees_uniqueness (right_item, left_item))
+      {
+	if (*const_item)
+	  return left_item->eq(*const_item, 1);
+	*const_item=left_item;
+	return 1;
+      }
+    }
+  }
+  return 0;
+}
+
+
 /****************************************************************************
   Create internal temporary table
 ****************************************************************************/
@@ -12118,8 +12188,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-10-27 16:57:51 +0000
@@ -660,3 +660,8 @@ 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);
+bool const_expression_in_where(COND *conds,Item *item, Item **comp_item);
+bool const_field_in_where(COND *conds, Field *field, Item **comp_item);
+

=== modified file 'sql/sql_update.cc'
--- a/sql/sql_update.cc	2008-07-15 14:13:21 +0000
+++ b/sql/sql_update.cc	2008-10-27 16:57:51 +0000
@@ -84,6 +84,41 @@ 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)
+{
+  Item *dummy_const_item= NULL;
+  key_part_map part_map= (key_part_map)1;
+  KEY_PART_INFO *keyinfo= table->key_info[index].key_part;
+  KEY_PART_INFO *keyinfo_end= keyinfo + table->key_info[index].key_parts;
+
+  DBUG_ASSERT(keyinfo != keyinfo_end);
+  table->const_key_parts[index]= (key_part_map)0;
+  if (const_field_in_where(conds, keyinfo->field, &dummy_const_item))
+    table->const_key_parts[index]|= part_map;
+
+  for (keyinfo++, part_map<<= 1;
+       keyinfo < keyinfo_end;
+       keyinfo++, part_map<<= 1)
+  {
+    if (const_field_in_where(conds, keyinfo->field, &dummy_const_item))
+      table->const_key_parts[index]|= part_map;
+  }
+}
+
+
+/*
   Process usual UPDATE
 
   SYNOPSIS
@@ -278,6 +313,42 @@ 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 (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);
+
+      ORDER *ord; // the 1st ORDER list item that isn't a constant in WHERE
+      for (ord= order; ord; ord= ord->next)
+      {
+        Item *dummy_comp_item= NULL;
+        DBUG_ASSERT(!order->item[0]->with_sum_func); // should never happen
+        if (!const_expression_in_where(conds, ord->item[0], &dummy_comp_item))
+          break;
+      }
+
+      if (!ord)
+        need_sort= FALSE;
+      else
+      {
+        uint dummy;
+        switch (test_if_order_by_key(ord, 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
   {

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