maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

Hello Sergey,

Please review the next step for MDEV-7950.

It does the following:

- Adds specific implementations of virtual method get_mm_tree() into
  Item_func_like, Item_bool_rowready_func2, Item_func_spatial_rel

- Gets rid of two virtual calls: select_optimize() and have_rev_func().

- Removes virtual methods Item_func::select_optimize(),
  Item_func::have_rev_func(),
  as well as enum Item_func::optimize_type,
  because they are not needed any more.

Thanks.

diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index 93e4788..57a88e3 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -4806,22 +4806,21 @@ longlong Item_func_like::val_int()
   We can optimize a where if first character isn't a wildcard
 */
 
-Item_func::optimize_type Item_func_like::select_optimize() const
+bool Item_func_like::is_sargable_pattern() const
 {
   if (!args[1]->const_item() || args[1]->is_expensive())
-    return OPTIMIZE_NONE;
+    return false;
 
   String* res2= args[1]->val_str((String *)&cmp.value2);
   if (!res2)
-    return OPTIMIZE_NONE;
+    return false;
 
   if (!res2->length()) // Can optimize empty wildcard: column LIKE ''
-    return OPTIMIZE_OP;
+    return true;
 
   DBUG_ASSERT(res2->ptr());
   char first= res2->ptr()[0];
-  return (first == wild_many || first == wild_one) ?
-    OPTIMIZE_NONE : OPTIMIZE_OP;
+  return first != wild_many && first != wild_one;
 }
 
 
diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h
index 2f26e48..7e2c5a1 100644
--- a/sql/item_cmpfunc.h
+++ b/sql/item_cmpfunc.h
@@ -304,10 +304,7 @@ class Item_bool_func2 :public Item_bool_func
   {
     return cmp.set_cmp_func(this, tmp_arg, tmp_arg+1, TRUE);
   }
-  optimize_type select_optimize() const { return OPTIMIZE_OP; }
   virtual enum Functype rev_functype() const { return UNKNOWN_FUNC; }
-  bool have_rev_func() const { return rev_functype() != UNKNOWN_FUNC; }
-
   virtual inline void print(String *str, enum_query_type query_type)
   {
     Item_func::print_op(str, query_type);
@@ -355,6 +352,10 @@ class Item_bool_rowready_func2 :public Item_bool_func2
     return add_key_fields_optimize_op(join, key_fields, and_level,
                                       usable_tables, sargables, false);
   }
+  SEL_TREE *get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr)
+  {
+    return get_mm_tree_op2(param, true);
+  }
 };
 
 /**
@@ -591,7 +592,6 @@ class Item_func_ne :public Item_bool_rowready_func2
   longlong val_int();
   enum Functype functype() const { return NE_FUNC; }
   cond_result eq_cmp_result() const { return COND_FALSE; }
-  optimize_type select_optimize() const { return OPTIMIZE_KEY; } 
   const char *func_name() const { return "<>"; }
   Item *negated_item();
   void add_key_fields(JOIN *join, KEY_FIELD **key_fields, uint *and_level,
@@ -642,7 +642,6 @@ class Item_func_between :public Item_func_opt_neg
   Item_func_between(Item *a, Item *b, Item *c)
     :Item_func_opt_neg(a, b, c), compare_as_dates(FALSE) { sargable= TRUE; }
   longlong val_int();
-  optimize_type select_optimize() const { return OPTIMIZE_KEY; }
   enum Functype functype() const   { return BETWEEN; }
   const char *func_name() const { return "between"; }
   bool fix_fields(THD *, Item **);
@@ -1334,8 +1333,6 @@ class Item_func_in :public Item_func_opt_neg
     }
     DBUG_VOID_RETURN;
   }
-  optimize_type select_optimize() const
-    { return OPTIMIZE_KEY; }
   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);
@@ -1382,9 +1379,9 @@ class Item_func_null_predicate :public Item_bool_func
 {
 public:
   Item_func_null_predicate(Item *a) :Item_bool_func(a) { sargable= true; }
-  optimize_type select_optimize() const { return OPTIMIZE_NULL; }
   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);
   CHARSET_INFO *compare_collation() const
   { return args[0]->collation.collation; }
   void fix_length_and_dec() { decimals=0; max_length=1; maybe_null=0; }
@@ -1490,6 +1487,12 @@ class Item_func_like :public Item_bool_func2
 
   bool escape_used_in_parsing;
   bool use_sampling;
+  /**
+    Determines if the LIKE pattern is suitable for range optimizer:
+    It must be inexpensive const Item with no wildcards on the
+    leftmost position.
+  */
+  bool is_sargable_pattern() const;
 
 public:
   int escape;
