revision-id: 5e6a3a090998496e15ef8c46294b4e702a29684d (mariadb-10.3.6-128-g5e6a3a0) parent(s): 4de3fd4ea29be8ed9f9b6ad83de523c21ff8c9d7 author: Igor Babaev committer: Igor Babaev timestamp: 2019-02-19 02:44:27 -0800 message: MDEV-7486: Condition pushdown from HAVING into WHERE Optimized the code that removed multiple equalities pushed from HAVING into WHERE. Now this removal is postponed until all multiple equalities are eliminated in substitute_for_best_equal_field(). --- mysql-test/main/having.result | 2 +- mysql-test/main/having_cond_pushdown.result | 2 +- mysql-test/main/union.result | 2 +- sql/item.cc | 4 -- sql/item.h | 5 +- sql/item_cmpfunc.cc | 4 -- sql/item_cmpfunc.h | 19 +------ sql/opt_subselect.cc | 84 ----------------------------- sql/sql_lex.cc | 54 +++++++------------ sql/sql_select.cc | 23 ++++---- 10 files changed, 36 insertions(+), 163 deletions(-) diff --git a/mysql-test/main/having.result b/mysql-test/main/having.result index 18066c9..b1cd776 100644 --- a/mysql-test/main/having.result +++ b/mysql-test/main/having.result @@ -483,7 +483,7 @@ HAVING (table2.f2 = 8); id select_type table type possible_keys key key_len ref rows filtered Extra 1 SIMPLE NULL NULL NULL NULL NULL NULL NULL NULL Impossible WHERE noticed after reading const tables Warnings: -Note 1003 select `test`.`table1`.`f1` AS `f1`,7 AS `f2` from `test`.`t1` `table1` join `test`.`t1` `table2` where 0 group by `test`.`table1`.`f1`,7 having 1 +Note 1003 select `test`.`table1`.`f1` AS `f1`,7 AS `f2` from `test`.`t1` `table1` join `test`.`t1` `table2` where 0 group by `test`.`table1`.`f1`,7 having multiple equal(8, 7) DROP TABLE t1; # # Bug#52336 Segfault / crash in 5.1 copy_fields (param=0x9872980) at sql_select.cc:15355 diff --git a/mysql-test/main/having_cond_pushdown.result b/mysql-test/main/having_cond_pushdown.result index 1555638..9d2fbce 100644 --- a/mysql-test/main/having_cond_pushdown.result +++ b/mysql-test/main/having_cond_pushdown.result @@ -1596,7 +1596,7 @@ EXPLAIN "access_type": "ALL", "rows": 5, "filtered": 100, - "attached_condition": "t1.c = t1.a and t1.a > 1 or t1.a < 3" + "attached_condition": "t1.a > 1 and t1.c = t1.a or t1.a < 3" } } } diff --git a/mysql-test/main/union.result b/mysql-test/main/union.result index a7688a1..e975e0f 100644 --- a/mysql-test/main/union.result +++ b/mysql-test/main/union.result @@ -2332,7 +2332,7 @@ id select_type table type possible_keys key key_len ref rows filtered Extra 1 PRIMARY NULL NULL NULL NULL NULL NULL NULL NULL No tables used 2 UNION NULL NULL NULL NULL NULL NULL NULL NULL Impossible WHERE noticed after reading const tables Warnings: -Note 1003 /* select#1 */ select 1 AS `1`,2 AS `2` union all /* select#2 */ select 1 AS `i`,count(0) AS `COUNT(*)` from `test`.`t2` where 0 group by 1 having 1 +Note 1003 /* select#1 */ select 1 AS `1`,2 AS `2` union all /* select#2 */ select 1 AS `i`,count(0) AS `COUNT(*)` from `test`.`t2` where 0 group by 1 having multiple equal(10, `i`) DROP TABLE t1,t2; # # Start of 10.3 tests diff --git a/sql/item.cc b/sql/item.cc index e5b833b..a6b4402 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -7309,7 +7309,6 @@ Item *Item::build_pushable_cond(THD *thd, { List<Item> equalities; Item *new_cond= NULL; - Item_equal *item_equal= (Item_equal *)this; if (((Item_equal *)this)->create_pushable_equalities(thd, &equalities, checker, arg) || (equalities.elements == 0)) @@ -7325,9 +7324,6 @@ Item *Item::build_pushable_cond(THD *thd, equalities, &cond_value, false); - if (equalities.elements == - (item_equal->elements_count()-1) && item_equal->upper_levels) - item_equal->upper_levels->work_references--; return new_cond; } diff --git a/sql/item.h b/sql/item.h index 4feaebf..2691a69 100644 --- a/sql/item.h +++ b/sql/item.h @@ -151,8 +151,9 @@ bool mark_unsupported_function(const char *w1, const char *w2, #define NO_EXTRACTION_FL (1 << 6) #define FULL_EXTRACTION_FL (1 << 7) -#define SUBSTITUTION_FL (1 << 8) -#define EXTRACTION_MASK (NO_EXTRACTION_FL | FULL_EXTRACTION_FL) +#define DELETION_FL (1 << 8) +#define SUBSTITUTION_FL (1 << 9) +#define EXTRACTION_MASK (NO_EXTRACTION_FL | FULL_EXTRACTION_FL | DELETION_FL) extern const char *item_empty_name; diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index b4ff4ab..55a0625 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -6509,8 +6509,6 @@ Item_equal::Item_equal(THD *thd, Item_equal *item_equal): with_const= item_equal->with_const; cond_false= item_equal->cond_false; upper_levels= item_equal->upper_levels; - if (item_equal->upper_levels) - item_equal->upper_levels->increase_references(); } @@ -7353,8 +7351,6 @@ Item_equal::excl_dep_on_group_fields_for_having_pushdown(st_select_lex *sel) { if (item->excl_dep_on_group_fields_for_having_pushdown(sel)) { - if (upper_levels) - upper_levels->references--; set_extraction_flag(FULL_EXTRACTION_FL); return true; } diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index e85c8fe..ed9dc5f 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -3223,17 +3223,12 @@ class COND_EQUAL: public Sql_alloc COND_EQUAL *upper_levels; /* multiple equalities of upper and levels */ List<Item_equal> current_level; /* list of multiple equalities of the current and level */ - uint references; /* number of conditions that have - reference on this COND_EQUAL */ - uint work_references; /* same as references */ COND_EQUAL() { upper_levels= 0; - references= 0; - work_references= 0; } COND_EQUAL(Item_equal *item, MEM_ROOT *mem_root) - :upper_levels(0), references(0), work_references(0) + :upper_levels(0) { current_level.push_back(item, mem_root); } @@ -3241,8 +3236,6 @@ class COND_EQUAL: public Sql_alloc { max_members= cond_equal.max_members; upper_levels= cond_equal.upper_levels; - references= cond_equal.references; - work_references= cond_equal.work_references; if (cond_equal.current_level.is_empty()) current_level.empty(); else @@ -3252,16 +3245,6 @@ class COND_EQUAL: public Sql_alloc { return (current_level.elements == 0); } - void increase_references() - { - references++; - work_references++; - } - void clean_references() - { - references= 0; - work_references= 0; - } }; diff --git a/sql/opt_subselect.cc b/sql/opt_subselect.cc index 4f45d77..1db1fbc 100644 --- a/sql/opt_subselect.cc +++ b/sql/opt_subselect.cc @@ -5554,82 +5554,6 @@ int select_value_catcher::send_data(List<Item> &items) /** @brief - Set missing links on multiply equalities - - @param thd the thread handle - @param cond the condition to set links for - @param inherited path to all inherited multiple equality items - @param build_cond_equal flag to control if COND_EQUAL for AND-condition - should be built - - @details - The method traverses cond and set links for the upper COND_EQUAL levels - where needed. - If build_cond_equal is set to true it builds for each AND-level except the - external one COND_EQUAL. -*/ - -static -void set_cond_equal_links(THD *thd, Item *cond, COND_EQUAL *inherited, - bool build_cond_equal) -{ - if (cond->type() == Item::FUNC_ITEM && - ((Item_func*) cond)->functype() == Item_func::MULT_EQUAL_FUNC) - { - ((Item_equal *)cond)->upper_levels= inherited; - if (inherited) - inherited->increase_references(); - } - - if (cond->type() != Item::COND_ITEM) - return; - - List_iterator<Item> it(*((Item_cond *)cond)->argument_list()); - Item *item; - while ((item=it++)) - { - if (item->type() != Item::COND_ITEM || - ((Item_cond*) item)->functype() != Item_func::COND_AND_FUNC) - { - set_cond_equal_links(thd, item, inherited, build_cond_equal); - continue; - } - Item_cond_and *and_item= (Item_cond_and *)item; - if (build_cond_equal) - { - COND_EQUAL new_cond_equal; - List_iterator<Item> li(*and_item->argument_list()); - Item *elem; - - while ((elem=li++)) - { - if (elem->type() == Item::FUNC_ITEM && - ((Item_func*) elem)->functype() == Item_func::MULT_EQUAL_FUNC) - { - if (new_cond_equal.current_level.push_back((Item_equal *)elem, - thd->mem_root)) - return; - li.remove(); - } - } - List<Item> *equal_list= - (List<Item> *)&and_item->m_cond_equal.current_level; - and_item->m_cond_equal.copy(new_cond_equal); - and_item->argument_list()->append(equal_list); - } - and_item->m_cond_equal.upper_levels= inherited; - and_item->m_cond_equal.clean_references(); - if (inherited) - inherited->increase_references(); - - set_cond_equal_links(thd, item, &and_item->m_cond_equal, - build_cond_equal); - } -} - - -/** - @brief Conjunct conditions after optimize_cond() call @param thd the thread handle @@ -5658,7 +5582,6 @@ Item *and_new_conditions_to_optimized_cond(THD *thd, Item *cond, bool build_cond_equal) { COND_EQUAL new_cond_equal; - COND_EQUAL *inherited= 0; Item *item; Item_equal *equality; bool is_simplified_cond= false; @@ -5731,7 +5654,6 @@ Item *and_new_conditions_to_optimized_cond(THD *thd, Item *cond, and_args->append((List<Item> *) cond_equalities); *cond_eq= &((Item_cond_and *) cond)->m_cond_equal; - inherited= &((Item_cond_and *)cond)->m_cond_equal; propagate_new_equalities(thd, cond, cond_equalities, cond_equal->upper_levels, @@ -5815,7 +5737,6 @@ Item *and_new_conditions_to_optimized_cond(THD *thd, Item *cond, and_cond->m_cond_equal.copy(new_cond_equal); cond= (Item *)and_cond; *cond_eq= &((Item_cond_and *)cond)->m_cond_equal; - inherited= &((Item_cond_and *)cond)->m_cond_equal; } else { @@ -5840,11 +5761,6 @@ Item *and_new_conditions_to_optimized_cond(THD *thd, Item *cond, if (is_simplified_cond) cond= cond->remove_eq_conds(thd, cond_value, true); - if (cond) - { - set_cond_equal_links(thd, cond, inherited, build_cond_equal); - } - return cond; } diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index c71754b..c4c1419 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -7759,8 +7759,8 @@ void binlog_unsafe_map_init() @param thd The thread handle @details - This method looks through the fields that are used in the GROUP BY of this - st_select_lex and saves onfo on these fields. + This method looks through the fields that are used in the GROUP BY of this + st_select_lex and saves info on these fields. */ void st_select_lex::collect_grouping_fields_for_derived(THD *thd, @@ -7874,9 +7874,6 @@ st_select_lex::check_cond_extraction_for_grouping_fields(THD *thd, Item *cond, cond->set_extraction_flag(NO_EXTRACTION_FL); if (count_full == arg_list->elements) { - if (and_cond != 0 && !and_cond->m_cond_equal.is_empty() && - and_cond->m_cond_equal.upper_levels) - and_cond->m_cond_equal.upper_levels->work_references--; cond->set_extraction_flag(FULL_EXTRACTION_FL); } if (cond->get_extraction_flag() != 0) @@ -9969,9 +9966,12 @@ bool cleanup_condition_pushed_from_having(THD *thd, Item *cond) As in the pushdown from HAVING into WHERE conditions are not just cloned so they can be later pushed down as it is for pushdown into materialized derived tables/views or IN subqueries, but also should be removed from - the HAVING clause there comes a problem with multiple equalities removal. - It is solved with the removal from multiple equalities list 'm_cond_equal' - of 'cond' conditions that are marked with the FULL_EXTRACTION_FLAG flag. + the HAVING clause. + The multiple equalities of the HAVING clause are not removed in this + function, but rather marked as to be removed later. Their removal is + done in substitute_for_best_equal_field() called for HAVING at the moment + when all multiple equalities referencing the top level multiple equalities + have been already eliminated. @retval condition without removed subformulas @@ -9983,6 +9983,12 @@ Item *remove_pushed_top_conjuncts_for_having(THD *thd, Item *cond) if (cond->get_extraction_flag() == FULL_EXTRACTION_FL) { cond->clear_extraction_flag(); + if (cond->type() == Item::FUNC_ITEM && + ((Item_func*) cond)->functype() == Item_func::MULT_EQUAL_FUNC) + { + cond->set_extraction_flag(DELETION_FL); + return cond; + } return 0; } if (cond->type() != Item::COND_ITEM) @@ -9991,32 +9997,6 @@ Item *remove_pushed_top_conjuncts_for_having(THD *thd, Item *cond) if (((Item_cond*) cond)->functype() == Item_func::COND_AND_FUNC) { List<Item> *cond_arg_list= ((Item_cond_and *)cond)->argument_list(); - List<Item_equal> *cond_equalities= - &((Item_cond_and*) cond)->m_cond_equal.current_level; - cond_arg_list->disjoin((List<Item> *) cond_equalities); - List_iterator<Item_equal> it(*cond_equalities); - Item_equal *eq_item; - - if (((Item_cond_and*) cond)->m_cond_equal.work_references == 0) - { - while ((eq_item= it++)) - { - if (eq_item->get_extraction_flag() == FULL_EXTRACTION_FL) - { - eq_item->clear_extraction_flag(); - it.remove(); - } - } - ((Item_cond_and*) cond)->m_cond_equal.clean_references(); - } - else - { - while ((eq_item= it++)) - eq_item->clear_extraction_flag(); - ((Item_cond_and*) cond)->m_cond_equal.work_references= - ((Item_cond_and*) cond)->m_cond_equal.references; - } - cond_arg_list->append((List<Item> *) cond_equalities); List_iterator<Item> li(*cond_arg_list); Item *item; while ((item= li++)) @@ -10024,7 +10004,11 @@ Item *remove_pushed_top_conjuncts_for_having(THD *thd, Item *cond) if (item->get_extraction_flag() == FULL_EXTRACTION_FL) { item->clear_extraction_flag(); - li.remove(); + if (item->type() == Item::FUNC_ITEM && + ((Item_func*) item)->functype() == Item_func::MULT_EQUAL_FUNC) + item->set_extraction_flag(DELETION_FL); + else + li.remove(); } } switch (cond_arg_list->elements) diff --git a/sql/sql_select.cc b/sql/sql_select.cc index c598532..94968cc 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -2290,7 +2290,8 @@ int JOIN::optimize_stage2() DBUG_PRINT("error",("Error from substitute_for_best_equal")); DBUG_RETURN(1); } - having->update_used_tables(); + if (having) + having->update_used_tables(); DBUG_EXECUTE("having", print_where(having, "after substitute_best_equal", @@ -5210,11 +5211,7 @@ make_join_statistics(JOIN *join, List<TABLE_LIST> &tables_list, { if (*s->on_expr_ref && s->cond_equal && s->cond_equal->upper_levels == orig_cond_equal) - { s->cond_equal->upper_levels= join->cond_equal; - if (s->cond_equal->upper_levels) - s->cond_equal->upper_levels->references++; - } } } } @@ -14775,8 +14772,6 @@ COND *Item_func_eq::build_equal_items(THD *thd, set_if_bigger(thd->lex->current_select->max_equal_elems, item_equal->n_field_items()); item_equal->upper_levels= inherited; - if (inherited) - inherited->increase_references(); if (cond_equal_ref) *cond_equal_ref= new (thd->mem_root) COND_EQUAL(item_equal, thd->mem_root); @@ -14811,8 +14806,6 @@ COND *Item_func_eq::build_equal_items(THD *thd, and_cond->update_used_tables(); if (cond_equal_ref) *cond_equal_ref= &and_cond->m_cond_equal; - if (inherited) - inherited->increase_references(); return and_cond; } } @@ -14938,8 +14931,6 @@ static COND *build_equal_items(JOIN *join, COND *cond, if (*cond_equal_ref) { (*cond_equal_ref)->upper_levels= inherited; - if (inherited) - inherited->increase_references(); inherited= *cond_equal_ref; } } @@ -15175,7 +15166,7 @@ Item *eliminate_item_equal(THD *thd, COND *cond, COND_EQUAL *upper_levels, ((Item_func *) cond)->functype() == Item_func::EQ_FUNC) || (cond->type() == Item::COND_ITEM && ((Item_func *) cond)->functype() == Item_func::COND_AND_FUNC)); - + /* Pick the "head" item: the constant one or the first in the join order (if the first in the join order happends to be inside an SJM nest, that's @@ -15442,8 +15433,12 @@ static COND* substitute_for_best_equal_field(THD *thd, JOIN_TAB *context_tab, COND *eq_cond= 0; List_iterator_fast<Item_equal> it(cond_equal->current_level); bool false_eq_cond= FALSE; + bool all_deleted= true; while ((item_equal= it++)) { + if (item_equal->get_extraction_flag() == DELETION_FL) + continue; + all_deleted= false; eq_cond= eliminate_item_equal(thd, eq_cond, cond_equal->upper_levels, item_equal); if (!eq_cond) @@ -15482,7 +15477,7 @@ static COND* substitute_for_best_equal_field(THD *thd, JOIN_TAB *context_tab, } } } - if (!eq_cond) + if (!eq_cond && !all_deleted) { /* We are out of memory doing the transformation. @@ -15501,6 +15496,8 @@ static COND* substitute_for_best_equal_field(THD *thd, JOIN_TAB *context_tab, cond_equal= item_equal->upper_levels; if (cond_equal && cond_equal->current_level.head() == item_equal) cond_equal= cond_equal->upper_levels; + if (item_equal->get_extraction_flag() == DELETION_FL) + return 0; cond= eliminate_item_equal(thd, 0, cond_equal, item_equal); return cond ? cond : org_cond; }