From 9f2bddbd80fae92840c2db07b75968e816adc213 Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 2 Nov 2010 15:27:01 +0200 Subject: [PATCH] Fixed LP BUG#652727 and LP BUG#643424. The fixes for #643424 was part of the fix for #652727, that's why both fixes are pushed together. - The cause for #643424 was the improper use of get_partial_join_cost(), which assumed that the 'n_tables' parameter was the upper bound for query plan node indexes. Fixed by generalizing get_partial_join_cost() as a method that computes the cost of any partial join. - The cause of #652727 was that JOIN::choose_subquery_plan() incorrectly deleted the contents of the old keyuse array in the cases when an injected plan would not provide more key accesses, and reoptimization was not actually performed. --- mysql-test/r/subselect_mat_cost.result | 51 ++++++++++++++++++ mysql-test/t/subselect_mat_cost.test | 68 ++++++++++++++++++++++++ sql/opt_subselect.cc | 48 ++++++++--------- sql/sql_select.cc | 72 ++++++++++++-------------- sql/sql_select.h | 16 ++++-- 5 files changed, 188 insertions(+), 67 deletions(-) diff --git a/mysql-test/r/subselect_mat_cost.result b/mysql-test/r/subselect_mat_cost.result index 9d6986cf092..b5db804f7ff 100644 --- a/mysql-test/r/subselect_mat_cost.result +++ b/mysql-test/r/subselect_mat_cost.result @@ -3769,3 +3769,54 @@ drop procedure delete_materialization_data; drop procedure set_all_columns_not_null; drop procedure set_all_columns_nullable; drop table t1, t2, t1_1024, t2_1024; +# +# LP BUG#643424 valgrind warning in choose_subquery_plan() +# +CREATE TABLE t1 ( +pk int(11) NOT NULL AUTO_INCREMENT, +c1 int(11) DEFAULT NULL, +c2 int(11) DEFAULT NULL, +PRIMARY KEY (pk), +KEY c2 (c2)); +INSERT INTO t1 VALUES (1,NULL,2); +INSERT INTO t1 VALUES (2,7,9); +INSERT INTO t1 VALUES (9,NULL,8); +CREATE TABLE t2 ( +pk int(11) NOT NULL AUTO_INCREMENT, +c1 int(11) DEFAULT NULL, +c2 int(11) DEFAULT NULL, +PRIMARY KEY (pk), +KEY c2 (c2)); +INSERT INTO t2 VALUES (1,1,7); +set @save_optimizer_switch=@@optimizer_switch; +set @@optimizer_switch='materialization=on,in_to_exists=on,semijoin=off'; +SELECT pk FROM t1 WHERE (c2, c1) IN (SELECT c2, c2 FROM t2); +pk +set session optimizer_switch=@save_optimizer_switch; +drop table t1, t2; +# +# LP BUG#652727 Crash in create_ref_for_key() +# +CREATE TABLE t2 ( +pk int(11) NOT NULL AUTO_INCREMENT, +c1 int(11) DEFAULT NULL, +PRIMARY KEY (pk)); +INSERT INTO t2 VALUES (10,7); +INSERT INTO t2 VALUES (11,1); +INSERT INTO t2 VALUES (17,NULL); +CREATE TABLE t1 ( +pk int(11) NOT NULL AUTO_INCREMENT, +c1 int(11) DEFAULT NULL, +PRIMARY KEY (pk)); +INSERT INTO t1 VALUES (15,1); +INSERT INTO t1 VALUES (19,NULL); +CREATE TABLE t3 (c2 int(11) DEFAULT NULL, KEY c2 (c2)); +INSERT INTO t3 VALUES (1); +set @save_optimizer_switch=@@optimizer_switch; +set @@optimizer_switch='materialization=on,in_to_exists=on,semijoin=off'; +SELECT c2 +FROM t3 +WHERE (2, 6) IN (SELECT t1.c1, t1.c1 FROM t1 STRAIGHT_JOIN t2 ON t2.pk = t1.pk); +c2 +set session optimizer_switch=@save_optimizer_switch; +drop table t1, t2, t3; diff --git a/mysql-test/t/subselect_mat_cost.test b/mysql-test/t/subselect_mat_cost.test index a268322dc5f..b817c05f7d4 100644 --- a/mysql-test/t/subselect_mat_cost.test +++ b/mysql-test/t/subselect_mat_cost.test @@ -201,3 +201,71 @@ drop procedure delete_materialization_data; drop procedure set_all_columns_not_null; drop procedure set_all_columns_nullable; drop table t1, t2, t1_1024, t2_1024; + +--echo # +--echo # LP BUG#643424 valgrind warning in choose_subquery_plan() +--echo # + +CREATE TABLE t1 ( + pk int(11) NOT NULL AUTO_INCREMENT, + c1 int(11) DEFAULT NULL, + c2 int(11) DEFAULT NULL, + PRIMARY KEY (pk), + KEY c2 (c2)); + +INSERT INTO t1 VALUES (1,NULL,2); +INSERT INTO t1 VALUES (2,7,9); +INSERT INTO t1 VALUES (9,NULL,8); + +CREATE TABLE t2 ( + pk int(11) NOT NULL AUTO_INCREMENT, + c1 int(11) DEFAULT NULL, + c2 int(11) DEFAULT NULL, + PRIMARY KEY (pk), + KEY c2 (c2)); + +INSERT INTO t2 VALUES (1,1,7); + +set @save_optimizer_switch=@@optimizer_switch; +set @@optimizer_switch='materialization=on,in_to_exists=on,semijoin=off'; + +SELECT pk FROM t1 WHERE (c2, c1) IN (SELECT c2, c2 FROM t2); + +set session optimizer_switch=@save_optimizer_switch; + +drop table t1, t2; + + +--echo # +--echo # LP BUG#652727 Crash in create_ref_for_key() +--echo # + +CREATE TABLE t2 ( + pk int(11) NOT NULL AUTO_INCREMENT, + c1 int(11) DEFAULT NULL, + PRIMARY KEY (pk)); + +INSERT INTO t2 VALUES (10,7); +INSERT INTO t2 VALUES (11,1); +INSERT INTO t2 VALUES (17,NULL); + +CREATE TABLE t1 ( + pk int(11) NOT NULL AUTO_INCREMENT, + c1 int(11) DEFAULT NULL, + PRIMARY KEY (pk)); + +INSERT INTO t1 VALUES (15,1); +INSERT INTO t1 VALUES (19,NULL); + +CREATE TABLE t3 (c2 int(11) DEFAULT NULL, KEY c2 (c2)); +INSERT INTO t3 VALUES (1); + +set @save_optimizer_switch=@@optimizer_switch; +set @@optimizer_switch='materialization=on,in_to_exists=on,semijoin=off'; + +SELECT c2 +FROM t3 +WHERE (2, 6) IN (SELECT t1.c1, t1.c1 FROM t1 STRAIGHT_JOIN t2 ON t2.pk = t1.pk); + +set session optimizer_switch=@save_optimizer_switch; +drop table t1, t2, t3; diff --git a/sql/opt_subselect.cc b/sql/opt_subselect.cc index a15418ad23d..a9241e7f18c 100644 --- a/sql/opt_subselect.cc +++ b/sql/opt_subselect.cc @@ -1204,8 +1204,8 @@ bool optimize_semijoin_nests(JOIN *join, table_map all_table_map) sjm->tables= n_tables; sjm->is_used= FALSE; double subjoin_out_rows, subjoin_read_time; - get_partial_join_cost(join, n_tables, - &subjoin_read_time, &subjoin_out_rows); + join->get_partial_join_cost(n_tables + join->const_tables, + &subjoin_read_time, &subjoin_out_rows); sjm->materialization_cost.convert_from_cost(subjoin_read_time); sjm->rows= subjoin_out_rows; @@ -3615,16 +3615,14 @@ bool JOIN::optimize_unflattened_subqueries() */ bool JOIN::choose_subquery_plan(table_map join_tables) -{ - /* The original QEP of the subquery. */ +{ /* The original QEP of the subquery. */ DYNAMIC_ARRAY save_keyuse; /* Copy of the JOIN::keyuse array. */ POSITION save_best_positions[MAX_TABLES+1]; /* Copy of JOIN::best_positions */ /* Copies of the JOIN_TAB::keyuse pointers for each JOIN_TAB. */ KEYUSE *save_join_tab_keyuse[MAX_TABLES]; /* Copies of JOIN_TAB::checked_keys for each JOIN_TAB. */ key_map save_join_tab_checked_keys[MAX_TABLES]; - - bool in_exists_reoptimized= false; + enum_reopt_result reopt_result= REOPT_NONE; Item_in_subselect *in_subs; if (select_lex->master_unit()->item && @@ -3667,8 +3665,8 @@ bool JOIN::choose_subquery_plan(table_map join_tables) double in_exists_strategy_cost; if (outer_join) - get_partial_join_cost(outer_join, outer_join->tables, - &outer_read_time, &outer_record_count); + outer_join->get_partial_join_cost(outer_join->tables, + &outer_read_time, &outer_record_count); else { /* @@ -3679,8 +3677,8 @@ bool JOIN::choose_subquery_plan(table_map join_tables) outer_record_count= 1; /* TODO */ } - get_partial_join_cost(inner_join, inner_join->tables, - &inner_read_time_1, &inner_record_count_1); + inner_join->get_partial_join_cost(inner_join->tables, + &inner_read_time_1, &inner_record_count_1); if (in_to_exists_where && const_tables != tables) { @@ -3689,13 +3687,17 @@ bool JOIN::choose_subquery_plan(table_map join_tables) conditions. */ if (save_query_plan(&save_keyuse, save_best_positions, - save_join_tab_keyuse, save_join_tab_checked_keys) || - reoptimize(in_to_exists_where, join_tables, save_best_positions)) + save_join_tab_keyuse, save_join_tab_checked_keys)) + return TRUE; + reopt_result= reoptimize(in_to_exists_where, join_tables); + if (reopt_result == REOPT_OLD_PLAN) + restore_query_plan(&save_keyuse, save_best_positions, + save_join_tab_keyuse, save_join_tab_checked_keys); + else if (reopt_result == REOPT_ERROR) return TRUE; - in_exists_reoptimized= true; - get_partial_join_cost(inner_join, inner_join->tables, - &inner_read_time_2, &inner_record_count_2); + inner_join->get_partial_join_cost(inner_join->tables, + &inner_read_time_2, &inner_record_count_2); } else { @@ -3765,8 +3767,8 @@ bool JOIN::choose_subquery_plan(table_map join_tables) if (in_subs->in_strategy & SUBS_MATERIALIZATION) { - /* Restore orginal query plan used for materialization. */ - if (in_exists_reoptimized) + /* Restore the original query plan used for materialization. */ + if (reopt_result == REOPT_NEW_PLAN) restore_query_plan(&save_keyuse, save_best_positions, save_join_tab_keyuse, save_join_tab_checked_keys); @@ -3792,14 +3794,11 @@ bool JOIN::choose_subquery_plan(table_map join_tables) } else if (in_subs->in_strategy & SUBS_IN_TO_EXISTS) { - /* Keep the new query plan with injected conditions, delete the old one. */ - if (save_keyuse.elements) - { - DBUG_ASSERT(in_exists_reoptimized); + /* Keep the new query plan with injected conditions, delete the old plan. */ + if (reopt_result == REOPT_NEW_PLAN) delete_dynamic(&save_keyuse); - } - if (!in_exists_reoptimized && in_to_exists_where && const_tables != tables) + if (reopt_result == REOPT_NONE && in_to_exists_where && const_tables != tables) { /* The subquery was not reoptimized either because the user allowed only the @@ -3811,7 +3810,8 @@ bool JOIN::choose_subquery_plan(table_map join_tables) join_tab[i].keyuse= NULL; join_tab[i].checked_keys.clear_all(); } - if (reoptimize(in_to_exists_where, join_tables, NULL)) + if ((reopt_result= reoptimize(in_to_exists_where, join_tables)) == + REOPT_ERROR) return TRUE; } diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 6f9c02b77ec..56d6309897c 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -229,8 +229,6 @@ static bool update_sum_func(Item_sum **func); static void select_describe(JOIN *join, bool need_tmp_table,bool need_order, bool distinct, const char *message=NullS); static void add_group_and_distinct_keys(JOIN *join, JOIN_TAB *join_tab); -void get_partial_join_cost(JOIN *join, uint idx, double *read_time_arg, - double *record_count_arg); static uint make_join_orderinfo(JOIN *join); static int join_read_record_no_init(JOIN_TAB *tab); @@ -5422,40 +5420,43 @@ greedy_search(JOIN *join, } -/* - Calculate a cost of given partial join order +/** + Calculate a cost of given partial join order in join->positions. - SYNOPSIS - get_partial_join_cost() - join IN Join to use. join->positions holds the - partial join order - idx IN # tables in the partial join order - read_time_arg OUT Store read time here - record_count_arg OUT Store record count here + @param n_tables[in] # tables in the partial join order after the last + constant table + @param read_time_arg[out] store read time here + @param record_count_arg[out] store record count here - DESCRIPTION - - This is needed for semi-join materialization code. The idea is that - we detect sj-materialization after we've put all sj-inner tables into - the join prefix + @note + When used by semi-join materialization code the idea is that we + detect sj-materialization after we've put all sj-inner tables into + the join prefix. prefix-tables semi-join-inner-tables tN ^--we're here and we'll need to get the cost of prefix-tables prefix again. + + When used with non-flattened subqueries, the method computes the + total cost of query plan. + + @returns + read_time_arg and record_count_arg contain the computed cost. */ -void get_partial_join_cost(JOIN *join, uint n_tables, double *read_time_arg, - double *record_count_arg) +void JOIN::get_partial_join_cost(uint n_tables, + double *read_time_arg, double *record_count_arg) { double record_count= 1; double read_time= 0.0; - for (uint i= join->const_tables; i < n_tables + join->const_tables ; i++) + + for (uint i= const_tables; i < n_tables; i++) { - if (join->best_positions[i].records_read) + if (best_positions[i].records_read) { - record_count *= join->best_positions[i].records_read; - read_time += join->best_positions[i].read_time; + record_count *= best_positions[i].records_read; + read_time += best_positions[i].read_time; } } *read_time_arg= read_time;// + record_count / TIME_FOR_COMPARE; @@ -19331,12 +19332,12 @@ void JOIN::restore_query_plan(DYNAMIC_ARRAY *save_keyuse, 4. Re-sort and re-filter the JOIN::keyuse array with the newly added KEYUSE elements. - @retval 0 OK - @retval 1 memory allocation error + @retval REOPT_NEW_PLAN there is a new plan. + @retval REOPT_OLD_PLAN no new improved plan was produced, use the old one. + @retval REOPT_ERROR an irrecovarable error occured during reoptimization. */ -int JOIN::reoptimize(Item *added_where, table_map join_tables, - POSITION *save_best_positions) +JOIN::enum_reopt_result JOIN::reoptimize(Item *added_where, table_map join_tables) { DYNAMIC_ARRAY added_keyuse; SARGABLE_PARAM *sargables= 0; /* Used only as a dummy parameter. */ @@ -19346,25 +19347,18 @@ int JOIN::reoptimize(Item *added_where, table_map join_tables, ~outer_join, select_lex, &sargables)) { delete_dynamic(&added_keyuse); - return 1; + return REOPT_ERROR; } if (!added_keyuse.elements) - { - /* No need to optimize if no new access methods were discovered. */ - if (save_best_positions) - memcpy((uchar*) best_positions, (uchar*) save_best_positions, - sizeof(POSITION) * (tables + 1)); - delete_dynamic(&added_keyuse); - return 0; - } + return REOPT_OLD_PLAN; /* Add the new access methods to the keyuse array. */ if (!keyuse.buffer && my_init_dynamic_array(&keyuse, sizeof(KEYUSE), 20, 64)) { delete_dynamic(&added_keyuse); - return 1; + return REOPT_ERROR; } allocate_dynamic(&keyuse, keyuse.elements + added_keyuse.elements); memcpy(keyuse.buffer + keyuse.elements * keyuse.size_of_element, @@ -19374,14 +19368,14 @@ int JOIN::reoptimize(Item *added_where, table_map join_tables, delete_dynamic(&added_keyuse); if (sort_and_filter_keyuse(&keyuse)) - return 1; + return REOPT_ERROR; optimize_keyuse(this, &keyuse); /* Re-run the join optimizer to compute a new query plan. */ if (choose_plan(this, join_tables)) - return 1; + return REOPT_ERROR; - return 0; + return REOPT_NEW_PLAN; } /** diff --git a/sql/sql_select.h b/sql/sql_select.h index 0bb8c9e9285..14af63f5dd1 100644 --- a/sql/sql_select.h +++ b/sql/sql_select.h @@ -1374,9 +1374,16 @@ private: JOIN& operator=(const JOIN &rhs); /**< not implemented */ protected: + /* Results of reoptimizing a JOIN via JOIN::reoptimize(). */ + enum enum_reopt_result { + REOPT_NEW_PLAN, /* there is a new reoptimized plan */ + REOPT_OLD_PLAN, /* no new improved plan can be found, use the old one */ + REOPT_ERROR, /* an irrecovarable error occured during reoptimization */ + REOPT_NONE /* not yet reoptimized */ + }; + /* Support for plan reoptimization with rewritten conditions. */ - int reoptimize(Item *added_where, table_map join_tables, - POSITION *save_best_positions); + enum_reopt_result reoptimize(Item *added_where, table_map join_tables); int save_query_plan(DYNAMIC_ARRAY *save_keyuse, POSITION *save_positions, KEYUSE **save_join_tab_keyuse, key_map *save_join_tab_checked_keys); @@ -1762,6 +1769,9 @@ public: } bool setup_subquery_caches(); bool choose_subquery_plan(table_map join_tables); + void get_partial_join_cost(uint n_tables, + double *read_time_arg, double *record_count_arg); + private: /** TRUE if the query contains an aggregate function but has no GROUP @@ -1993,8 +2003,6 @@ inline Item * and_items(Item* cond, Item *item) return (cond? (new Item_cond_and(cond, item)) : item); } bool choose_plan(JOIN *join,table_map join_tables); -void get_partial_join_cost(JOIN *join, uint n_tables, double *read_time_arg, - double *record_count_arg); void optimize_wo_join_buffering(JOIN *join, uint first_tab, uint last_tab, table_map last_remaining_tables, bool first_alt, uint no_jbuf_before,