← Back to team overview

maria-developers team mailing list archive

Please review MDEV-11357 Split Item_cache::get_cache() into virtual methods in Type_handler

 

Hello Sanja,

Can you please review a patch for MDEV-11357?

This is another per-requisite task for:
MDEV-4912 Add a plugin to field types (column types)

Thanks!
commit 913dd04e6348e883bba18b6b1dcba4e45fc598bf
Author: Alexander Barkov <bar@xxxxxxxxxxx>
Date:   Sat Nov 26 21:19:48 2016 +0400

    MDEV-11357 Split Item_cache::get_cache() into virtual methods in Type_handler
    
    This patch:
    - Adds a new virtual method Type_handler::Item_get_cache
    - Splits moves Item_cache::get_cache() into the new method, every
      "case XXX_RESULT" to the corresponding Type_handler_xxx::Item_get_cache.
    - Adds Item::get_cache as a convenience wrapper, to make the caller code
      shorter.
    - Changes the last argument of Arg_comparator::cache_converted_constant()
      from Item_result to "const Type_handler *".
    - Removes subselect_engine::cmp_type, subselect_engine::res_type,
      subselect_engine::res_field_type and derives subselect_engine
      from Type_handler_hybrid_field_type instead.
    - Makes Type_handler_varchar public, as it's now needed as the
      default data type handler for subselect_engine.

diff --git a/sql/item.cc b/sql/item.cc
index ee453fd..c97f41f 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -6811,7 +6811,7 @@ Item* Item::cache_const_expr_transformer(THD *thd, uchar *arg)
   if (*(bool*)arg)
   {
     *((bool*)arg)= FALSE;
-    Item_cache *cache= Item_cache::get_cache(thd, this);
+    Item_cache *cache= get_cache(thd);
     if (!cache)
       return NULL;
     cache->setup(thd, this);
@@ -7851,7 +7851,7 @@ Item_cache_wrapper::Item_cache_wrapper(THD *thd, Item *item_arg):
   name_length= item_arg->name_length;
   with_subselect=  orig_item->with_subselect;
 
-  if ((expr_value= Item_cache::get_cache(thd, orig_item)))
+  if ((expr_value= orig_item->get_cache(thd)))
     expr_value->setup(thd, orig_item);
 
   fixed= 1;
@@ -9105,41 +9105,6 @@ int stored_field_cmp_to_item(THD *thd, Field *field, Item *item)
   return 0;
 }
 
