Hi Sergey, On 05/27/2015 11:23 AM, Sergey Vojtovich wrote:

Hi Alexander,

ok to push. I just wish to study this code a bit before we continue.

Thanks! I'm sending the next step, as agreed on IRC.

Thanks, Sergey

On Wed, May 27, 2015 at 08:03:57AM +0400, Alexander Barkov wrote:

...

Hi Sergey,

please review the next step for MDEV-7950.

It breaks get_mm_parts() into a virtual method in Item, so - replaces one virtual call item->type() to another virtual call, item->get_mm_tree(). - and also removes this virtual call for functype(), which used to distinguish between COND_AND_FUNC and COND_OR_FUNC:

...

- if (((Item_cond*) cond)->functype() == Item_func::COND_AND_FUNC)

The next patches will:

1. Split the code in Item::get_mm_tree() responsible for functions into methods in Item_func and its descendants: Item_func Item_func_like, to get rid of the select_optimize() call Item_func_between Item_func_in Item_equal

2. Remove virtual method Item_func::select_optimize(), as well as enum Item_func::optimize_type, because they won't be needed any more.

Thanks.

...

diff --git a/sql/item.h b/sql/item.h index 56b2edc..9cabbb1 100644 --- a/sql/item.h +++ b/sql/item.h @@ -62,6 +62,8 @@ class user_var_entry; class JOIN; struct KEY_FIELD; struct SARGABLE_PARAM; +class RANGE_OPT_PARAM; +class SEL_TREE;

static inline uint32 @@ -1147,6 +1149,15 @@ class Item: public Type_std_attributes { return; } + /* + Make a select tree for all keys in a condition or a condition part + @param param Context + @param cond_ptr[OUT] Store a replacement item here if the condition + can be simplified, e.g.: + WHERE part1 OR part2 OR part3 + with one of the partN evalutating to SEL_TREE::ALWAYS. + */ + virtual SEL_TREE *get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr); /* Checks whether the item is: - a simple equality (field=field_item or field=constant_item), or diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 204e6ed..f814666 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -1733,6 +1733,7 @@ class Item_cond :public Item_bool_func void add_key_fields(JOIN *join, KEY_FIELD **key_fields, uint *and_level, table_map usable_tables, SARGABLE_PARAM **sargables); + SEL_TREE *get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr); virtual void print(String *str, enum_query_type query_type); void split_sum_func(THD *thd, Item **ref_pointer_array, List<Item> &fields); friend int setup_conds(THD *thd, TABLE_LIST *tables, TABLE_LIST *leaves, @@ -2082,6 +2083,7 @@ class Item_cond_and :public Item_cond COND_EQUAL **cond_equal_ref); void add_key_fields(JOIN *join, KEY_FIELD **key_fields, uint *and_level, table_map usable_tables, SARGABLE_PARAM **sargables); + SEL_TREE *get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr); };

inline bool is_cond_and(Item *item) diff --git a/sql/opt_range.cc b/sql/opt_range.cc index 0f1fa40..d03e399 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -952,7 +952,6 @@ static SEL_TREE * get_mm_parts(RANGE_OPT_PARAM *param,COND *cond_func,Field *fie static SEL_ARG *get_mm_leaf(RANGE_OPT_PARAM *param,COND *cond_func,Field *field, KEY_PART *key_part, Item_func::Functype type,Item *value); -static SEL_TREE *get_mm_tree(RANGE_OPT_PARAM *param, Item **cond);

static bool is_key_scan_ror(PARAM *param, uint keynr, uint8 nparts); static ha_rows check_quick_select(PARAM *param, uint idx, bool index_only, @@ -3131,7 +3130,7 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use,

if (cond) { - if ((tree= get_mm_tree(¶m, &cond))) + if ((tree= cond->get_mm_tree(¶m, &cond))) { if (tree->type == SEL_TREE::IMPOSSIBLE) { @@ -3628,7 +3627,7 @@ bool calculate_cond_selectivity_for_table(THD *thd, TABLE *table, Item **cond)

thd->no_errors=1;

- tree= get_mm_tree(¶m, cond); + tree= cond[0]->get_mm_tree(¶m, cond);

if (!tree) goto free_alloc; @@ -4061,7 +4060,7 @@ bool prune_partitions(THD *thd, TABLE *table, Item *pprune_cond) SEL_TREE *tree; int res;

- tree= get_mm_tree(range_par, &pprune_cond); + tree= pprune_cond->get_mm_tree(range_par, &pprune_cond); if (!tree) goto all_used;

@@ -7967,104 +7966,106 @@ static SEL_TREE *get_full_func_mm_tree(RANGE_OPT_PARAM *param, NULL - Could not infer anything from condition cond. SEL_TREE with type=IMPOSSIBLE - condition can never be true. */ - -static SEL_TREE *get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr) +SEL_TREE *Item_cond_and::get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr) { - SEL_TREE *tree=0; - SEL_TREE *ftree= 0; - Item_field *field_item= 0; - bool inv= FALSE; - Item *value= 0; - Item *cond= *cond_ptr; - DBUG_ENTER("get_mm_tree"); - - if (cond->type() == Item::COND_ITEM) + DBUG_ENTER("Item_cond_and::get_mm_tree"); + SEL_TREE *tree= NULL; + List_iterator<Item> li(*argument_list()); + Item *item; + while ((item= li++)) { - List_iterator<Item> li(*((Item_cond*) cond)->argument_list()); + SEL_TREE *new_tree= li.ref()[0]->get_mm_tree(param,li.ref()); + if (param->statement_should_be_aborted()) + DBUG_RETURN(NULL); + tree= tree_and(param, tree, new_tree); + if (tree && tree->type == SEL_TREE::IMPOSSIBLE) + { + /* + Do not remove 'item' from 'cond'. We return a SEL_TREE::IMPOSSIBLE + and that is sufficient for the caller to see that the whole + condition is never true. + */ + break; + } + } + DBUG_RETURN(tree); +}