@@ -1500,7 +1503,11 @@ class Item_func_like :public Item_bool_func2
      escape_used_in_parsing(escape_used), use_sampling(0) {}
   longlong val_int();
   enum Functype functype() const { return LIKE_FUNC; }
-  optimize_type select_optimize() const;
+  SEL_TREE *get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr)
+  {
+    return is_sargable_pattern() ? get_mm_tree_op2(param, false) :
+                                   Item_func::get_mm_tree(param, cond_ptr);
+  }
   cond_result eq_cmp_result() const
   {
     /**
@@ -1918,7 +1925,6 @@ class Item_equal: public Item_bool_func
   enum Functype functype() const { return MULT_EQUAL_FUNC; }
   longlong val_int(); 
   const char *func_name() const { return "multiple equal"; }
-  optimize_type select_optimize() const { return OPTIMIZE_EQUAL; }
   void sort(Item_field_cmpfunc compare, void *arg);
   void fix_length_and_dec();
   bool fix_fields(THD *thd, Item **ref);
diff --git a/sql/item_func.h b/sql/item_func.h
index cb8975a..1dbd926 100644
--- a/sql/item_func.h
+++ b/sql/item_func.h
@@ -41,6 +41,21 @@ class Item_func :public Item_func_or_sum, public Used_tables_and_const_cache
   */
   uint allowed_arg_cols;
   String *val_str_from_val_str_ascii(String *str, String *str2);
+  /**
+    This method is invoked for binary operations:
+    - Comparison operators:  <, <=, =, <=>, =>, >
+    - LIKE
+    - Spatial relation predicates: Crosses, Touches, Contains, Disjoint,
+                                   Equals, Intersects, Overlaps, Within
+    @param param        - Range optimizer context
+    @param optimize_rev - Determines whether the range optimizer should try
+                          to optimize REVERSE_OPERATION(const,field)
+                          if the direct operation was not in the form:
+                          OPERATION(field,const), e.g.
+                              WHERE 10<field1  ->  WHERE field1>10
+    @return             - select tree for the function
+  */
+  SEL_TREE *get_mm_tree_op2(RANGE_OPT_PARAM *param, bool optimize_rev);
 public:
   table_map not_null_tables_cache;
 
@@ -60,8 +75,6 @@ class Item_func :public Item_func_or_sum, public Used_tables_and_const_cache
                   SUSERVAR_FUNC, GUSERVAR_FUNC, COLLATE_FUNC,
                   EXTRACT_FUNC, CHAR_TYPECAST_FUNC, FUNC_SP, UDF_FUNC,
                   NEG_FUNC, GSYSVAR_FUNC, DYNCOL_FUNC };
-  enum optimize_type { OPTIMIZE_NONE,OPTIMIZE_KEY,OPTIMIZE_OP, OPTIMIZE_NULL,
-                       OPTIMIZE_EQUAL };
   enum Type type() const { return FUNC_ITEM; }
   virtual enum Functype functype() const   { return UNKNOWN_FUNC; }
   Item_func(void):
@@ -136,8 +149,6 @@ class Item_func :public Item_func_or_sum, public Used_tables_and_const_cache
                           COND_EQUAL **cond_equal_ref);
   SEL_TREE *get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr);
   bool eq(const Item *item, bool binary_cmp) const;
-  virtual optimize_type select_optimize() const { return OPTIMIZE_NONE; }
-  virtual bool have_rev_func() const { return 0; }
   virtual Item *key_item() const { return args[0]; }
   virtual bool const_item() const { return const_item_cache; }
   void set_arguments(List<Item> &list)
diff --git a/sql/item_geofunc.h b/sql/item_geofunc.h
index 97001af..3024a14 100644
--- a/sql/item_geofunc.h
+++ b/sql/item_geofunc.h
@@ -287,7 +287,6 @@ class Item_func_spatial_rel: public Item_bool_func
   enum Functype functype() const { return spatial_rel; }
   enum Functype rev_functype() const { return spatial_rel; }
   bool is_null() { (void) val_int(); return null_value; }
-  optimize_type select_optimize() const { return OPTIMIZE_OP; }
   void add_key_fields(JOIN *join, KEY_FIELD **key_fields,
                       uint *and_level, table_map usable_tables,
                       SARGABLE_PARAM **sargables)
@@ -295,6 +294,14 @@ class Item_func_spatial_rel: public Item_bool_func
     return add_key_fields_optimize_op(join, key_fields, and_level,
                                       usable_tables, sargables, false);
   }
+  SEL_TREE *get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr)
+  {
+    /*
+     TODO:
+     MDEV-8239 Reverse spatial operations OP(const, field) do not get optimized
+    */
+    return get_mm_tree_op2(param, false);
+  }
 };
 
 