-Item_cache* Item_cache::get_cache(THD *thd, const Item *item)
-{
-  return get_cache(thd, item, item->cmp_type());
-}
-
-
-/**
-  Get a cache item of given type.
-
-  @param item         value to be cached
-  @param type         required type of cache
-
-  @return cache item
-*/
-
-Item_cache* Item_cache::get_cache(THD *thd, const Item *item,
-                                  const Item_result type)
-{
-  MEM_ROOT *mem_root= thd->mem_root;
-  switch (type) {
-  case INT_RESULT:
-    return new (mem_root) Item_cache_int(thd, item->field_type());
-  case REAL_RESULT:
-    return new (mem_root) Item_cache_real(thd);
-  case DECIMAL_RESULT:
-    return new (mem_root) Item_cache_decimal(thd);
-  case STRING_RESULT:
-    return new (mem_root) Item_cache_str(thd, item);
-  case ROW_RESULT:
-    return new (mem_root) Item_cache_row(thd);
-  case TIME_RESULT:
-    return new (mem_root) Item_cache_temporal(thd, item->field_type());
-  }
-  return 0;                                     // Impossible
-}
 
 void Item_cache::store(Item *item)
 {
@@ -9580,7 +9545,7 @@ bool Item_cache_row::setup(THD *thd, Item *item)
   {
     Item *el= item->element_index(i);
     Item_cache *tmp;
-    if (!(tmp= values[i]= Item_cache::get_cache(thd, el)))
+    if (!(tmp= values[i]= el->get_cache(thd)))
       return 1;
     tmp->setup(thd, el);
   }
diff --git a/sql/item.h b/sql/item.h
index 3b21176..a665bc6 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -695,6 +695,10 @@ class Item: public Value_source,
   {
     return Type_handler::string_type_handler(max_length)->field_type();
   }
+  Item_cache* get_cache(THD *thd) const
+  {
+    return type_handler()->Item_get_cache(thd, this);
+  }
   virtual enum Type type() const =0;
   /*
     real_type() is the type of base item.  This is same as type() for
@@ -5186,8 +5190,6 @@ class Item_cache: public Item_basic_constant,
   enum Item_result cmp_type () const
   { return Type_handler_hybrid_field_type::cmp_type(); }
 
-  static Item_cache* get_cache(THD *thd, const Item *item);
-  static Item_cache* get_cache(THD *thd, const Item* item, const Item_result type);
   virtual void keep_array() {}
   virtual void print(String *str, enum_query_type query_type);
   bool eq_def(const Field *field) 
diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index 7313332..98b179b 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -608,8 +608,8 @@ bool Arg_comparator::set_cmp_func_string()
     if (owner->agg_arg_charsets_for_comparison(&m_compare_collation, a, b))
       return true;
   }
-  a= cache_converted_constant(thd, a, &a_cache, compare_type());
-  b= cache_converted_constant(thd, b, &b_cache, compare_type());
+  a= cache_converted_constant(thd, a, &a_cache, compare_type_handler());
+  b= cache_converted_constant(thd, b, &b_cache, compare_type_handler());
   return false;
 }
 
@@ -656,8 +656,8 @@ bool Arg_comparator::set_cmp_func_int()
     if ((*a)->unsigned_flag ^ (*b)->unsigned_flag)
       func= &Arg_comparator::compare_e_int_diff_signedness;
   }
-  a= cache_converted_constant(thd, a, &a_cache, compare_type());
-  b= cache_converted_constant(thd, b, &b_cache, compare_type());
+  a= cache_converted_constant(thd, a, &a_cache, compare_type_handler());
+  b= cache_converted_constant(thd, b, &b_cache, compare_type_handler());
   return false;
 }
 
@@ -674,8 +674,8 @@ bool Arg_comparator::set_cmp_func_real()
     else if (func == &Arg_comparator::compare_e_real)
       func= &Arg_comparator::compare_e_real_fixed;
   }
-  a= cache_converted_constant(thd, a, &a_cache, compare_type());
-  b= cache_converted_constant(thd, b, &b_cache, compare_type());
+  a= cache_converted_constant(thd, a, &a_cache, compare_type_handler());
+  b= cache_converted_constant(thd, b, &b_cache, compare_type_handler());
   return false;
 }
 
@@ -684,8 +684,8 @@ bool Arg_comparator::set_cmp_func_decimal()
 {
   func= is_owner_equal_func() ? &Arg_comparator::compare_e_decimal :
                                 &Arg_comparator::compare_decimal;
-  a= cache_converted_constant(thd, a, &a_cache, compare_type());
-  b= cache_converted_constant(thd, b, &b_cache, compare_type());
+  a= cache_converted_constant(thd, a, &a_cache, compare_type_handler());
+  b= cache_converted_constant(thd, b, &b_cache, compare_type_handler());
   return false;
 }
 
@@ -709,18 +709,22 @@ bool Arg_comparator::set_cmp_func_decimal()
 
 Item** Arg_comparator::cache_converted_constant(THD *thd_arg, Item **value,
                                                 Item **cache_item,
-                                                Item_result type)
+                                                const Type_handler *handler)
 {
   /*
+    get_datetime_value creates Item_cache internally when comparing
+    values for the first row.
+    Arg_comparator::cache_converted_constant() is never called for TIME_RESULT.
+  */
+  DBUG_ASSERT(handler->cmp_type() != TIME_RESULT);
+  /*
     Don't need cache if doing context analysis only.
-    Also, get_datetime_value creates Item_cache internally.
-    Unless fixed, we should not do it here.
   */
   if (!thd_arg->lex->is_ps_or_view_context_analysis() &&
-      (*value)->const_item() && type != (*value)->result_type() &&
-      type != TIME_RESULT)
+      (*value)->const_item() &&
+      handler->cmp_type() != (*value)->cmp_type())
   {
-    Item_cache *cache= Item_cache::get_cache(thd_arg, *value, type);
+    Item_cache *cache= handler->Item_get_cache(thd_arg, *value);
     cache->setup(thd_arg, *value);
     *cache_item= cache;
     return cache_item;
@@ -1285,7 +1289,7 @@ bool Item_in_optimizer::fix_left(THD *thd)
     args[0]= ((Item_in_subselect *)args[1])->left_expr;
   }
   if ((!(*ref0)->fixed && (*ref0)->fix_fields(thd, ref0)) ||
-      (!cache && !(cache= Item_cache::get_cache(thd, *ref0))))
+      (!cache && !(cache= (*ref0)->get_cache(thd))))
     DBUG_RETURN(1);
   /*
     During fix_field() expression could be substituted.
@@ -2710,7 +2714,7 @@ Item_func_nullif::fix_length_and_dec()
     */
     m_cache= args[0]->cmp_type() == STRING_RESULT ?
              new (thd->mem_root) Item_cache_str_for_nullif(thd, args[0]) :
