[Commits] e8888cf9f8d: MDEV-25969: Condition pushdown into derived table doesn't work if select list uses SP

Sergei Petrunia psergey at askmonty.org
Mon Jun 21 01:06:00 EEST 2021


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;
 
 /*


More information about the commits mailing list