- if (((Item_cond*) cond)->functype() == Item_func::COND_AND_FUNC) + +SEL_TREE *Item_cond::get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr) +{ + DBUG_ENTER("Item_cond::get_mm_tree"); + List_iterator<Item> li(*argument_list()); + bool replace_cond= false; + Item *replacement_item= li++; + SEL_TREE *tree= li.ref()[0]->get_mm_tree(param, li.ref()); + if (param->statement_should_be_aborted()) + DBUG_RETURN(NULL); + if (tree) + { + if (tree->type == SEL_TREE::IMPOSSIBLE && + param->remove_false_where_parts) { - tree= NULL; - Item *item; - while ((item=li++)) - { - SEL_TREE *new_tree= get_mm_tree(param,li.ref()); - if (param->statement_should_be_aborted()) - DBUG_RETURN(NULL); - tree= tree_and(param,tree,new_tree); - if (tree && tree->type == SEL_TREE::IMPOSSIBLE) - { - /* - Do not remove 'item' from 'cond'. We return a SEL_TREE::IMPOSSIBLE - and that is sufficient for the caller to see that the whole - condition is never true. - */ - break; - } - } + /* See the other li.remove() call below */ + li.remove(); + if (argument_list()->elements <= 1) + replace_cond= true; } - else - { // COND OR - bool replace_cond= false; - Item *replacement_item= li++; - tree= get_mm_tree(param, li.ref()); - if (param->statement_should_be_aborted()) + + Item *item; + while ((item= li++)) + { + SEL_TREE *new_tree= li.ref()[0]->get_mm_tree(param, li.ref()); + if (new_tree == NULL || param->statement_should_be_aborted()) DBUG_RETURN(NULL); - if (tree) + tree= tree_or(param, tree, new_tree); + if (tree == NULL || tree->type == SEL_TREE::ALWAYS) { - if (tree->type == SEL_TREE::IMPOSSIBLE && - param->remove_false_where_parts) - { - /* See the other li.remove() call below */ - li.remove(); - if (((Item_cond*)cond)->argument_list()->elements <= 1) - replace_cond= true; - } - - Item *item; - while ((item=li++)) - { - SEL_TREE *new_tree=get_mm_tree(param,li.ref()); - if (new_tree == NULL || param->statement_should_be_aborted()) - DBUG_RETURN(NULL); - tree= tree_or(param,tree,new_tree); - if (tree == NULL || tree->type == SEL_TREE::ALWAYS) - { - replacement_item= *li.ref(); - break; - } + replacement_item= *li.ref(); + break; + }

- if (new_tree && new_tree->type == SEL_TREE::IMPOSSIBLE && - param->remove_false_where_parts) - { - /* - This is a condition in form + if (new_tree && new_tree->type == SEL_TREE::IMPOSSIBLE && + param->remove_false_where_parts) + { + /* + This is a condition in form

- cond = item1 OR ... OR item_i OR ... itemN + cond = item1 OR ... OR item_i OR ... itemN

- and item_i produces SEL_TREE(IMPOSSIBLE). We should remove item_i - from cond. This may cause 'cond' to become a degenerate, - one-way OR. In that case, we replace 'cond' with the remaining - item_i. - */ - li.remove(); - if (((Item_cond*)cond)->argument_list()->elements <= 1) - replace_cond= true; - } - else - replacement_item= *li.ref(); - } - - if (replace_cond) - *cond_ptr= replacement_item; + and item_i produces SEL_TREE(IMPOSSIBLE). We should remove item_i + from cond. This may cause 'cond' to become a degenerate, + one-way OR. In that case, we replace 'cond' with the remaining + item_i. + */ + li.remove(); + if (argument_list()->elements <= 1) + replace_cond= true; } + else + replacement_item= *li.ref(); } - DBUG_RETURN(tree); + + if (replace_cond) + *cond_ptr= replacement_item; } + DBUG_RETURN(tree); +} + + +SEL_TREE *Item::get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr) +{ + SEL_TREE *tree=0; + SEL_TREE *ftree= 0; + Item_field *field_item= 0; + bool inv= FALSE; + Item *value= 0; + DBUG_ENTER("Item::get_mm_tree"); + /* Here when simple cond */ - if (cond->const_item()) + if (const_item()) { - if (cond->is_expensive()) + if (is_expensive()) DBUG_RETURN(0); /* During the cond->val_int() evaluation we can come across a subselect @@ -8074,8 +8075,8 @@ static SEL_TREE *get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr) */ MEM_ROOT *tmp_root= param->mem_root; param->thd->mem_root= param->old_root; - tree= cond->val_int() ? new(tmp_root) SEL_TREE(SEL_TREE::ALWAYS) : - new(tmp_root) SEL_TREE(SEL_TREE::IMPOSSIBLE); + tree= val_int() ? new(tmp_root) SEL_TREE(SEL_TREE::ALWAYS) : + new(tmp_root) SEL_TREE(SEL_TREE::IMPOSSIBLE); param->thd->mem_root= tmp_root; DBUG_RETURN(tree); } @@ -8083,23 +8084,23 @@ static SEL_TREE *get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr) table_map ref_tables= 0; table_map param_comp= ~(param->prev_tables | param->read_tables | param->current_table); - if (cond->type() != Item::FUNC_ITEM) + if (type() != Item::FUNC_ITEM) { // Should be a field - ref_tables= cond->used_tables(); + ref_tables= used_tables(); if ((ref_tables & param->current_table) || (ref_tables & ~(param->prev_tables | param->read_tables))) DBUG_RETURN(0); DBUG_RETURN(new SEL_TREE(SEL_TREE::MAYBE)); }

- Item_func *cond_func= (Item_func*) cond; + Item_func *cond_func= (Item_func*) this; if (cond_func->functype() == Item_func::BETWEEN || cond_func->functype() == Item_func::IN_FUNC) inv= ((Item_func_opt_neg *) cond_func)->negated; else if (cond_func->select_optimize() == Item_func::OPTIMIZE_NONE) DBUG_RETURN(0);

- param->cond= cond; + param->cond= this;

switch (cond_func->functype()) { case Item_func::BETWEEN: @@ -8149,7 +8150,7 @@ static SEL_TREE *get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr) } case Item_func::MULT_EQUAL_FUNC: { - Item_equal *item_equal= (Item_equal *) cond; + Item_equal *item_equal= (Item_equal *) this; if (!(value= item_equal->get_const()) || value->is_expensive()) DBUG_RETURN(0); Item_equal_fields_iterator it(*item_equal); @@ -8160,7 +8161,7 @@ static SEL_TREE *get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr) Item_result cmp_type= field->cmp_type(); if (!((ref_tables | field->table->map) & param_comp)) { - tree= get_mm_parts(param, cond, field, Item_func::EQ_FUNC, + tree= get_mm_parts(param, this, field, Item_func::EQ_FUNC, value,cmp_type); ftree= !ftree ? tree : tree_and(param, ftree, tree); }