-             Item_cache::get_cache(thd, args[0]);
+             args[0]->get_cache(thd);
     m_cache->setup(thd, args[0]);
     m_cache->store(args[0]);
     m_cache->set_used_tables(args[0]->used_tables());
diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h
index 2c69896..98bcb2d 100644
--- a/sql/item_cmpfunc.h
+++ b/sql/item_cmpfunc.h
@@ -118,7 +118,7 @@ class Arg_comparator: public Sql_alloc
   int compare_e_time()     { return compare_e_temporal(MYSQL_TYPE_TIME); }
 
   Item** cache_converted_constant(THD *thd, Item **value, Item **cache,
-                                  Item_result type);
+                                  const Type_handler *type);
   inline bool is_owner_equal_func()
   {
     return (owner->type() == Item::FUNC_ITEM &&
diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc
index 21c6333..8c88281 100644
--- a/sql/item_subselect.cc
+++ b/sql/item_subselect.cc
@@ -1057,7 +1057,7 @@ void Item_maxmin_subselect::no_rows_in_result()
   */
   if (parsing_place != SELECT_LIST || const_item())
     return;
-  value= Item_cache::get_cache(thd, new (thd->mem_root) Item_null(thd));
+  value= (new (thd->mem_root) Item_null(thd))->get_cache(thd);
   null_value= 0;
   was_values= 0;
   make_const();
@@ -1075,7 +1075,7 @@ void Item_singlerow_subselect::no_rows_in_result()
   */
   if (parsing_place != SELECT_LIST || const_item())
     return;
-  value= Item_cache::get_cache(thd, new (thd->mem_root) Item_null(thd));
+  value= (new (thd->mem_root) Item_null(thd))->get_cache(thd);
   reset();
   make_const();
 }
@@ -1165,12 +1165,17 @@ void Item_singlerow_subselect::store(uint i, Item *item)
 
 enum Item_result Item_singlerow_subselect::result_type() const
 {
-  return engine->type();
+  return engine->result_type();
 }
 
 enum Item_result Item_singlerow_subselect::cmp_type() const
 {
-  return engine->cmptype();
+  return engine->cmp_type();
+}
+
+const Type_handler *Item_singlerow_subselect::type_handler() const
+{
+  return engine->type_handler();
 }
 
 /* 
@@ -3635,24 +3640,21 @@ void subselect_engine::set_row(List<Item> &item_list, Item_cache **row)
 {
   Item *sel_item;
   List_iterator_fast<Item> li(item_list);
-  cmp_type= res_type= STRING_RESULT;
-  res_field_type= MYSQL_TYPE_VAR_STRING;
+  set_handler(&type_handler_varchar);
   for (uint i= 0; (sel_item= li++); i++)
   {
     item->max_length= sel_item->max_length;
-    res_type= sel_item->result_type();
-    cmp_type= sel_item->cmp_type();
-    res_field_type= sel_item->field_type();
+    set_handler(sel_item->type_handler());
     item->decimals= sel_item->decimals;
     item->unsigned_flag= sel_item->unsigned_flag;
     maybe_null= sel_item->maybe_null;
-    if (!(row[i]= Item_cache::get_cache(thd, sel_item, sel_item->cmp_type())))
+    if (!(row[i]= sel_item->get_cache(thd)))
       return;
     row[i]->setup(thd, sel_item);
  //psergey-backport-timours:   row[i]->store(sel_item);
   }
   if (item_list.elements > 1)
-    cmp_type= res_type= ROW_RESULT;
+    set_handler(&type_handler_row);
 }
 
 void subselect_single_select_engine::fix_length_and_dec(Item_cache **row)
diff --git a/sql/item_subselect.h b/sql/item_subselect.h
index 8334057..8928648 100644
--- a/sql/item_subselect.h
+++ b/sql/item_subselect.h
@@ -301,6 +301,7 @@ class Item_singlerow_subselect :public Item_subselect
   enum Item_result result_type() const;
   enum Item_result cmp_type() const;
   enum_field_types field_type() const;
+  const Type_handler *type_handler() const;
   void fix_length_and_dec();
 
   uint cols();
@@ -747,15 +748,13 @@ class Item_allany_subselect :public Item_in_subselect
 };
 
 
-class subselect_engine: public Sql_alloc
+class subselect_engine: public Sql_alloc,
+                        public Type_handler_hybrid_field_type
 {
 protected:
   select_result_interceptor *result; /* results storage class */
   THD *thd; /* pointer to current THD */
   Item_subselect *item; /* item, that use this engine */
-  enum Item_result res_type; /* type of results */
-  enum Item_result cmp_type; /* how to compare the results */
-  enum_field_types res_field_type; /* column type of the results */
   bool maybe_null; /* may be null (first item in select) */
 public:
 
@@ -766,12 +765,11 @@ class subselect_engine: public Sql_alloc
 
   subselect_engine(Item_subselect *si,
                    select_result_interceptor *res):
+    Type_handler_hybrid_field_type(&type_handler_varchar),
     thd(NULL)
   {
     result= res;
     item= si;
-    cmp_type= res_type= STRING_RESULT;
-    res_field_type= MYSQL_TYPE_VAR_STRING;
     maybe_null= 0;
   }
   virtual ~subselect_engine() {}; // to satisfy compiler
@@ -808,9 +806,6 @@ class subselect_engine: public Sql_alloc
   virtual int exec()= 0;
   virtual uint cols()= 0; /* return number of columns in select */
   virtual uint8 uncacheable()= 0; /* query is uncacheable */
-  enum Item_result type() { return res_type; }
-  enum Item_result cmptype() { return cmp_type; }
-  enum_field_types field_type() { return res_field_type; }
   virtual void exclude()= 0;
   virtual bool may_be_null() { return maybe_null; };
   virtual table_map upper_select_const_tables()= 0;
diff --git a/sql/item_sum.cc b/sql/item_sum.cc
index 3754bc1..9ffb056 100644
--- a/sql/item_sum.cc
+++ b/sql/item_sum.cc
@@ -1185,14 +1185,14 @@ Item_sum_hybrid::fix_fields(THD *thd, Item **ref)
 
 void Item_sum_hybrid::setup_hybrid(THD *thd, Item *item, Item *value_arg)
 {
-  if (!(value= Item_cache::get_cache(thd, item, item->cmp_type())))
+  if (!(value= item->get_cache(thd)))
     return;
   value->setup(thd, item);
   value->store(value_arg);
   /* Don't cache value, as it will change */
   if (!item->const_item())
     value->set_used_tables(RAND_TABLE_BIT);
-  if (!(arg_cache= Item_cache::get_cache(thd, item, item->cmp_type())))
+  if (!(arg_cache= item->get_cache(thd)))
     return;
   arg_cache->setup(thd, item);
   /* Don't cache value, as it will change */
diff --git a/sql/item_windowfunc.cc b/sql/item_windowfunc.cc
index a13967e..27eeeff 100644
--- a/sql/item_windowfunc.cc
+++ b/sql/item_windowfunc.cc
@@ -301,7 +301,7 @@ bool Item_sum_hybrid_simple::add()
 
 void Item_sum_hybrid_simple::setup_hybrid(THD *thd, Item *item)
 {
-  if (!(value= Item_cache::get_cache(thd, item, item->cmp_type())))
+  if (!(value= item->get_cache(thd)))
     return;
   value->setup(thd, item);
   value->store(item);
diff --git a/sql/opt_subselect.cc b/sql/opt_subselect.cc
index a09826a..39df7b8 100644
--- a/sql/opt_subselect.cc
+++ b/sql/opt_subselect.cc
@@ -5228,7 +5228,7 @@ int select_value_catcher::setup(List<Item> *items)
   List_iterator<Item> li(*items);
   for (uint i= 0; (sel_item= li++); i++)
   {
-    if (!(row[i]= Item_cache::get_cache(thd, sel_item)))
+    if (!(row[i]= sel_item->get_cache(thd)))
       return TRUE;
     row[i]->setup(thd, sel_item);
   }
diff --git a/sql/sp_rcontext.cc b/sql/sp_rcontext.cc
index 6e72446..8873b87 100644
--- a/sql/sp_rcontext.cc
+++ b/sql/sp_rcontext.cc
@@ -387,7 +387,7 @@ Item_cache *sp_rcontext::create_case_expr_holder(THD *thd,
 
   thd->set_n_backup_active_arena(thd->spcont->callers_arena, &current_arena);
 
-  holder= Item_cache::get_cache(thd, item);
+  holder= item->get_cache(thd);
 
   thd->restore_active_arena(thd->spcont->callers_arena, &current_arena);
 
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 8fabc8f..80faca6 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -3514,7 +3514,7 @@ int select_max_min_finder_subselect::send_data(List<Item> &items)
   {
     if (!cache)
     {
-      cache= Item_cache::get_cache(thd, val_item);
+      cache= val_item->get_cache(thd);
       switch (val_item->result_type()) {
       case REAL_RESULT:
 	op= &select_max_min_finder_subselect::cmp_real;
diff --git a/sql/sql_type.cc b/sql/sql_type.cc
index e78a3a6..f94e18d 100644
--- a/sql/sql_type.cc
+++ b/sql/sql_type.cc
@@ -40,7 +40,6 @@ static Type_handler_timestamp2  type_handler_timestamp2;
 static Type_handler_olddecimal  type_handler_olddecimal;
 static Type_handler_newdecimal  type_handler_newdecimal;
 static Type_handler_string      type_handler_string;
-static Type_handler_varchar     type_handler_varchar;
 static Type_handler_tiny_blob   type_handler_tiny_blob;
 static Type_handler_medium_blob type_handler_medium_blob;
 static Type_handler_long_blob   type_handler_long_blob;
@@ -54,6 +53,7 @@ static Type_handler_set         type_handler_set;
 
 Type_handler_null        type_handler_null;
 Type_handler_row         type_handler_row;
+Type_handler_varchar     type_handler_varchar;
 
 
 /**
@@ -722,3 +722,42 @@ bool Type_handler_temporal_result::set_comparator_func(Arg_comparator *cmp) cons
 
 
 /*************************************************************************/
+
+Item_cache *
+Type_handler_row::Item_get_cache(THD *thd, const Item *item) const
+{
+  return new (thd->mem_root) Item_cache_row(thd);
+}
+
+Item_cache *
+Type_handler_int_result::Item_get_cache(THD *thd, const Item *item) const
+{
+  return new (thd->mem_root) Item_cache_int(thd, item->field_type());
+}
+
+Item_cache *
+Type_handler_real_result::Item_get_cache(THD *thd, const Item *item) const
+{
+  return new (thd->mem_root) Item_cache_real(thd);
+}
+
+Item_cache *
+Type_handler_decimal_result::Item_get_cache(THD *thd, const Item *item) const
+{
+  return new (thd->mem_root) Item_cache_decimal(thd);
+}
+
+Item_cache *
+Type_handler_string_result::Item_get_cache(THD *thd, const Item *item) const
+{
+  return new (thd->mem_root) Item_cache_str(thd, item);
+}
+
+Item_cache *
+Type_handler_temporal_result::Item_get_cache(THD *thd, const Item *item) const
+{
+  return new (thd->mem_root) Item_cache_temporal(thd, item->field_type());
+}
+
+/*************************************************************************/
+
diff --git a/sql/sql_type.h b/sql/sql_type.h
index f34be07..87cd5c4 100644
--- a/sql/sql_type.h
+++ b/sql/sql_type.h
@@ -25,6 +25,7 @@
 
 class Field;
 class Item;
+class Item_cache;
 class Type_std_attributes;
 class Sort_param;
 class Arg_comparator;
@@ -286,6 +287,7 @@ class Type_handler
 
   virtual int Item_save_in_field(Item *item, Field *field,
                                  bool no_conversions) const= 0;
+  virtual Item_cache *Item_get_cache(THD *thd, const Item *item) const= 0;
   virtual bool set_comparator_func(Arg_comparator *cmp) const= 0;
 };
 
@@ -338,6 +340,7 @@ class Type_handler_row: public Type_handler
     DBUG_ASSERT(0);
     return 1;
   }
+  Item_cache *Item_get_cache(THD *thd, const Item *item) const;
   bool set_comparator_func(Arg_comparator *cmp) const;
 };
 
@@ -356,6 +359,7 @@ class Type_handler_real_result: public Type_handler
                   const Type_std_attributes *item,
                   SORT_FIELD_ATTR *attr) const;
   int Item_save_in_field(Item *item, Field *field, bool no_conversions) const;
+  Item_cache *Item_get_cache(THD *thd, const Item *item) const;
   bool set_comparator_func(Arg_comparator *cmp) const;
 };
 
@@ -373,6 +377,7 @@ class Type_handler_decimal_result: public Type_handler
                   const Type_std_attributes *item,
                   SORT_FIELD_ATTR *attr) const;
   int Item_save_in_field(Item *item, Field *field, bool no_conversions) const;
+  Item_cache *Item_get_cache(THD *thd, const Item *item) const;
   bool set_comparator_func(Arg_comparator *cmp) const;
 };
 
