From 190efb06e7c9fe96bb4f4294f3b0ddbc4eef12de Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 29 Jan 2007 15:07:11 +0100 Subject: [PATCH] BUG#20604: FORCE INDEX uses keys disabled by ALTER TABLE The function that checks whether we can use keys for aggregates, find_key_for_maxmin(), assumes that keys disabled by ALTER TABLE ... DISABLE KEYS are not in the set table->keys_in_use_for_query. I.E., if a key is in this set, the optimizer assumes it is free to use it. The bug is that keys disabled with ALTER TABLE ... DISABLE KEYS still appear in table->keys_in_use_for_query When the TABLE object has been initialized with setup_tables(). Before setup_tables is called, however, keys that are disabled in the aforementioned way are not included in TABLE::keys_in_use_for_query. The provided patch changes the code that updates keys_is_use_for_query so that it assumes that keys_is_use_for_query already takes into account all disabled keys, and generally all keys that should be used by the query. mysql-test/r/key.result: Test for BUG#20604. The important part of the test is the explain output that tests what indexes are used. mysql-test/t/key.test: The minimal test case that reveals the bug. The optimizer for aggregates relies on keys disabled with ALTER TABLE ... DISABLE KEYS not being in the set TABLE::keys_in_use_for_query. When the execution engine tries to use a disabled index, MyISAM returns an error. sql/sql_base.cc: Exclude the keys disabled by ALTER TABLE ... DISABLE_KEYS from TABLE::keys_in_use_for_query, and in general, don't introduce any new keys. We may not know why keys have been removed at previous stages. sql/sql_select.cc: The intersection operation between table->s->keys_in_use and table->keys_in_use_for_query is no longer necessary. We can trust that the latter is a subset of the former. sql/table.h: Added comments to TABLE_SHARE::keys_in_use and TABLE::keys_in_use_for_query. --- mysql-test/r/key.result | 7 +++++++ mysql-test/t/key.test | 11 +++++++++++ sql/sql_base.cc | 7 ++++++- sql/sql_select.cc | 11 ++++++----- sql/table.h | 23 +++++++++++++++++++++-- 5 files changed, 51 insertions(+), 8 deletions(-) diff --git a/mysql-test/r/key.result b/mysql-test/r/key.result index ea8d4338d08..853b837c46e 100644 --- a/mysql-test/r/key.result +++ b/mysql-test/r/key.result @@ -482,3 +482,10 @@ alter table t1 drop index i3, drop index i2, drop index i1; alter table t1 add index i3 (c3), add index i2 (c2), add unique index i1 (c1); ERROR 23000: Duplicate entry '1' for key 'i1' drop table t1; +CREATE TABLE t1( a TINYINT, KEY(a) ) ENGINE=MyISAM; +INSERT INTO t1 VALUES( 1 ); +ALTER TABLE t1 DISABLE KEYS; +EXPLAIN SELECT MAX(a) FROM t1 FORCE INDEX(a); +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 system NULL NULL NULL NULL 1 +drop table t1; diff --git a/mysql-test/t/key.test b/mysql-test/t/key.test index 7c6b38cb871..5a5057f9c97 100644 --- a/mysql-test/t/key.test +++ b/mysql-test/t/key.test @@ -442,3 +442,14 @@ alter table t1 drop index i3, drop index i2, drop index i1; alter table t1 add index i3 (c3), add index i2 (c2), add unique index i1 (c1); drop table t1; + +# +# Bug #20604: Test for disabled keys with aggregate functions and FORCE INDEX. +# + +CREATE TABLE t1( a TINYINT, KEY(a) ) ENGINE=MyISAM; +INSERT INTO t1 VALUES( 1 ); +ALTER TABLE t1 DISABLE KEYS; +EXPLAIN SELECT MAX(a) FROM t1 FORCE INDEX(a); + +drop table t1; diff --git a/sql/sql_base.cc b/sql/sql_base.cc index c03910a7829..bb84e89a2ef 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -5462,7 +5462,12 @@ bool setup_tables(THD *thd, Name_resolution_context *context, get_key_map_from_key_list(&map, table, table_list->use_index); if (map.is_set_all()) DBUG_RETURN(1); - table->keys_in_use_for_query=map; + /* + Don't introduce keys in keys_in_use_for_query that weren't there + before. FORCE/USE INDEX should not add keys, it should only remove + all keys except the key(s) specified in the hint. + */ + table->keys_in_use_for_query.intersect(map); } if (table_list->ignore_index) { diff --git a/sql/sql_select.cc b/sql/sql_select.cc index c11553e9eed..bd2a7a87c53 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -12262,13 +12262,14 @@ test_if_skip_sort_order(JOIN_TAB *tab,ORDER *order,ha_rows select_limit, DBUG_ENTER("test_if_skip_sort_order"); LINT_INIT(ref_key_parts); + /* Check which keys can be used to resolve ORDER BY. */ + usable_keys= table->keys_in_use_for_query; + /* - Check which keys can be used to resolve ORDER BY. - We must not try to use disabled keys. + Keys disabled by ALTER TABLE ... DISABLE KEYS should have already + been taken into account. */ - usable_keys= table->s->keys_in_use; - /* we must not consider keys that are disabled by IGNORE INDEX */ - usable_keys.intersect(table->keys_in_use_for_query); + DBUG_ASSERT(usable_keys.is_subset(table->s->keys_in_use)); for (ORDER *tmp_order=order; tmp_order ; tmp_order=tmp_order->next) { diff --git a/sql/table.h b/sql/table.h index 82083d79570..3c9f2cac164 100644 --- a/sql/table.h +++ b/sql/table.h @@ -158,7 +158,12 @@ typedef struct st_table_share LEX_STRING path; /* Path to .frm file (from datadir) */ LEX_STRING normalized_path; /* unpack_filename(path) */ LEX_STRING connect_string; - key_map keys_in_use; /* Keys in use for table */ + + /* + Set of keys in use, implemented as a Bitmap. + Excludes keys disabled by ALTER TABLE ... DISABLE KEYS. + */ + key_map keys_in_use; key_map keys_for_keyread; ha_rows min_rows, max_rows; /* create information */ ulong avg_row_length; /* create information */ @@ -313,7 +318,21 @@ struct st_table { byte *write_row_record; /* Used as optimisation in THD::write_row */ byte *insert_values; /* used by INSERT ... UPDATE */ - key_map quick_keys, used_keys, keys_in_use_for_query, merge_keys; + key_map quick_keys, used_keys; + + /* + A set of keys that can be used in the query that references this + table + + All indexes disabled on the table's TABLE_SHARE (see TABLE::s) will be + subtracted from this set upon instantiation. Thus for any TABLE t it holds + that t.keys_in_use_for_query is a subset of t.s.keys_in_use. Generally we + must not introduce any new keys here (see setup_tables). + + The set is implemented as a bitmap. + */ + key_map keys_in_use_for_query; + key_map merge_keys; KEY *key_info; /* data of keys in database */ Field *next_number_field; /* Set if next_number is activated */