revision-id: e8888cf9f8de304e1d7e6253cff8a3def2f4d379 (mariadb-10.2.31-962-ge8888cf9f8d) parent(s): f3dd96ad25efe23081981f52a54a57b17a5a890e author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2021-06-21 01:06:00 +0300 message: MDEV-25969: Condition pushdown into derived table doesn't work if select list uses SP Consider a query in form: select ... from (select item2 as COL1) as T where COL1=123 Condition pushdown into derived table will try to push "COL1=123" condition down into table T. The process of pushdown involves "substituting" the item, that is, replacing Item_field("T.COL1") with its "producing item" item2. In order to use item2, one needs to clone it (call Item::build_clone). If the item is not cloneable (e.g. SP function call is not), the pushdown process will fail and nothing at all will be pushed. This patch makes pushdown_cond_for_derived() to first extract the portion of a condition which can be pushed, and then push only that. Included changes: - TABLE_LIST::check_pushable_cond_for_table() can now accept the criteria that item must meet in order to be pushable. It is used in two places: first, extract the condition that only depends on a given table, and then, extract the condition that's pushable into a given SELECT. - TABLE_LIST::build_pushable_cond_for_table() works the same as before but it is no longer a member of TABLE_LIST for the sake of symmetry with check_pushable_cond_for_table. --- mysql-test/r/derived_cond_pushdown.result | 45 +++++++++++++++++++++++++++++ mysql-test/t/derived_cond_pushdown.test | 48 +++++++++++++++++++++++++++++++ sql/item.cc | 24 ++++++++++++++++ sql/item.h | 8 ++++++ sql/item_cmpfunc.h | 3 ++ sql/item_func.h | 1 + sql/sql_derived.cc | 15 ++++++++-- sql/table.cc | 37 +++++++++++++----------- sql/table.h | 7 +++-- 9 files changed, 167 insertions(+), 21 deletions(-) diff --git a/mysql-test/r/derived_cond_pushdown.result b/mysql-test/r/derived_cond_pushdown.result index 25237aa11a9..bae971ced74 100644 --- a/mysql-test/r/derived_cond_pushdown.result +++ b/mysql-test/r/derived_cond_pushdown.result @@ -10634,4 +10634,49 @@ m 7 drop view v1; drop table t1; +# +# MDEV-25969: Condition pushdown into derived table doesn't work if select list uses SP +# +create function f1(a int) returns int DETERMINISTIC return (a+1); +create table t1 ( +pk int primary key, +a int, +b int, +key(a) +); +create table t2(a int); +insert into t2 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); +create table t3(a int); +insert into t3 select A.a + B.a* 10 + C.a * 100 from t2 A, t2 B, t2 C; +insert into t1 select a,a,a from t3; +create view v1 as +select +t1.a as col1, +f1(t1.b) as col2 +from +t1; +create view v2 as +select +t1.a as col1, +f1(t1.b) as col2 +from +t1; +create view v3 as +select col2, col1 from v1 +union all +select col2, col1 from v2; +explain select * from v3 where col1=123; +id select_type table type possible_keys key key_len ref rows Extra +1 PRIMARY <derived2> ALL NULL NULL NULL NULL 2 Using where +2 DERIVED t1 ref a a 5 const 1 +3 UNION t1 ref a a 5 const 1 +# This must use ref accesses for reading table t1, not full scans: +explain select * from v3 where col1=123 and col2=321; +id select_type table type possible_keys key key_len ref rows Extra +1 PRIMARY <derived2> ALL NULL NULL NULL NULL 2 Using where +2 DERIVED t1 ref a a 5 const 1 +3 UNION t1 ref a a 5 const 1 +drop function f1; +drop view v1,v2,v3; +drop table t1, t2,t3; # End of 10.2 tests diff --git a/mysql-test/t/derived_cond_pushdown.test b/mysql-test/t/derived_cond_pushdown.test index 31b49047bf1..374b61f846a 100644 --- a/mysql-test/t/derived_cond_pushdown.test +++ b/mysql-test/t/derived_cond_pushdown.test @@ -2212,4 +2212,52 @@ select * from v1 where m > 0; drop view v1; drop table t1; +--echo # +--echo # MDEV-25969: Condition pushdown into derived table doesn't work if select list uses SP +--echo # +create function f1(a int) returns int DETERMINISTIC return (a+1); + +create table t1 ( + pk int primary key, + a int, + b int, + key(a) +); + +create table t2(a int); +insert into t2 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); + +create table t3(a int); +insert into t3 select A.a + B.a* 10 + C.a * 100 from t2 A, t2 B, t2 C; + +insert into t1 select a,a,a from t3; + +create view v1 as +select + t1.a as col1, + f1(t1.b) as col2 +from + t1; + +create view v2 as +select + t1.a as col1, + f1(t1.b) as col2 +from + t1; +create view v3 as +select col2, col1 from v1 +union all +select col2, col1 from v2; + +explain select * from v3 where col1=123; + +--echo # This must use ref accesses for reading table t1, not full scans: +explain select * from v3 where col1=123 and col2=321; + +drop function f1; +drop view v1,v2,v3; +drop table t1, t2,t3; + + --echo # End of 10.2 tests diff --git a/sql/item.cc b/sql/item.cc index 42272fe0148..d729da23daa 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -7249,6 +7249,30 @@ Item *find_producing_item(Item *item, st_select_lex *sel) return NULL; } + +/* + @brief Check if this item cannot be pushed down into derived table + + @detail + This function checks if derived_field_transformer_for_where() will + fail. It will fail if the "producing item" (column in the derived table) + cannot be cloned. + + @return + false - Ok, can be pushed + true - Cannot be pushed +*/ + +bool Item_field::check_non_pushable_processor(void *arg) +{ + st_select_lex *sel= (st_select_lex *)arg; + Item *producing_item= find_producing_item(this, sel); + if (producing_item) + return producing_item->walk(&Item::check_non_cloneable_processor, 0, 0); + return false; // Ok +} + + Item *Item_field::derived_field_transformer_for_where(THD *thd, uchar *arg) { st_select_lex *sel= (st_select_lex *)arg; diff --git a/sql/item.h b/sql/item.h index c94709c733e..f0753f7ded1 100644 --- a/sql/item.h +++ b/sql/item.h @@ -1829,6 +1829,12 @@ class Item: public Value_source, */ virtual bool check_valid_arguments_processor(void *arg) { return 0; } virtual bool update_vcol_processor(void *arg) { return 0; } + + /* Return true if the item can NOT be pushed down into a derived table */ + virtual bool check_non_pushable_processor(void *arg) { return 0; } + + /* Return true if the item cannot be cloned (get_copy() will return NULL) */ + virtual bool check_non_cloneable_processor(void *arg) { return 0; } /*============== End of Item processor list ======================*/ virtual Item *get_copy(THD *thd, MEM_ROOT *mem_root)=0; @@ -2894,6 +2900,8 @@ class Item_field :public Item_ident, friend class Item_default_value; friend class Item_insert_value; friend class st_select_lex_unit; + + bool check_non_pushable_processor(void *arg) override; }; diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 26469d88929..ce27989bb16 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -2135,6 +2135,7 @@ class Item_func_regex :public Item_bool_func const char *func_name() const { return "regexp"; } enum precedence precedence() const { return IN_PRECEDENCE; } Item *get_copy(THD *thd, MEM_ROOT *mem_root) { return 0; } + bool check_non_cloneable_processor(void *arg) { return true; } void print(String *str, enum_query_type query_type) { print_op(str, query_type); @@ -2163,6 +2164,7 @@ class Item_func_regexp_instr :public Item_int_func bool fix_length_and_dec(); const char *func_name() const { return "regexp_instr"; } Item *get_copy(THD *thd, MEM_ROOT *mem_root) { return 0; } + bool check_non_cloneable_processor(void *arg) { return true; } }; @@ -2421,6 +2423,7 @@ class Item_equal: public Item_bool_func void set_context_field(Item_field *ctx_field) { context_field= ctx_field; } void set_link_equal_fields(bool flag) { link_equal_fields= flag; } Item* get_copy(THD *thd, MEM_ROOT *mem_root) { return 0; } + bool check_non_cloneable_processor(void *arg) override { return true; } /* This does not comply with the specification of the virtual method, but Item_equal items are processed distinguishly anyway diff --git a/sql/item_func.h b/sql/item_func.h index 496109b0e24..5dfcdb956cf 100644 --- a/sql/item_func.h +++ b/sql/item_func.h @@ -2480,6 +2480,7 @@ class Item_func_sp :public Item_func return TRUE; } Item *get_copy(THD *thd, MEM_ROOT *mem_root) { return 0; } + bool check_non_cloneable_processor(void *arg) override { return true; } bool eval_not_null_tables(void *opt_arg) { not_null_tables_cache= 0; diff --git a/sql/sql_derived.cc b/sql/sql_derived.cc index 3ab93840d80..468e7c326aa 100644 --- a/sql/sql_derived.cc +++ b/sql/sql_derived.cc @@ -1261,8 +1261,12 @@ bool pushdown_cond_for_derived(THD *thd, Item *cond, TABLE_LIST *derived) This condition has to be fixed yet. */ Item *extracted_cond; - derived->check_pushable_cond_for_table(cond); - extracted_cond= derived->build_pushable_cond_for_table(thd, cond); + table_map derived_tbl_bit= derived->table->map; + check_pushable_cond_for_table(cond, + [=](Item *cond) { + return cond->excl_dep_on_table(derived_tbl_bit); + }); + extracted_cond= build_pushable_cond_for_table(thd, derived_tbl_bit, cond); if (!extracted_cond) { /* Nothing can be pushed into the derived table */ @@ -1287,6 +1291,13 @@ bool pushdown_cond_for_derived(THD *thd, Item *cond, TABLE_LIST *derived) if (!sl->join->group_list && !sl->with_sum_func) { /* extracted_cond_copy is pushed into where of sl */ + check_pushable_cond_for_table(extracted_cond_copy, + [=](Item *cond) { + return !cond->walk(&Item::check_non_pushable_processor, + false, (void*)sl); + }); + extracted_cond_copy= build_pushable_cond_for_table(thd, derived_tbl_bit, + extracted_cond_copy); extracted_cond_copy= extracted_cond_copy->transform(thd, &Item::derived_field_transformer_for_where, (uchar*) sl); diff --git a/sql/table.cc b/sql/table.cc index 1004f583448..c2206b55184 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -8438,19 +8438,19 @@ double KEY::actual_rec_per_key(uint i) @param cond The condition whose subformulas are to be marked @details - This method recursively traverses the AND-OR condition cond and for each subformula - of the codition it checks whether it can be usable for the extraction of a condition + Recursively traverse the AND-OR condition cond and for each subformula + of the codition check whether it can be usable for the extraction of a condition that can be pushed into this table. The subformulas that are not usable are marked with the flag NO_EXTRACTION_FL. @note - This method is called before any call of TABLE_LIST::build_pushable_cond_for_table. + This function is called before any call of build_pushable_cond_for_table(). The flag NO_EXTRACTION_FL set in a subformula allows to avoid building clone for the subformula when extracting the pushable condition. */ -void TABLE_LIST::check_pushable_cond_for_table(Item *cond) +void check_pushable_cond_for_table(Item *cond, + std::function<bool(Item*)> pushable_filter) { - table_map tab_map= table->map; cond->clear_extraction_flag(); if (cond->type() == Item::COND_ITEM) { @@ -8460,7 +8460,7 @@ void TABLE_LIST::check_pushable_cond_for_table(Item *cond) Item *item; while ((item=li++)) { - check_pushable_cond_for_table(item); + check_pushable_cond_for_table(item, pushable_filter); if (item->get_extraction_flag() != NO_EXTRACTION_FL) count++; else if (!and_cond) @@ -8475,25 +8475,29 @@ void TABLE_LIST::check_pushable_cond_for_table(Item *cond) item->clear_extraction_flag(); } } - else if (!cond->excl_dep_on_table(tab_map)) - cond->set_extraction_flag(NO_EXTRACTION_FL); + else + { + if (!pushable_filter(cond)) + cond->set_extraction_flag(NO_EXTRACTION_FL); + } } /** @brief - Build condition extractable from the given one depended only on this table + Build condition extractable from the given one depended only on the table - @param thd The thread handle - @param cond The condition from which the pushable one is to be extracted - + @param thd The thread handle + @param tab_map Table the condition must depend on. + @param cond The condition from which the pushable one is to be extracted + @details For the given condition cond this method finds out what condition depended only on this table can be extracted from cond. If such condition C exists the method builds the item for it. The method uses the flag NO_EXTRACTION_FL set by the preliminary call of - the method TABLE_LIST::check_pushable_cond_for_table to figure out whether - a subformula depends only on this table or not. + check_pushable_cond_for_table() to figure out whether a subformula depends + only on this table or not. @note The built condition C is always implied by the condition cond (cond => C). The method tries to build the most restictive such @@ -8508,9 +8512,8 @@ void TABLE_LIST::check_pushable_cond_for_table(Item *cond) NULL if there is no such a condition */ -Item* TABLE_LIST::build_pushable_cond_for_table(THD *thd, Item *cond) +Item* build_pushable_cond_for_table(THD *thd, table_map tab_map, Item *cond) { - table_map tab_map= table->map; bool is_multiple_equality= cond->type() == Item::FUNC_ITEM && ((Item_func*) cond)->functype() == Item_func::MULT_EQUAL_FUNC; if (cond->get_extraction_flag() == NO_EXTRACTION_FL) @@ -8539,7 +8542,7 @@ Item* TABLE_LIST::build_pushable_cond_for_table(THD *thd, Item *cond) return 0; continue; } - Item *fix= build_pushable_cond_for_table(thd, item); + Item *fix= build_pushable_cond_for_table(thd, tab_map, item); if (!fix && !cond_and) return 0; if (!fix) diff --git a/sql/table.h b/sql/table.h index 83c72f76831..ff80bda5b6e 100644 --- a/sql/table.h +++ b/sql/table.h @@ -16,6 +16,7 @@ along with this program; if not, write to the Free Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1335 USA */ +#include <functional> #include "my_global.h" /* NO_EMBEDDED_ACCESS_CHECKS */ #include "sql_plist.h" #include "sql_list.h" /* Sql_alloc */ @@ -2534,8 +2535,6 @@ struct TABLE_LIST return false; } void set_lock_type(THD* thd, enum thr_lock_type lock); - void check_pushable_cond_for_table(Item *cond); - Item *build_pushable_cond_for_table(THD *thd, Item *cond); void remove_join_columns() { @@ -2562,6 +2561,10 @@ struct TABLE_LIST ulong m_table_ref_version; }; +Item *build_pushable_cond_for_table(THD *thd, table_map tab_map, Item *cond); +void check_pushable_cond_for_table(Item *cond, + std::function<bool(Item*)> pushable_filter); + class Item; /*