Sanja, These are some general notes about the patch: - Change Item::add_if_unique() to a method of base_list, and the List template class. - There is no need in subselect_engine::nest_level() because a subquery predicate is always at the same nest level irrespective of the chosen engine. The class Item_subselect has a member Item_subselect::unit, which should be sufficient to check the nest_level of the subquery. IMO this will simplify the patch visibly. I am pretty sure that if you add an assert, you will see that any engine will return the same nest_level as Item_subselect::unit->first_select(). - The commit comment is hard to understand. - Please first describe what was the problem, why there was a crash, and what is the generic problem with the current implementation. - Then describe what is the solution. - Fix the English of the commit comment. There are few more comments below marked with "timour:". Timour === modified file 'mysql-test/r/subselect_cache.result' --- a/mysql-test/r/subselect_cache.result 2011-02-03 15:00:28 +0000 +++ b/mysql-test/r/subselect_cache.result 2011-07-19 09:15:34 +0000 @@ -3272,6 +3272,7 @@ FROM t2 ) AND table1 .`col_varchar_key` col_varchar_nokey drop table t1,t2; set @@optimizer_switch= default; +set optimizer_switch='subquery_cache=on'; # LP BUG#615378 (incorrect NULL result returning in Item_cache) CREATE TABLE `t1` ( `pk` int(11) NOT NULL AUTO_INCREMENT, @@ -3310,3 +3311,25 @@ GROUP BY field3 HAVING (field3 <= 'h' AND field2 != 4) ; field2 field3 drop tables t1, t2, t3; +# +# Aggregate function in parameters test +# +CREATE TABLE t1 ( a INT, b INT, c INT, KEY (a, b)); +INSERT INTO t1 VALUES +( 1, 1, 1 ), +( 1, 2, 2 ), +( 1, 3, 3 ), +( 1, 4, 6 ), +( 1, 5, 5 ), +( 1, 9, 13 ), +( 2, 1, 6 ), +( 2, 2, 7 ), +( 2, 3, 8 ); +SELECT a, AVG(t1.b), +(SELECT t11.c FROM t1 t11 WHERE t11.a = t1.a AND t11.b = AVG(t1.b)) AS t11c +FROM t1 GROUP BY a; +a AVG(t1.b) t11c +1 4.0000 6 +2 2.0000 7 +DROP TABLE t1; +set @@optimizer_switch= default; === modified file 'mysql-test/r/subselect_scache.result' --- a/mysql-test/r/subselect_scache.result 2011-07-15 08:36:36 +0000 +++ b/mysql-test/r/subselect_scache.result 2011-07-19 09:15:34 +0000 @@ -232,7 +232,7 @@ id select_type table type possible_keys 3 DEPENDENT SUBQUERY t3 ALL NULL NULL NULL NULL 3 100.00 Using where Warnings: Note 1276 Field or reference 'test.t4.a' of SELECT #3 was resolved in SELECT #1 -Note 1003 select `test`.`t4`.`b` AS `b`,(select avg((`test`.`t2`.`a` + (select min(`test`.`t3`.`a`) from `test`.`t3` where (`test`.`t3`.`a` >= `test`.`t4`.`a`)))) from `test`.`t2`) AS `(select avg(t2.a+(select min(t3.a) from t3 where t3.a >= t4.a)) from t2)` from `test`.`t4` +Note 1003 select `test`.`t4`.`b` AS `b`,<expr_cache><`test`.`t4`.`a`>((select avg((`test`.`t2`.`a` + (select min(`test`.`t3`.`a`) from `test`.`t3` where (`test`.`t3`.`a` >= `test`.`t4`.`a`)))) from `test`.`t2`)) AS `(select avg(t2.a+(select min(t3.a) from t3 where t3.a >= t4.a)) from t2)` from `test`.`t4` select * from t3 where exists (select * from t2 where t2.b=t3.a); a 7 @@ -2835,7 +2835,7 @@ id select_type table type possible_keys 1 PRIMARY t1 ALL NULL NULL NULL NULL 8 100.00 2 DEPENDENT SUBQUERY t2 ALL NULL NULL NULL NULL 9 100.00 Using where Warnings: -Note 1003 select `test`.`t1`.`one` AS `one`,`test`.`t1`.`two` AS `two`,<expr_cache><`test`.`t1`.`two`,`test`.`t1`.`one`>(<in_optimizer>((`test`.`t1`.`one`,`test`.`t1`.`two`),<exists>(select `test`.`t2`.`one`,`test`.`t2`.`two` from `test`.`t2` where ((`test`.`t2`.`flag` = '0') and trigcond(((<cache>(`test`.`t1`.`one`) = `test`.`t2`.`one`) or isnull(`test`.`t2`.`one`))) and trigcond(((<cache>(`test`.`t1`.`two`) = `test`.`t2`.`two`) or isnull(`test`.`t2`.`two`)))) having (trigcond(<is_not_null_test>(`test`.`t2`.`one`)) and trigcond(<is_not_null_test>(`test`.`t2`.`two`)))))) AS `test` from `test`.`t1` +Note 1003 select `test`.`t1`.`one` AS `one`,`test`.`t1`.`two` AS `two`,<expr_cache><`test`.`t1`.`one`,`test`.`t1`.`two`>(<in_optimizer>((`test`.`t1`.`one`,`test`.`t1`.`two`),<exists>(select `test`.`t2`.`one`,`test`.`t2`.`two` from `test`.`t2` where ((`test`.`t2`.`flag` = '0') and trigcond(((<cache>(`test`.`t1`.`one`) = `test`.`t2`.`one`) or isnull(`test`.`t2`.`one`))) and trigcond(((<cache>(`test`.`t1`.`two`) = `test`.`t2`.`two`) or isnull(`test`.`t2`.`two`)))) having (trigcond(<is_not_null_test>(`test`.`t2`.`one`)) and trigcond(<is_not_null_test>(`test`.`t2`.`two`)))))) AS `test` from `test`.`t1` explain extended SELECT one,two from t1 where ROW(one,two) IN (SELECT one,two FROM t2 WHERE flag = 'N'); id select_type table type possible_keys key key_len ref rows filtered Extra 1 PRIMARY t1 ALL NULL NULL NULL NULL 8 100.00 @@ -2847,7 +2847,7 @@ id select_type table type possible_keys 1 PRIMARY t1 ALL NULL NULL NULL NULL 8 100.00 2 DEPENDENT SUBQUERY t2 ALL NULL NULL NULL NULL 9 100.00 Using where; Using temporary Warnings: -Note 1003 select `test`.`t1`.`one` AS `one`,`test`.`t1`.`two` AS `two`,<expr_cache><`test`.`t1`.`two`,`test`.`t1`.`one`>(<in_optimizer>((`test`.`t1`.`one`,`test`.`t1`.`two`),<exists>(select `test`.`t2`.`one`,`test`.`t2`.`two` from `test`.`t2` where (`test`.`t2`.`flag` = '0') group by `test`.`t2`.`one`,`test`.`t2`.`two` having (trigcond(((<cache>(`test`.`t1`.`one`) = `test`.`t2`.`one`) or isnull(`test`.`t2`.`one`))) and trigcond(((<cache>(`test`.`t1`.`two`) = `test`.`t2`.`two`) or isnull(`test`.`t2`.`two`))) and trigcond(<is_not_null_test>(`test`.`t2`.`one`)) and trigcond(<is_not_null_test>(`test`.`t2`.`two`)))))) AS `test` from `test`.`t1` +Note 1003 select `test`.`t1`.`one` AS `one`,`test`.`t1`.`two` AS `two`,<expr_cache><`test`.`t1`.`one`,`test`.`t1`.`two`>(<in_optimizer>((`test`.`t1`.`one`,`test`.`t1`.`two`),<exists>(select `test`.`t2`.`one`,`test`.`t2`.`two` from `test`.`t2` where (`test`.`t2`.`flag` = '0') group by `test`.`t2`.`one`,`test`.`t2`.`two` having (trigcond(((<cache>(`test`.`t1`.`one`) = `test`.`t2`.`one`) or isnull(`test`.`t2`.`one`))) and trigcond(((<cache>(`test`.`t1`.`two`) = `test`.`t2`.`two`) or isnull(`test`.`t2`.`two`))) and trigcond(<is_not_null_test>(`test`.`t2`.`one`)) and trigcond(<is_not_null_test>(`test`.`t2`.`two`)))))) AS `test` from `test`.`t1` DROP TABLE t1,t2; CREATE TABLE t1 (a char(5), b char(5)); INSERT INTO t1 VALUES (NULL,'aaa'), ('aaa','aaa'); === modified file 'mysql-test/t/subselect_cache.test' --- a/mysql-test/t/subselect_cache.test 2010-11-28 13:02:12 +0000 +++ b/mysql-test/t/subselect_cache.test 2011-07-19 09:15:34 +0000 @@ -1567,6 +1567,7 @@ FROM t2 ) AND table1 .`col_varchar_key` drop table t1,t2; set @@optimizer_switch= default; +set optimizer_switch='subquery_cache=on'; # --echo # LP BUG#615378 (incorrect NULL result returning in Item_cache) # @@ -1614,3 +1615,6 @@ GROUP BY field3 HAVING (field3 <= 'h' AND field2 != 4) ; --enable_warnings drop tables t1, t2, t3; + +--echo # +--echo # Aggregate function in parameters test timour: Change the above to: "Test aggregate functions as parameters to ... (to what?)" +--echo # + +CREATE TABLE t1 ( a INT, b INT, c INT, KEY (a, b)); + +INSERT INTO t1 VALUES + ( 1, 1, 1 ), + ( 1, 2, 2 ), + ( 1, 3, 3 ), + ( 1, 4, 6 ), + ( 1, 5, 5 ), + ( 1, 9, 13 ), + + ( 2, 1, 6 ), + ( 2, 2, 7 ), + ( 2, 3, 8 ); + +SELECT a, AVG(t1.b), +(SELECT t11.c FROM t1 t11 WHERE t11.a = t1.a AND t11.b = AVG(t1.b)) AS t11c +FROM t1 GROUP BY a; + +DROP TABLE t1; + + +set @@optimizer_switch= default; === modified file 'sql/item.cc' --- a/sql/item.cc 2011-07-09 09:47:41 +0000 +++ b/sql/item.cc 2011-07-19 09:15:34 +0000 @@ -651,14 +651,14 @@ Item* Item::transform(Item_transformer t A pointer to created wrapper item if successful, NULL - otherwise */ -Item* Item::set_expr_cache(THD *thd, List<Item *> &depends_on) +Item* Item::set_expr_cache(THD *thd) { DBUG_ENTER("Item::set_expr_cache"); Item_cache_wrapper *wrapper; if ((wrapper= new Item_cache_wrapper(this)) && !wrapper->fix_fields(thd, (Item**)&wrapper)) { - if (wrapper->set_cache(thd, depends_on)) + if (wrapper->set_cache(thd)) DBUG_RETURN(NULL); DBUG_RETURN(wrapper); } @@ -666,6 +666,26 @@ Item* Item::set_expr_cache(THD *thd, Lis } +/** + Add this item to the items list if there is no such item in the list + + @param list The list of the Items to add to +*/ + timour: This method implements a set, which is a very generic functionality. Please change this into a method of base_list and List template class. +void Item::add_if_unique(List<Item> &list) +{ + List_iterator_fast<Item> li(list); + Item *item; + while ((item= li++)) + { + if (eq(item, FALSE)) + return; + } + list.push_back(this); + return; +} + + Item_ident::Item_ident(Name_resolution_context *context_arg, const char *db_name_arg,const char *table_name_arg, const char *field_name_arg) @@ -742,6 +762,15 @@ bool Item_ident::remove_dependence_proce } +bool Item_ident::collect_outer_ref_processor(uchar *param) +{ + Collect_deps_prm *prm= (Collect_deps_prm *)param; + if (depended_from && depended_from->nest_level < prm->nest_level) + add_if_unique(*prm->parameters); + return FALSE; +} + + /** Store the pointer to this item field into a list if not already there. @@ -4357,9 +4386,6 @@ Item_field::fix_outer_field(THD *thd, Fi ((ref_type == REF_ITEM || ref_type == FIELD_ITEM) ? (Item_ident*) (*reference) : 0)); - context->select_lex-> - register_dependency_item(last_checked_context->select_lex, - reference); /* A reference to a view field had been found and we substituted it instead of this Item (find_field_in_tables @@ -4460,9 +4486,6 @@ Item_field::fix_outer_field(THD *thd, Fi mark_as_dependent(thd, last_checked_context->select_lex, context->select_lex, rf, rf); - context->select_lex-> - register_dependency_item(last_checked_context->select_lex, - reference); return 0; } @@ -4471,9 +4494,6 @@ Item_field::fix_outer_field(THD *thd, Fi mark_as_dependent(thd, last_checked_context->select_lex, context->select_lex, this, (Item_ident*)*reference); - context->select_lex-> - register_dependency_item(last_checked_context->select_lex, - reference); if (last_checked_context->select_lex->having_fix_field) { Item_ref *rf; @@ -6304,9 +6324,6 @@ bool Item_ref::fix_fields(THD *thd, Item refer_type == FIELD_ITEM) ? (Item_ident*) (*reference) : 0)); - context->select_lex-> - register_dependency_item(last_checked_context->select_lex, - reference); /* view reference found, we substituted it instead of this Item, so can quit @@ -6357,9 +6374,6 @@ bool Item_ref::fix_fields(THD *thd, Item thd->change_item_tree(reference, fld); mark_as_dependent(thd, last_checked_context->select_lex, thd->lex->current_select, fld, fld); - context->select_lex-> - register_dependency_item(last_checked_context->select_lex, - reference); /* A reference is resolved to a nest level that's outer or the same as the nest level of the enclosing set function : adjust the value of @@ -6383,9 +6397,6 @@ bool Item_ref::fix_fields(THD *thd, Item DBUG_ASSERT(*ref && (*ref)->fixed); mark_as_dependent(thd, last_checked_context->select_lex, context->select_lex, this, this); - context->select_lex-> - register_dependency_item(last_checked_context->select_lex, - reference); /* A reference is resolved to a nest level that's outer or the same as the nest level of the enclosing set function : adjust the value of @@ -6400,12 +6411,6 @@ bool Item_ref::fix_fields(THD *thd, Item } else if (ref_type() != VIEW_REF) { - if (depended_from && reference) - { - DBUG_ASSERT(context->select_lex != get_depended_from()); - context->select_lex->register_dependency_item(get_depended_from(), - reference); - } /* It could be that we're referring to something that's in ancestor selects. We must make an appropriate mark_as_dependent() call for each such @@ -6901,6 +6906,7 @@ Item_cache_wrapper::~Item_cache_wrapper( } + Item_cache_wrapper::Item_cache_wrapper(Item *item_arg) :orig_item(item_arg), expr_cache(NULL), expr_value(NULL) { @@ -6914,6 +6920,7 @@ Item_cache_wrapper::Item_cache_wrapper(I unsigned_flag= orig_item->unsigned_flag; name= item_arg->name; name_length= item_arg->name_length; + with_subselect= orig_item->with_subselect; if ((expr_value= Item_cache::get_cache(orig_item))) expr_value->setup(orig_item); @@ -6922,11 +6929,28 @@ Item_cache_wrapper::Item_cache_wrapper(I } +/** + Initialize the cache if it is needed +*/ + timour: It seems that the cache is not initialized "if needed". Insted it is intialized on-demand, if not yet initialized. Isn't it so? If yes, please fix the comment. I also don't like the method name - better something like init_on_demand(). +void Item_cache_wrapper::check_initialization() +{ + if (!expr_cache->is_inited()) + { + orig_item->get_cache_parameters(parameters); + expr_cache->init(); + } +} + + void Item_cache_wrapper::print(String *str, enum_query_type query_type) { str->append(func_name()); if (expr_cache) + { + check_initialization(); expr_cache->print(str, query_type); + } else str->append(STRING_WITH_LEN("<<DISABLED>>")); str->append('('); @@ -6962,6 +6986,7 @@ void Item_cache_wrapper::cleanup() expr_cache= 0; /* expr_value is Item so it will be destroyed from list of Items */ expr_value= 0; + parameters.empty(); DBUG_VOID_RETURN; } @@ -6982,11 +7007,11 @@ void Item_cache_wrapper::cleanup() @retval TRUE Error */ -bool Item_cache_wrapper::set_cache(THD *thd, List<Item*> &depends_on) +bool Item_cache_wrapper::set_cache(THD *thd) { DBUG_ENTER("Item_cache_wrapper::set_cache"); DBUG_ASSERT(expr_cache == 0); - expr_cache= new Expression_cache_tmptable(thd, depends_on, expr_value); + expr_cache= new Expression_cache_tmptable(thd, parameters, expr_value); DBUG_RETURN(expr_cache == NULL); } @@ -7011,6 +7036,7 @@ Item *Item_cache_wrapper::check_cache() { Expression_cache_tmptable::result res; Item *cached_value; + check_initialization(); res= expr_cache->check_value(&cached_value); if (res == Expression_cache_tmptable::HIT) DBUG_RETURN(cached_value); === modified file 'sql/item.h' --- a/sql/item.h 2011-07-03 21:59:01 +0000 +++ b/sql/item.h 2011-07-19 09:15:34 +0000 @@ -1155,6 +1155,15 @@ public: { return FALSE; } + struct Collect_deps_prm + { + int nest_level; + List<Item> *parameters; + }; + /** + Collect outer references + */ + virtual bool collect_outer_ref_processor(uchar *arg) {return FALSE; } /** Find a function of a given type @@ -1249,7 +1258,7 @@ public: { return Field::GEOM_GEOMETRY; }; String *check_well_formed_result(String *str, bool send_error= 0); bool eq_by_collation(Item *item, bool binary_cmp, CHARSET_INFO *cs); - Item* set_expr_cache(THD *thd, List<Item*> &depends_on); + Item* set_expr_cache(THD *thd); virtual Item *get_cached_item() { return NULL; } virtual Item_equal *get_item_equal() { return NULL; } @@ -1273,6 +1282,23 @@ public: walk(&Item::view_used_tables_processor, 0, (uchar *) view); return view->view_used_tables; } + + /** + Collect and add to the list cache parameters for this Item. + + @note Now implemented only for subqueries and in_optimizer, + if we need it for general function then this method should + be defined for Item_func. + */ + virtual void get_cache_parameters(List<Item> ¶meters) { }; + + /** + Add this item to the items list if there is no such item in the list + + @param list The list of the Items to add to + */ + + void add_if_unique(List<Item> &list); }; @@ -1677,6 +1703,10 @@ public: virtual void print(String *str, enum_query_type query_type); virtual bool change_context_processor(uchar *cntx) { context= (Name_resolution_context *)cntx; return FALSE; } + /** + Collect outer references + */ + virtual bool collect_outer_ref_processor(uchar *arg); friend bool insert_fields(THD *thd, Name_resolution_context *context, const char *db_name, const char *table_name, List_iterator<Item> *it, @@ -2739,8 +2769,11 @@ private: */ Item_cache *expr_value; + List<Item> parameters; + Item *check_cache(); - inline void cache(); + void cache(); + void check_initialization(); public: Item_cache_wrapper(Item *item_arg); @@ -2750,7 +2783,7 @@ public: enum Type type() const { return EXPR_CACHE_ITEM; } enum Type real_type() const { return orig_item->type(); } - bool set_cache(THD *thd, List<Item*> &depends_on); + bool set_cache(THD *thd); bool fix_fields(THD *thd, Item **it); void fix_length_and_dec() {} @@ -2832,6 +2865,9 @@ public: if (result_type() == ROW_RESULT) orig_item->bring_value(); } + virtual bool is_expensive() { return orig_item->is_expensive(); } + bool is_expensive_processor(uchar *arg) + { return orig_item->is_expensive_processor(arg); } bool check_vcol_func_processor(uchar *arg) { return trace_unsupported_by_check_vcol_func_processor("cache"); === modified file 'sql/item_cmpfunc.cc' --- a/sql/item_cmpfunc.cc 2011-07-11 19:48:35 +0000 +++ b/sql/item_cmpfunc.cc 2011-07-19 09:15:34 +0000 @@ -1464,32 +1464,40 @@ Item *Item_in_optimizer::expr_cache_inse DBUG_ENTER("Item_in_optimizer::expr_cache_insert_transformer"); if (args[1]->type() != Item::SUBSELECT_ITEM) DBUG_RETURN(this); // MAX/MIN transformed => do nothing - List<Item*> &depends_on= ((Item_subselect *)args[1])->depends_on; if (expr_cache) DBUG_RETURN(expr_cache); + if (args[1]->expr_cache_is_needed(thd) && + (expr_cache= set_expr_cache(thd))) + DBUG_RETURN(expr_cache); + + DBUG_RETURN(this); +} + + +/** + Collect and add to the list cache parameters for this Item. + + @param parameters The list where to add parameters +*/ + +void Item_in_optimizer::get_cache_parameters(List<Item> ¶meters) +{ /* Add left expression to the list of the parameters of the subquery */ if (args[0]->cols() == 1) - depends_on.push_front((Item**)args); + args[0]->add_if_unique(parameters); else { for (uint i= 0; i < args[0]->cols(); i++) { - depends_on.push_front(args[0]->addr(i)); + args[0]->element_index(i)->add_if_unique(parameters); } } - - if (args[1]->expr_cache_is_needed(thd) && - (expr_cache= set_expr_cache(thd, depends_on))) - DBUG_RETURN(expr_cache); - - /* no cache => return list in original state just to be safe */ - for (uint i= 0; i < args[0]->cols(); i++) - depends_on.pop(); - DBUG_RETURN(this); + args[1]->get_cache_parameters(parameters); } + /* The implementation of optimized \<outer expression\> [NOT] IN \<subquery\> predicates. The implementation works as follows. === modified file 'sql/item_cmpfunc.h' --- a/sql/item_cmpfunc.h 2011-07-13 18:05:33 +0000 +++ b/sql/item_cmpfunc.h 2011-07-19 09:15:34 +0000 @@ -259,6 +259,7 @@ public: bool is_expensive(); void set_join_tab_idx(uint join_tab_idx_arg) { args[1]->set_join_tab_idx(join_tab_idx_arg); } + virtual void get_cache_parameters(List<Item> ¶meters); }; class Comp_creator === modified file 'sql/item_subselect.cc' --- a/sql/item_subselect.cc 2011-07-18 20:45:38 +0000 +++ b/sql/item_subselect.cc 2011-07-19 09:15:34 +0000 @@ -128,7 +128,6 @@ void Item_subselect::cleanup() } if (engine) engine->cleanup(); - depends_on.empty(); reset(); value_assigned= 0; expr_cache= 0; @@ -520,6 +519,11 @@ bool Item_subselect::walk(Item_processor { for (SELECT_LEX *lex= unit->first_select(); lex; lex= lex->next_select()) { + /* + List_iterator<Item> li(lex->join ? + lex->join->all_fields : + lex->item_list); + */ List_iterator<Item> li(lex->item_list); Item *item; ORDER *order; @@ -583,6 +587,12 @@ bool Item_subselect::exec() } +void Item_subselect::get_cache_parameters(List<Item> ¶meters) +{ + Collect_deps_prm prm= { engine->nest_level(), ¶meters }; + walk(&Item::collect_outer_ref_processor, TRUE, (uchar*)&prm); +} + int Item_in_subselect::optimize(double *out_rows, double *cost) { int res; @@ -647,7 +657,7 @@ int Item_in_subselect::optimize(double * bool Item_subselect::expr_cache_is_needed(THD *thd) { - return (depends_on.elements && + return ((engine->uncacheable() & UNCACHEABLE_DEPENDENT) && engine->cols() == 1 && optimizer_flag(thd, OPTIMIZER_SWITCH_SUBQUERY_CACHE) && !(engine->uncacheable() & (UNCACHEABLE_RAND | @@ -675,8 +685,7 @@ bool Item_subselect::expr_cache_is_neede bool Item_in_subselect::expr_cache_is_needed(THD *thd) { - return (depends_on.elements && - optimizer_flag(thd, OPTIMIZER_SWITCH_SUBQUERY_CACHE) && + return (optimizer_flag(thd, OPTIMIZER_SWITCH_SUBQUERY_CACHE) && !(engine->uncacheable() & (UNCACHEABLE_RAND | UNCACHEABLE_SIDEEFFECT))); } @@ -1009,7 +1018,7 @@ Item* Item_singlerow_subselect::expr_cac DBUG_RETURN(expr_cache); if (expr_cache_is_needed(thd) && - (expr_cache= set_expr_cache(thd, depends_on))) + (expr_cache= set_expr_cache(thd))) DBUG_RETURN(expr_cache); DBUG_RETURN(this); } @@ -1270,7 +1279,7 @@ Item* Item_exists_subselect::expr_cache_ DBUG_RETURN(expr_cache); if (substype() == EXISTS_SUBS && expr_cache_is_needed(thd) && - (expr_cache= set_expr_cache(thd, depends_on))) + (expr_cache= set_expr_cache(thd))) DBUG_RETURN(expr_cache); DBUG_RETURN(this); } @@ -4263,7 +4272,9 @@ subselect_hash_sj_engine::make_unique_en tab->ref.tmp_table_index_lookup_init(thd, tmp_key, it, FALSE); DBUG_RETURN(new subselect_uniquesubquery_engine(thd, tab, item, - semi_join_conds)); + semi_join_conds, + materialize_engine-> + nest_level())); } @@ -5725,3 +5736,15 @@ end: void subselect_table_scan_engine::cleanup() { } + timour: Make the two methods below inline. + +int subselect_single_select_engine::nest_level() +{ + return select_lex->nest_level; +}; + + +int subselect_union_engine::nest_level() +{ + return unit->first_select()->nest_level; +}; === modified file 'sql/item_subselect.h' --- a/sql/item_subselect.h 2011-07-18 20:45:38 +0000 +++ b/sql/item_subselect.h 2011-07-19 09:15:34 +0000 @@ -102,14 +102,6 @@ public: List<Ref_to_outside> upper_refs; st_select_lex *parent_select; - /** - List of references on items subquery depends on (externally resolved); - - @note We can't store direct links on Items because it could be - substituted with other item (for example for grouping). - */ - List<Item*> depends_on; - /* TRUE<=>Table Elimination has made it redundant to evaluate this select (and so it is not part of QEP, etc) @@ -225,6 +217,7 @@ public: @retval FALSE otherwise */ bool is_expensive_processor(uchar *arg) { return TRUE; } + /** Get the SELECT_LEX structure associated with this Item. @return the SELECT_LEX structure associated with this Item @@ -232,6 +225,7 @@ public: st_select_lex* get_select_lex(); const char *func_name() const { DBUG_ASSERT(0); return "subselect"; } virtual bool expr_cache_is_needed(THD *); + virtual void get_cache_parameters(List<Item> ¶meters); friend class select_result_interceptor; friend class Item_in_optimizer; @@ -646,6 +640,7 @@ public: virtual bool no_rows() = 0; virtual enum_engine_type engine_type() { return ABSTRACT_ENGINE; } virtual int get_identifier() { DBUG_ASSERT(0); return 0; } + virtual int nest_level()= 0; protected: void set_row(List<Item> &item_list, Item_cache **row); }; @@ -679,6 +674,7 @@ public: bool no_rows(); virtual enum_engine_type engine_type() { return SINGLE_SELECT_ENGINE; } int get_identifier(); + virtual int nest_level(); friend class subselect_hash_sj_engine; friend class Item_in_subselect; @@ -708,6 +704,7 @@ public: bool is_executed() const; bool no_rows(); virtual enum_engine_type engine_type() { return UNION_ENGINE; } + virtual int nest_level(); }; @@ -736,6 +733,7 @@ class subselect_uniquesubquery_engine: p protected: st_join_table *tab; Item *cond; /* The WHERE condition of subselect */ + int save_nest_level; /* TRUE<=> last execution produced empty set. Valid only when left expression is NULL. @@ -746,8 +744,9 @@ public: // constructor can assign THD because it will be called after JOIN::prepare subselect_uniquesubquery_engine(THD *thd_arg, st_join_table *tab_arg, - Item_subselect *subs, Item *where) - :subselect_engine(thd_arg, subs, 0), tab(tab_arg), cond(where) + Item_subselect *subs, Item *where, int lvl) + :subselect_engine(thd_arg, subs, 0), tab(tab_arg), cond(where), + save_nest_level(lvl) {} ~subselect_uniquesubquery_engine(); void cleanup(); @@ -769,6 +768,7 @@ public: int copy_ref_key_simple(); /* TIMOUR: this method needs refactoring. */ bool no_rows() { return empty_result_set; } virtual enum_engine_type engine_type() { return UNIQUESUBQUERY_ENGINE; } + virtual int nest_level() { return save_nest_level;}; }; @@ -811,8 +811,8 @@ public: // constructor can assign THD because it will be called after JOIN::prepare subselect_indexsubquery_engine(THD *thd_arg, st_join_table *tab_arg, Item_subselect *subs, Item *where, - Item *having_arg, bool chk_null) - :subselect_uniquesubquery_engine(thd_arg, tab_arg, subs, where), + Item *having_arg, bool chk_null, int lvl) + :subselect_uniquesubquery_engine(thd_arg, tab_arg, subs, where, lvl), check_null(chk_null), having(having_arg) {} @@ -905,6 +905,8 @@ public: bool temp= FALSE); bool no_tables();//=>base class + virtual int nest_level() { return materialize_engine->nest_level();}; + protected: /* The engine used to compute the IN predicate. */ subselect_engine *lookup_engine; @@ -1185,6 +1187,7 @@ public: return !(((Item_in_subselect *) item)->null_value); } void print(String*, enum_query_type); + virtual int nest_level() { return lookup_engine->nest_level(); } friend void subselect_hash_sj_engine::cleanup(); }; === modified file 'sql/item_sum.cc' --- a/sql/item_sum.cc 2011-06-27 16:07:24 +0000 +++ b/sql/item_sum.cc 2011-07-19 09:15:34 +0000 @@ -319,7 +319,6 @@ bool Item_sum::register_sum_func(THD *th if (aggr_level >= 0) { ref_by= ref; - thd->lex->current_select->register_dependency_item(aggr_sel, ref); /* Add the object to the list of registered objects assigned to aggr_sel */ if (!aggr_sel->inner_sum_func_list) next= this; @@ -355,6 +354,16 @@ bool Item_sum::register_sum_func(THD *th return FALSE; } + +bool Item_sum::collect_outer_ref_processor(uchar *param) +{ + Collect_deps_prm *prm= (Collect_deps_prm *)param; + SELECT_LEX *ds; + if ((ds= depended_from()) && ds->nest_level < prm->nest_level) + add_if_unique(*prm->parameters); + return FALSE; +} + Item_sum::Item_sum(List<Item> &list) :arg_count(list.elements), forced_const(FALSE) === modified file 'sql/item_sum.h' --- a/sql/item_sum.h 2011-05-10 15:17:43 +0000 +++ b/sql/item_sum.h 2011-07-19 09:15:34 +0000 @@ -374,6 +374,7 @@ public: virtual Field *create_tmp_field(bool group, TABLE *table, uint convert_blob_length); bool walk(Item_processor processor, bool walk_subquery, uchar *argument); + virtual bool collect_outer_ref_processor(uchar *param); bool init_sum_func_check(THD *thd); bool check_sum_func(THD *thd, Item **ref); bool register_sum_func(THD *thd, Item **ref); === modified file 'sql/opt_range.cc' --- a/sql/opt_range.cc 2011-07-04 11:51:16 +0000 +++ b/sql/opt_range.cc 2011-07-19 09:15:34 +0000 @@ -11731,17 +11731,19 @@ check_group_min_max_predicates(Item *con Disallow loose index scan if the MIN/MAX argument field is referenced by a subquery in the WHERE clause. */ + if (cond_type == Item::SUBSELECT_ITEM) { Item_subselect *subs_cond= (Item_subselect*) cond; if (subs_cond->is_correlated) { - DBUG_ASSERT(subs_cond->depends_on.elements > 0); - List_iterator_fast<Item*> li(subs_cond->depends_on); - Item **dep; + DBUG_ASSERT(subs_cond->upper_refs.elements > 0); + List_iterator_fast<Item_subselect::Ref_to_outside> + li(subs_cond->upper_refs); + Item_subselect::Ref_to_outside *dep; while ((dep= li++)) { - if ((*dep)->eq(min_max_arg_item, FALSE)) + if (dep->item->eq(min_max_arg_item, FALSE)) DBUG_RETURN(FALSE); } } === modified file 'sql/opt_subselect.cc' --- a/sql/opt_subselect.cc 2011-07-19 07:45:46 +0000 +++ b/sql/opt_subselect.cc 2011-07-19 09:15:34 +0000 @@ -4167,7 +4167,10 @@ int rewrite_to_index_subquery_engine(JOI timour: Here you could introduce one variable 'nest_level' in the beginning of the method. subselect_uniquesubquery_engine(thd, join_tab, unit->item, - where))); + where, + join-> + select_lex-> + nest_level))); } else if (join_tab[0].type == JT_REF && join_tab[0].ref.items[0]->name == in_left_expr_name) @@ -4183,7 +4186,10 @@ int rewrite_to_index_subquery_engine(JOI unit->item, where, NULL, - 0))); + 0, + join-> + select_lex-> + nest_level))); } } else if (join_tab[0].type == JT_REF_OR_NULL && join_tab[0].ref.items[0]->name == in_left_expr_name && @@ -4199,7 +4205,10 @@ int rewrite_to_index_subquery_engine(JOI unit->item, join->conds, join->having, - 1))); + 1, + join-> + select_lex-> + nest_level))); } } === modified file 'sql/sql_base.cc' --- a/sql/sql_base.cc 2011-07-11 21:00:44 +0000 +++ b/sql/sql_base.cc 2011-07-19 09:15:34 +0000 @@ -6454,11 +6454,6 @@ find_field_in_tables(THD *thd, Item_iden { mark_select_range_as_dependent(thd, last_select, current_sel, found, *ref, item); - if (item->can_be_depended) - { - DBUG_ASSERT((*ref) == (Item*)item); - current_sel->register_dependency_item(last_select, ref); - } } } return found; === modified file 'sql/sql_class.h' --- a/sql/sql_class.h 2011-07-17 07:52:07 +0000 +++ b/sql/sql_class.h 2011-07-19 09:15:34 +0000 @@ -74,6 +74,22 @@ public: /** + Item iterator over List_iterator_fast for Items +*/ + +class Item_iterator_list: public Item_iterator +{ + List_iterator<Item> list; +public: + Item_iterator_list(List_iterator<Item> &arg_list): + list(arg_list) {} + void open() { list.rewind(); } + Item *next() { return (list++); } + void close() {} +}; + + +/** Item iterator over Item interface for rows */ === modified file 'sql/sql_expression_cache.cc' --- a/sql/sql_expression_cache.cc 2011-06-27 16:07:24 +0000 +++ b/sql/sql_expression_cache.cc 2011-07-19 09:15:34 +0000 @@ -23,9 +23,9 @@ ulong subquery_cache_miss, subquery_cache_hit; Expression_cache_tmptable::Expression_cache_tmptable(THD *thd, - List<Item*> &dependants, - Item *value) - :cache_table(NULL), table_thd(thd), list(&dependants), val(value), + List<Item> &dependants, + Item *value) + :cache_table(NULL), table_thd(thd), items(dependants), val(value), inited (0) { DBUG_ENTER("Expression_cache_tmptable::Expression_cache_tmptable"); @@ -60,50 +60,32 @@ static uint field_enumerator(uchar *arg) void Expression_cache_tmptable::init() { - List_iterator<Item*> li(*list); - Item_iterator_ref_list it(li); - Item **item; + List_iterator<Item> li(items); + Item_iterator_list it(li); uint field_counter; DBUG_ENTER("Expression_cache_tmptable::init"); DBUG_ASSERT(!inited); inited= TRUE; cache_table= NULL; - while ((item= li++)) - { - DBUG_ASSERT(item); - if (*item) - { - DBUG_ASSERT((*item)->fixed); - items.push_back((*item)); - } - else - { - /* - This is possible when optimizer already executed this subquery and - optimized out the condition predicate. - */ - li.remove(); - } - } - - if (list->elements == 0) + if (items.elements == 0) { DBUG_PRINT("info", ("All parameters were removed by optimizer.")); DBUG_VOID_RETURN; } + /* add result field */ + items.push_front(val); + cache_table_param.init(); /* dependent items and result */ - cache_table_param.field_count= list->elements + 1; + cache_table_param.field_count= items.elements; /* postpone table creation to index description */ cache_table_param.skip_create_table= 1; - items.push_front(val); - if (!(cache_table= create_tmp_table(table_thd, &cache_table_param, items, (ORDER*) NULL, - FALSE, FALSE, + FALSE, TRUE, ((table_thd->options | TMP_TABLE_ALL_COLUMNS) & ~(OPTION_BIG_TABLES | @@ -122,16 +104,13 @@ void Expression_cache_tmptable::init() goto error; } - /* This list do not contain result field */ - it.open(); - - field_counter=1; + field_counter= 1; if (cache_table->alloc_keys(1) || cache_table->add_tmp_key(0, items.elements - 1, &field_enumerator, (uchar*)&field_counter, TRUE) || ref.tmp_table_index_lookup_init(table_thd, cache_table->key_info, it, - TRUE)) + TRUE, 1 /* skip result field*/)) { DBUG_PRINT("error", ("creating index failed")); goto error; @@ -192,13 +171,6 @@ Expression_cache::result Expression_cach int res; DBUG_ENTER("Expression_cache_tmptable::check_value"); - /* - We defer cache initialization to get item references that are - used at the execution phase. - */ - if (!inited) - init(); - if (cache_table) { DBUG_PRINT("info", ("status: %u has_record %u", @@ -274,16 +246,17 @@ err: void Expression_cache_tmptable::print(String *str, enum_query_type query_type) { - List_iterator<Item*> li(*list); - Item **item; + List_iterator<Item> li(items); + Item *item; bool is_first= TRUE; str->append('<'); + li++; // skip result field while ((item= li++)) { if (!is_first) str->append(','); - (*item)->print(str, query_type); + item->print(str, query_type); is_first= FALSE; } str->append('>'); === modified file 'sql/sql_expression_cache.h' --- a/sql/sql_expression_cache.h 2010-10-27 03:03:59 +0000 +++ b/sql/sql_expression_cache.h 2011-07-19 09:15:34 +0000 @@ -37,6 +37,15 @@ public: Print cache parameters */ virtual void print(String *str, enum_query_type query_type)= 0; + + /** + Is this cache initialized + */ + virtual bool is_inited()= 0; + /** + Initialize this cache + */ + virtual void init()= 0; }; struct st_table_ref; @@ -51,15 +60,16 @@ class Item_field; class Expression_cache_tmptable :public Expression_cache { public: - Expression_cache_tmptable(THD *thd, List<Item*> &dependants, Item *value); + Expression_cache_tmptable(THD *thd, List<Item> &dependants, Item *value); virtual ~Expression_cache_tmptable(); virtual result check_value(Item **value); virtual my_bool put_value(Item *value); void print(String *str, enum_query_type query_type); + bool is_inited() { return inited; }; + void init(); private: - void init(); /* tmp table parameters */ TMP_TABLE_PARAM cache_table_param; @@ -71,10 +81,8 @@ private: struct st_table_ref ref; /* Cached result */ Item_field *cached_result; - /* List of references to the parameters of the expression */ - List<Item*> *list; - /* List of items */ - List<Item> items; + /* List of parameter items */ + List<Item> &items; /* Value Item example */ Item *val; /* Set on if the object has been succesfully initialized with init() */ === modified file 'sql/sql_lex.cc' --- a/sql/sql_lex.cc 2011-07-19 03:05:33 +0000 +++ b/sql/sql_lex.cc 2011-07-19 09:15:34 +0000 @@ -1879,59 +1879,6 @@ void st_select_lex_unit::exclude_tree() } -/** - Register reference to an item which the subqueries depends on - - @param def_sel select against which the item is resolved - @param dependency reference to the item - - @details - This function puts the reference dependency to an item that is either an - outer field or an aggregate function resolved against an outer select into - the list 'depends_on'. It adds it to the 'depends_on' lists for each - subquery between this one and 'def_sel' - the subquery against which the - item is resolved. -*/ - -void st_select_lex::register_dependency_item(st_select_lex *def_sel, - Item **dependency) -{ - SELECT_LEX *s= this; - DBUG_ENTER("st_select_lex::register_dependency_item"); - DBUG_ASSERT(this != def_sel); - DBUG_ASSERT(*dependency); - //psergey-add-stupid: - while (def_sel->merged_into) - def_sel= def_sel->merged_into; - //:eof - do - { - /* check duplicates */ - List_iterator_fast<Item*> li(s->master_unit()->item->depends_on); - Item **dep; - while ((dep= li++)) - { - if ((*dep)->eq(*dependency, FALSE)) - { - DBUG_PRINT("info", ("dependency %s already present", - ((*dependency)->name ? - (*dependency)->name : - "<no name>"))); - DBUG_VOID_RETURN; - } - } - - s->master_unit()->item->depends_on.push_back(dependency); - DBUG_PRINT("info", ("depends_on: Select: %d added: %s", - s->select_number, - ((*dependency)->name ? - (*dependency)->name : - "<no name>"))); - } while ((s= s->outer_select()) != def_sel); - DBUG_VOID_RETURN; -} - - /* st_select_lex_node::mark_as_dependent mark all st_select_lex struct from this to 'last' as dependent === modified file 'sql/sql_lex.h' --- a/sql/sql_lex.h 2011-07-17 06:57:43 +0000 +++ b/sql/sql_lex.h 2011-07-19 09:15:34 +0000 @@ -793,7 +793,6 @@ public: inline bool is_subquery_function() { return master_unit()->item != 0; } bool mark_as_dependent(THD *thd, st_select_lex *last, Item *dependency); - void register_dependency_item(st_select_lex *last, Item **dependency); bool set_braces(bool value); bool inc_in_sum_expr(); === modified file 'sql/sql_select.cc' --- a/sql/sql_select.cc 2011-07-18 06:12:31 +0000 +++ b/sql/sql_select.cc 2011-07-19 09:15:34 +0000 @@ -9613,6 +9613,7 @@ bool JOIN_TAB::preread_init() @param thd Thread handle @param tmp_key The temporary table key @param it The iterator of items for lookup in the key + @param skip Number of fields from the beginning to skip @details Build TABLE_REF object for lookup in the key 'tmp_key' using items @@ -9625,9 +9626,11 @@ bool JOIN_TAB::preread_init() bool TABLE_REF::tmp_table_index_lookup_init(THD *thd, KEY *tmp_key, Item_iterator &it, - bool value) + bool value, + uint skip) { uint tmp_key_parts= tmp_key->key_parts; + uint i; DBUG_ENTER("TABLE_REF::tmp_table_index_lookup_init"); key= 0; /* The only temp table index. */ @@ -9648,7 +9651,8 @@ bool TABLE_REF::tmp_table_index_lookup_i uchar *cur_ref_buff= key_buff; it.open(); - for (uint i= 0; i < tmp_key_parts; i++, cur_key_part++, ref_key++) + for (i= 0; i < skip; i++) it.next(); + for (i= 0; i < tmp_key_parts; i++, cur_key_part++, ref_key++) { Item *item= it.next(); DBUG_ASSERT(item); === modified file 'sql/sql_select.h' --- a/sql/sql_select.h 2011-07-17 07:52:07 +0000 +++ b/sql/sql_select.h 2011-07-19 09:15:34 +0000 @@ -135,7 +135,7 @@ typedef struct st_table_ref bool disable_cache; bool tmp_table_index_lookup_init(THD *thd, KEY *tmp_key, Item_iterator &it, - bool value); + bool value, uint skip= 0); } TABLE_REF;