diff --git a/sql/opt_range.cc b/sql/opt_range.cc
index 190cd45..e936755 100644
--- a/sql/opt_range.cc
+++ b/sql/opt_range.cc
@@ -7812,10 +7812,19 @@ static SEL_TREE *get_func_mm_tree(RANGE_OPT_PARAM *param, Item_func *cond_func,
   {
     /* 
        Here the function for the following predicates are processed:
-       <, <=, =, >=, >, LIKE, IS NULL, IS NOT NULL.
+       <, <=, =, >=, >, LIKE, IS NULL, IS NOT NULL, spatial relation predicates
        If the predicate is of the form (value op field) it is handled
        as the equivalent predicate (field rev_op value), e.g.
        2 <= a is handled as a >= 2.
+
+       TODO:
+       MDEV-8239 Reverse spatial operations OP(const, field) do not get optimized
+       Currently, for spatial predicates:
+       - field is always in arguments()[0]
+       - value is always in arguments()[1].
+       The below cast to Item_bool_func2 with a rev_functype() call will not
+       be valid for spatial predicates when MDEV-8239 gets fixed and
+       field/value start to be passed in the opposite order.
     */
     Item_func::Functype func_type=
       (value != cond_func->arguments()[0]) ? cond_func->functype() :
@@ -8200,14 +8209,49 @@ SEL_TREE *Item_equal::get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr)
 }
 
 
+/*
+  This is invoked for the functions that cannot use range optimizer
+*/
 SEL_TREE *Item_func::get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr)
 {
   DBUG_ENTER("Item_func::get_mm_tree");
   if (const_item())
     DBUG_RETURN(get_mm_tree_for_const(param, this));
+  DBUG_RETURN(0);
+}
 
-  if (select_optimize() == Item_func::OPTIMIZE_NONE)
-    DBUG_RETURN(0);
+
+SEL_TREE *
+Item_func_null_predicate::get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr)
+{
+  DBUG_ENTER("Item_func_null_predicate::get_mm_tree");
+  if (const_item())
+    DBUG_RETURN(get_mm_tree_for_const(param, this));
+
+  param->cond= this;
+  if (arguments()[0]->real_item()->type() == Item::FIELD_ITEM)
+  {
+    Item_field *field_item= (Item_field*) (arguments()[0]->real_item());
+    if (!field_item->const_item())
+      DBUG_RETURN(get_full_func_mm_tree(param, this, field_item, NULL, false));
+  }
+  DBUG_RETURN(NULL);
+}
+
+
+/*
+  This method is invoked for binary operations:
+  - Comparison operators:  <, <=, =, <=>, =>, >
+  - LIKE
+  - Spatial relation predicates: Crosses, Touches, Contains, Disjoint,
+                                 Equals, Intersects, Overlaps, Within
+*/
+SEL_TREE *Item_func::get_mm_tree_op2(RANGE_OPT_PARAM *param, bool optimize_rev)
+{
+  DBUG_ENTER("Item_func::get_mm_tree_op2");
+  DBUG_ASSERT(arg_count == 2);
+  if (const_item())
+    DBUG_RETURN(get_mm_tree_for_const(param, this));
 
   param->cond= this;
 
@@ -8215,7 +8259,7 @@ SEL_TREE *Item_func::get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr)
   if (arguments()[0]->real_item()->type() == Item::FIELD_ITEM)
   {
     Item_field *field_item= (Item_field*) (arguments()[0]->real_item());
-    Item *value= arg_count > 1 ? arguments()[1] : NULL;
+    Item *value= arguments()[1];
     if (value && value->is_expensive())
       DBUG_RETURN(0);
     if (!arguments()[0]->real_item()->const_item())
@@ -8237,7 +8281,7 @@ SEL_TREE *Item_func::get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr)
     call to get_full_func_mm_tree() with reversed operands (see
     below) may succeed.
   */
-  if (!ftree && have_rev_func() &&
+  if (!ftree && optimize_rev &&
       arguments()[1]->real_item()->type() == Item::FIELD_ITEM)
   {
     Item_field *field_item= (Item_field*) (arguments()[1]->real_item());
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index dccad7a..b50433e 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -4762,8 +4762,7 @@ Item_func_like::add_key_fields(JOIN *join, KEY_FIELD **key_fields,
                                uint *and_level, table_map usable_tables,
                                SARGABLE_PARAM **sargables)
 {
-  if (is_local_field(args[0]) &&
-      Item_func_like::select_optimize() == OPTIMIZE_OP)
+  if (is_local_field(args[0]) && is_sargable_pattern())
   {
     /*
       SELECT * FROM t1 WHERE field LIKE const_pattern