@@ -390,6 +395,7 @@ class Type_handler_int_result: public Type_handler
                   const Type_std_attributes *item,
                   SORT_FIELD_ATTR *attr) const;
   int Item_save_in_field(Item *item, Field *field, bool no_conversions) const;
+  Item_cache *Item_get_cache(THD *thd, const Item *item) const;
   bool set_comparator_func(Arg_comparator *cmp) const;
 };
 
@@ -405,6 +411,7 @@ class Type_handler_temporal_result: public Type_handler
   void sortlength(THD *thd,
                   const Type_std_attributes *item,
                   SORT_FIELD_ATTR *attr) const;
+  Item_cache *Item_get_cache(THD *thd, const Item *item) const;
   bool set_comparator_func(Arg_comparator *cmp) const;
 };
 
@@ -424,6 +431,7 @@ class Type_handler_string_result: public Type_handler
                   const Type_std_attributes *item,
                   SORT_FIELD_ATTR *attr) const;
   int Item_save_in_field(Item *item, Field *field, bool no_conversions) const;
+  Item_cache *Item_get_cache(THD *thd, const Item *item) const;
   bool set_comparator_func(Arg_comparator *cmp) const;
 };
 
@@ -836,5 +844,6 @@ class Type_handler_hybrid_real_field_type:
 
 extern Type_handler_row   type_handler_row;
 extern Type_handler_null  type_handler_null;
+extern Type_handler_varchar type_handler_varchar;
 
 #endif /* SQL_TYPE_H_INCLUDED */