revision-id: d13f3a43c0fffd0daffbaf564760b73d6228b171 (mariadb-10.4.10-33-gd13f3a4) parent(s): ed355f59dd7e0065ebde15223c2f39f8b71b2958 author: Igor Babaev committer: Igor Babaev timestamp: 2020-01-03 11:15:00 -0800 message: MDEV-21184 Assertion `used_tables_cache == 0' failed in Item_func::fix_fields with condition_pushdown_from_having This bug could manifest itself for queries with GROUP BY and HAVING clauses when the HAVING clause was a conjunctive condition that depended exclusively on grouping fields and at least one conjunct contained an equality of the form fld=sq where fld is a grouping field and sq is a constant subquery. In this case the optimizer tries to perform a pushdown of the HAVING condition into WHERE. To construct the pushable condition the optimizer first transforms all multiple equalities in HAVING into simple equalities. This has to be done for a proper processing of the pushed conditions in WHERE. The multiple equalities at all AND/OR levels must be converted to simple equalities because any multiple equality may refer to a multiple equality at the upper level. Before this patch the conversion was performed like this: multiple_equality(x,f1,...,fn) => x=f1 and ... and x=fn. When an equality item for x=fi was constructed both the items for x and fi were cloned. If x happened to be a constant subquery that could not be cloned the conversion failed. If the conversions of multiple equalities previously performed had succeeded then the whole condition became in an inconsistent state that could cause different failures. The solution provided by the patch is: 1. to use a different conversion rule if x is a constant multiple_equality(x,f1,...,fn) => f1=x and f2=f1 and ... and fn=f1 2. not to clone x if it's a constant. Such conversions cannot fail and besides the result of the conversion preserves the equivalence of f1,...,fn that can be used for other optimizations. This patch also made sure that expensive predicates are not pushed from HAVING to WHERE. --- mysql-test/main/derived_cond_pushdown.result | 8 +- mysql-test/main/having_cond_pushdown.result | 148 +++++++++++++++++++++++++++ mysql-test/main/having_cond_pushdown.test | 39 +++++++ sql/item.cc | 14 ++- sql/item.h | 7 +- sql/item_cmpfunc.cc | 105 +++++++++++-------- sql/item_cmpfunc.h | 3 +- sql/sql_lex.cc | 14 ++- 8 files changed, 280 insertions(+), 58 deletions(-) diff --git a/mysql-test/main/derived_cond_pushdown.result b/mysql-test/main/derived_cond_pushdown.result index c044b79..125de26 100644 --- a/mysql-test/main/derived_cond_pushdown.result +++ b/mysql-test/main/derived_cond_pushdown.result @@ -8937,13 +8937,13 @@ EXPLAIN "materialized": { "query_block": { "select_id": 2, - "having_condition": "t1.b = 1 and max_c > 37 and max_c > 30", + "having_condition": "max_c > 37 and max_c > 30", "table": { "table_name": "t1", "access_type": "ALL", "rows": 3, "filtered": 100, - "attached_condition": "t1.a = 1" + "attached_condition": "t1.a = 1 and t1.b = 1" } } } @@ -9012,13 +9012,13 @@ EXPLAIN "materialized": { "query_block": { "select_id": 2, - "having_condition": "t1.b = 1 and max_c > 37 and max_c > 30", + "having_condition": "max_c > 37 and max_c > 30", "table": { "table_name": "t1", "access_type": "ALL", "rows": 3, "filtered": 100, - "attached_condition": "t1.a = 1 and t1.d = 1" + "attached_condition": "t1.a = 1 and t1.b = 1 and t1.d = 1" } } } diff --git a/mysql-test/main/having_cond_pushdown.result b/mysql-test/main/having_cond_pushdown.result index 82a4813..9b12429 100644 --- a/mysql-test/main/having_cond_pushdown.result +++ b/mysql-test/main/having_cond_pushdown.result @@ -4776,3 +4776,151 @@ WHERE t1.a = 3 AND (t1.a < 2 AND t1.b > 3) GROUP BY t1.a; id select_type table type possible_keys key key_len ref rows Extra 1 SIMPLE NULL NULL NULL NULL NULL NULL NULL Impossible WHERE DROP TABLE t1; +# +# MDEV-21184: Constant subquery in condition movable to WHERE +# +CREATE TABLE t1(a int, b int); +INSERT INTO t1 VALUES +(1,10), (2,20), (1,11), (1,15), (2,20), (1,10), (2,21); +CREATE TABLE t2 (c INT); +INSERT INTO t2 VALUES (2),(3); +EXPLAIN FORMAT=JSON SELECT a FROM t1 GROUP BY a HAVING a = 8 OR a = ( SELECT MIN(c) FROM t2 ); +EXPLAIN +{ + "query_block": { + "select_id": 1, + "filesort": { + "sort_key": "t1.a", + "temporary_table": { + "table": { + "table_name": "t1", + "access_type": "ALL", + "rows": 7, + "filtered": 100, + "attached_condition": "t1.a = 8 or t1.a = (subquery#2)" + }, + "subqueries": [ + { + "query_block": { + "select_id": 2, + "table": { + "table_name": "t2", + "access_type": "ALL", + "rows": 2, + "filtered": 100 + } + } + } + ] + } + } + } +} +SELECT a FROM t1 GROUP BY a HAVING a = 8 OR a = ( SELECT MIN(c) FROM t2 ); +a +2 +EXPLAIN FORMAT=JSON SELECT a FROM t1 GROUP BY a,b +HAVING ( a = 8 OR a = ( SELECT MIN(c) FROM t2 ) ) and b < 20; +EXPLAIN +{ + "query_block": { + "select_id": 1, + "filesort": { + "sort_key": "t1.a, t1.b", + "temporary_table": { + "table": { + "table_name": "t1", + "access_type": "ALL", + "rows": 7, + "filtered": 100, + "attached_condition": "(t1.a = 8 or t1.a = (subquery#2)) and t1.b < 20" + }, + "subqueries": [ + { + "query_block": { + "select_id": 2, + "table": { + "table_name": "t2", + "access_type": "ALL", + "rows": 2, + "filtered": 100 + } + } + } + ] + } + } + } +} +SELECT a FROM t1 GROUP BY a,b +HAVING ( a = 8 OR a = ( SELECT MIN(c) FROM t2 ) ) and b < 20; +a +EXPLAIN FORMAT=JSON SELECT a FROM t1 GROUP BY a +HAVING ( a = 8 OR a = ( SELECT MIN(c) FROM t2 ) ) and SUM(b) > 20; +EXPLAIN +{ + "query_block": { + "select_id": 1, + "having_condition": "sum(t1.b) > 20", + "filesort": { + "sort_key": "t1.a", + "temporary_table": { + "table": { + "table_name": "t1", + "access_type": "ALL", + "rows": 7, + "filtered": 100, + "attached_condition": "t1.a = 8 or t1.a = (subquery#2)" + }, + "subqueries": [ + { + "query_block": { + "select_id": 2, + "table": { + "table_name": "t2", + "access_type": "ALL", + "rows": 2, + "filtered": 100 + } + } + } + ] + } + } + } +} +SELECT a FROM t1 GROUP BY a +HAVING ( a = 8 OR a = ( SELECT MIN(c) FROM t2 ) ) and SUM(b) > 20; +a +2 +EXPLAIN FORMAT=JSON SELECT a FROM t1 GROUP BY a HAVING a = ( SELECT MIN(c) FROM t2 ); +EXPLAIN +{ + "query_block": { + "select_id": 1, + "table": { + "table_name": "t1", + "access_type": "ALL", + "rows": 7, + "filtered": 100, + "attached_condition": "t1.a = (subquery#2)" + }, + "subqueries": [ + { + "query_block": { + "select_id": 2, + "table": { + "table_name": "t2", + "access_type": "ALL", + "rows": 2, + "filtered": 100 + } + } + } + ] + } +} +SELECT a FROM t1 GROUP BY a HAVING a = ( SELECT MIN(c) FROM t2 ); +a +2 +DROP TABLE t1,t2; diff --git a/mysql-test/main/having_cond_pushdown.test b/mysql-test/main/having_cond_pushdown.test index f1bf706..fc75122 100644 --- a/mysql-test/main/having_cond_pushdown.test +++ b/mysql-test/main/having_cond_pushdown.test @@ -1401,3 +1401,42 @@ EXPLAIN SELECT t1.a,MAX(t1.b),t1.c FROM t1 WHERE t1.a = 3 AND (t1.a < 2 AND t1.b > 3) GROUP BY t1.a; DROP TABLE t1; + +--echo # +--echo # MDEV-21184: Constant subquery in condition movable to WHERE +--echo # + +CREATE TABLE t1(a int, b int); +INSERT INTO t1 VALUES + (1,10), (2,20), (1,11), (1,15), (2,20), (1,10), (2,21); + +CREATE TABLE t2 (c INT); +INSERT INTO t2 VALUES (2),(3); + +let $q= +SELECT a FROM t1 GROUP BY a HAVING a = 8 OR a = ( SELECT MIN(c) FROM t2 ); + +eval EXPLAIN FORMAT=JSON $q; +eval $q; + +let $q= +SELECT a FROM t1 GROUP BY a,b + HAVING ( a = 8 OR a = ( SELECT MIN(c) FROM t2 ) ) and b < 20; + +eval EXPLAIN FORMAT=JSON $q; +eval $q; + +let $q= +SELECT a FROM t1 GROUP BY a + HAVING ( a = 8 OR a = ( SELECT MIN(c) FROM t2 ) ) and SUM(b) > 20; + +eval EXPLAIN FORMAT=JSON $q; +eval $q; + +let $q= +SELECT a FROM t1 GROUP BY a HAVING a = ( SELECT MIN(c) FROM t2 ); + +eval EXPLAIN FORMAT=JSON $q; +eval $q; + +DROP TABLE t1,t2; diff --git a/sql/item.cc b/sql/item.cc index 900a973..7b4571e 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -7352,7 +7352,7 @@ Item *Item::build_pushable_cond(THD *thd, List<Item> equalities; Item *new_cond= NULL; if (((Item_equal *)this)->create_pushable_equalities(thd, &equalities, - checker, arg) || + checker, arg, true) || (equalities.elements == 0)) return 0; @@ -10512,3 +10512,15 @@ void Item::register_in(THD *thd) next= thd->free_list; thd->free_list= this; } + + +bool Item::cleanup_excluding_immutables_processor (void *arg) +{ + if (!(get_extraction_flag() == IMMUTABLE_FL)) + return cleanup_processor(arg); + else + { + clear_extraction_flag(); + return false; + } +} diff --git a/sql/item.h b/sql/item.h index 2ac0964..205c070 100644 --- a/sql/item.h +++ b/sql/item.h @@ -152,8 +152,10 @@ bool mark_unsupported_function(const char *w1, const char *w2, #define NO_EXTRACTION_FL (1 << 6) #define FULL_EXTRACTION_FL (1 << 7) #define DELETION_FL (1 << 8) -#define SUBSTITUTION_FL (1 << 9) -#define EXTRACTION_MASK (NO_EXTRACTION_FL | FULL_EXTRACTION_FL | DELETION_FL) +#define IMMUTABLE_FL (1 << 9) +#define SUBSTITUTION_FL (1 << 10) +#define EXTRACTION_MASK \ + (NO_EXTRACTION_FL | FULL_EXTRACTION_FL | DELETION_FL | IMMUTABLE_FL) extern const char *item_empty_name; @@ -1867,6 +1869,7 @@ class Item: public Value_source, virtual bool cleanup_processor(void *arg); virtual bool cleanup_excluding_fields_processor (void *arg) { return cleanup_processor(arg); } + bool cleanup_excluding_immutables_processor (void *arg); virtual bool cleanup_excluding_const_fields_processor (void *arg) { return cleanup_processor(arg); } virtual bool collect_item_field_processor(void *arg) { return 0; } diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 9110f34..5ae5931 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -7410,6 +7410,7 @@ Item_equal::excl_dep_on_grouping_fields(st_select_lex *sel) of the tree of the object to check if multiple equality elements can be used to create equalities @param arg parameter to be passed to the checker + @param clone_const true <=> clone the constant member if there is any @details How the method works on examples: @@ -7420,36 +7421,31 @@ Item_equal::excl_dep_on_grouping_fields(st_select_lex *sel) Example 2: It takes MULT_EQ(1,a,b) and tries to create from its elements a set of - equalities {(1=a),(1=b)}. + equalities {(a=1),(a=b)}. How it is done: - 1. The method finds the left part of the equalities to be built. It will - be the same for all equalities. It is either: - a. A constant if there is any - b. A first element in the multiple equality that satisfies - checker function + 1. If there is a constant member c the first non-constant member x for + which the function checker returns true is taken and an item for + the equality x=c is created. When constructing the equality item + the left part of the equality is always taken as a clone of x while + the right part is taken as a clone of c only if clone_const == true. - For the example 1 the left element is field 'x'. - For the example 2 it is constant '1'. + 2. After this all equalities of the form x=a (where x designates the first + non-constant member for which checker returns true and a is some other + such member of the multiplle equality) are created. When constructing + an equality item both its parts are taken as clones of x and a. - 2. If the left element is found the rest elements of the multiple equality - are checked with the checker function if they can be right parts - of equalities. - If the element can be a right part of the equality, equality is built. - It is built with the left part element found at the step 1 and - the right part element found at this step (step 2). - - Suppose for the example above that both 'a' and 'b' fields can be used - to build equalities: + Suppose in the examples above that for 'x', 'a', and 'b' the function + checker returns true. Example 1: - for 'a' field (x=a) is built - for 'b' field (x=b) is built + the equality (x=a) is built + the equality (x=b) is built Example 2: - for 'a' field (1=a) is built - for 'b' field (1=b) is built + the equality (a=1) is built + the equality (a=b) is built 3. As a result we get a set of equalities built with the elements of this multiple equality. They are saved in the equality list. @@ -7458,15 +7454,17 @@ Item_equal::excl_dep_on_grouping_fields(st_select_lex *sel) {(x=a),(x=b)} Example 2: - {(1=a),(1=b)} + {(a=1),(a=b)} @note This method is called for condition pushdown into materialized derived table/view, and IN subquery, and pushdown from HAVING into WHERE. When it is called for pushdown from HAVING the empty checker is passed. - It happens because elements of this multiple equality don't need to be - checked if they can be used to build equalities. There are no elements - that can't be used to build equalities. + This is because in this case the elements of the multiple equality don't + need to be checked if they can be used to build equalities: either all + equalities can be pushed or none of them can be pushed. + When the function is called for pushdown from HAVING the value of the + parameter clone_const is always false. In other cases it's always true. @retval true if an error occurs @retval false otherwise @@ -7475,24 +7473,42 @@ Item_equal::excl_dep_on_grouping_fields(st_select_lex *sel) bool Item_equal::create_pushable_equalities(THD *thd, List<Item> *equalities, Pushdown_checker checker, - uchar *arg) + uchar *arg, + bool clone_const) { Item *item; + Item *left_item= NULL; + Item *right_item = get_const(); Item_equal_fields_iterator it(*this); - Item *left_item = get_const(); - if (!left_item) + + while ((item=it++)) { - while ((item=it++)) - { - left_item= item; - if (checker && !((item->*checker) (arg))) - continue; - break; - } + left_item= item; + if (checker && !((item->*checker) (arg))) + continue; + break; } + if (!left_item) return false; + if (right_item) + { + Item_func_eq *eq= 0; + Item *left_item_clone= left_item->build_clone(thd); + Item *right_item_clone= !clone_const ? + right_item : right_item->build_clone(thd); + if (!left_item_clone || !right_item_clone) + return true; + eq= new (thd->mem_root) Item_func_eq(thd, + left_item_clone, + right_item_clone); + if (!eq || equalities->push_back(eq, thd->mem_root)) + return true; + if (!clone_const) + right_item->set_extraction_flag(IMMUTABLE_FL); + } + while ((item=it++)) { if (checker && !((item->*checker) (arg))) @@ -7500,15 +7516,14 @@ bool Item_equal::create_pushable_equalities(THD *thd, Item_func_eq *eq= 0; Item *left_item_clone= left_item->build_clone(thd); Item *right_item_clone= item->build_clone(thd); - if (left_item_clone && right_item_clone) - { - left_item_clone->set_item_equal(NULL); - right_item_clone->set_item_equal(NULL); - eq= new (thd->mem_root) Item_func_eq(thd, - right_item_clone, - left_item_clone); - } - if (eq && equalities->push_back(eq, thd->mem_root)) + if (!(left_item_clone && right_item_clone)) + return true; + left_item_clone->set_item_equal(NULL); + right_item_clone->set_item_equal(NULL); + eq= new (thd->mem_root) Item_func_eq(thd, + right_item_clone, + left_item_clone); + if (!eq || equalities->push_back(eq, thd->mem_root)) return true; } return false; @@ -7533,7 +7548,7 @@ bool Item_equal::create_pushable_equalities(THD *thd, Item *Item_equal::multiple_equality_transformer(THD *thd, uchar *arg) { List<Item> equalities; - if (create_pushable_equalities(thd, &equalities, 0, 0)) + if (create_pushable_equalities(thd, &equalities, 0, 0, false)) return 0; switch (equalities.elements) diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 0a91d45..1d84ee6 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -3208,7 +3208,8 @@ class Item_equal: public Item_bool_func bool excl_dep_on_in_subq_left_part(Item_in_subselect *subq_pred); bool excl_dep_on_grouping_fields(st_select_lex *sel); bool create_pushable_equalities(THD *thd, List<Item> *equalities, - Pushdown_checker checker, uchar *arg); + Pushdown_checker checker, uchar *arg, + bool clone_const); /* Return the number of elements in this multiple equality */ uint elements_count() { return equal_items.elements; } friend class Item_equal_fields_iterator; diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 16bb53c..ea34679 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -7988,7 +7988,7 @@ st_select_lex::check_cond_extraction_for_grouping_fields(THD *thd, Item *cond) } else { - int fl= cond->excl_dep_on_grouping_fields(this) ? + int fl= cond->excl_dep_on_grouping_fields(this) && !cond->is_expensive() ? FULL_EXTRACTION_FL : NO_EXTRACTION_FL; cond->set_extraction_flag(fl); } @@ -9819,7 +9819,7 @@ st_select_lex::build_pushable_cond_for_having_pushdown(THD *thd, Item *cond) { List_iterator<Item> li(*((Item_cond*) result)->argument_list()); Item *item; - while ((item=li++)) + while ((item= li++)) { if (attach_to_conds.push_back(item, thd->mem_root)) return true; @@ -9839,8 +9839,13 @@ st_select_lex::build_pushable_cond_for_having_pushdown(THD *thd, Item *cond) */ if (cond->type() != Item::COND_ITEM) return false; + if (((Item_cond *)cond)->functype() != Item_cond::COND_AND_FUNC) { + /* + cond is not a conjunctive formula and it cannot be pushed into WHERE. + Try to extract a formula that can be pushed. + */ Item *fix= cond->build_pushable_cond(thd, 0, 0); if (!fix) return false; @@ -9860,7 +9865,6 @@ st_select_lex::build_pushable_cond_for_having_pushdown(THD *thd, Item *cond) Item *result= item->transform(thd, &Item::multiple_equality_transformer, (uchar *)item); - if (!result) return true; if (result->type() == Item::COND_ITEM && @@ -10188,8 +10192,8 @@ Item *st_select_lex::pushdown_from_having_into_where(THD *thd, Item *having) &Item::field_transformer_for_having_pushdown, (uchar *)this); - if (item->walk(&Item:: cleanup_processor, 0, STOP_PTR) || - item->fix_fields(thd, NULL)) + if (item->walk(&Item::cleanup_excluding_immutables_processor, 0, STOP_PTR) + || item->fix_fields(thd, NULL)) { attach_to_conds.empty(); goto exit;