#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#36569 | Gleb Shchepa | 20 Nov |
