maria-developers team mailing list archive

[Sergey Vojtovich <svoj@xxxxxxxxxxx>]

Hi Alexander,

what if we do something like this:
static SEL_TREE *get_mm_tree_XXX(Item *arg1, Item *arg2)
{
  return !arg1->real_item()->type() == Item::FIELD_ITEM ||
         arg1->real_item()->const_item() ||
         arg2->is_expensive() ? 0 :
         get_full_func_mm_tree(param, this, (Item_field*) (arg1->real_item()), arg2, false);
}

then we should be able to do tricks like this:
  if (!(ftree= get_mm_tree_XXX(arguments()[0], arguments()[1])))
    ftree= get_mm_tree_XXX(arguments()[1], arguments()[0]);
  DBUG_RETURN(ftree);

Thanks,
Sergey

On Thu, May 28, 2015 at 08:25:57AM +0400, Alexander Barkov wrote:
> 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