← Back to team overview

maria-developers team mailing list archive

Please review MDEV-12338 Split Item_type_holder::get_real_type() into virtual Item::real_type_handler()

 

Hi Vicențiu,

can you please review a patch for MDEV-12338?

Thanks!
diff --git a/sql/item.cc b/sql/item.cc
index b0337ef..2f2b773 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -5670,6 +5670,25 @@ bool Item_field::fix_fields(THD *thd, Item **reference)
   return TRUE;
 }
 
+
+const Type_handler *Item_field::real_type_handler() const
+{
+  /*
+    Item_field::field_type ask Field_type() but sometimes field return
+    a different type, like for enum/set, so we need to ask real type.
+  */
+  if (field->is_created_from_null_item)
+    return &type_handler_null;
+  /* work around about varchar type field detection */
+  enum_field_types type= field->real_type();
+  // TODO: We should add Field::real_type_handler() eventually
+  if (type == MYSQL_TYPE_STRING && field->type() == MYSQL_TYPE_VAR_STRING)
+    type= MYSQL_TYPE_VAR_STRING;
+  return Type_handler::get_handler_by_real_type(type);
+
+}
+
+
 /*
   @brief
   Mark virtual columns as used in a partitioning expression 
@@ -10009,7 +10028,7 @@ void Item_cache_row::set_null()
 
 Item_type_holder::Item_type_holder(THD *thd, Item *item)
   :Item(thd, item),
-   Type_handler_hybrid_real_field_type(get_real_type(item)),
+   Type_handler_hybrid_field_type(item->real_type_handler()),
    enum_set_typelib(0)
 {
   DBUG_ASSERT(item->fixed);
@@ -10025,87 +10044,6 @@ Item_type_holder::Item_type_holder(THD *thd, Item *item)
 
 
 /**
-  Find real field type of item.
-
-  @return
-    type of field which should be created to store item value
-*/
-
-enum_field_types Item_type_holder::get_real_type(Item *item)
-{
-  if (item->type() == REF_ITEM)
-    item= item->real_item();
-  switch(item->type())
-  {
-  case FIELD_ITEM:
-  {
-    /*
-      Item_field::field_type ask Field_type() but sometimes field return
-      a different type, like for enum/set, so we need to ask real type.
-    */
-    Field *field= ((Item_field *) item)->field;
-    enum_field_types type= field->real_type();
-    if (field->is_created_from_null_item)
-      return MYSQL_TYPE_NULL;
-    /* work around about varchar type field detection */
-    if (type == MYSQL_TYPE_STRING && field->type() == MYSQL_TYPE_VAR_STRING)
-      return MYSQL_TYPE_VAR_STRING;
-    return type;
-  }
-  case SUM_FUNC_ITEM:
-  {
-    /*
-      Argument of aggregate function sometimes should be asked about field
-      type
-    */
-    Item_sum *item_sum= (Item_sum *) item;
-    if (item_sum->keep_field_type())
-      return get_real_type(item_sum->get_arg(0));
-    break;
-  }
-  case FUNC_ITEM:
-    if (((Item_func *) item)->functype() == Item_func::GUSERVAR_FUNC)
-    {
-      /*
-        There are work around of problem with changing variable type on the
-        fly and variable always report "string" as field type to get
-        acceptable information for client in send_field, so we make field
-        type from expression type.
-      */
-      switch (item->result_type()) {
-      case STRING_RESULT:
-        return MYSQL_TYPE_VARCHAR;
-      case INT_RESULT:
-        return MYSQL_TYPE_LONGLONG;
-      case REAL_RESULT:
-        return MYSQL_TYPE_DOUBLE;
-      case DECIMAL_RESULT:
-        return MYSQL_TYPE_NEWDECIMAL;
-      case ROW_RESULT:
-      case TIME_RESULT:
-        DBUG_ASSERT(0);
-        return MYSQL_TYPE_VARCHAR;
-      }
-    }
-    break;
-  case TYPE_HOLDER:
-    /*
-      Item_type_holder and Item_blob should not appear in this context.
-      In case they for some reasons do, returning field_type() is wrong anyway.
-      They must return Item_type_holder::real_field_type() instead, to make
-      the code in sql_type.cc and sql_type.h happy, as it expectes
-      Field::real_type()-compatible rather than Field::field_type()-compatible
-      valies in some places, and may in the future add some asserts preventing
-      use of field_type() instead of real_type() and the other way around.
-    */
-    DBUG_ASSERT(0);
-  default:
-    break;
-  }
-  return item->field_type();
-}
-
-/**
   Find field type which can carry current Item_type_holder type and
   type of given Item.
 
@@ -10123,14 +10061,13 @@ bool Item_type_holder::join_types(THD *thd, Item *item)
   uint max_length_orig= max_length;
   uint decimals_orig= decimals;
   DBUG_ENTER("Item_type_holder::join_types");
-  DBUG_PRINT("info:", ("was type %d len %d, dec %d name %s",
-                       real_field_type(), max_length, decimals,
+  DBUG_PRINT("info:", ("was type %s len %d, dec %d name %s",
+                       real_type_handler()->name().ptr(), max_length, decimals,
                        (name ? name : "<NULL>")));
-  DBUG_PRINT("info:", ("in type %d len %d, dec %d",
-                       get_real_type(item),
+  DBUG_PRINT("info:", ("in type %s len %d, dec %d",
+                       item->real_type_handler()->name().ptr(),
                        item->max_length, item->decimals));
-  const Type_handler *item_type_handler=
-    Type_handler::get_handler_by_real_type(get_real_type(item));
+  const Type_handler *item_type_handler= item->real_type_handler();
   if (aggregate_for_result(item_type_handler))
   {
     my_error(ER_ILLEGAL_PARAMETER_DATA_TYPES2_FOR_OPERATION, MYF(0),
@@ -10218,13 +10155,13 @@ bool Item_type_holder::join_types(THD *thd, Item *item)
         int delta1= max_length_orig - decimals_orig;
         int delta2= item->max_length - item->decimals;
         max_length= MY_MAX(delta1, delta2) + decimals;
-        if (Item_type_holder::real_field_type() == MYSQL_TYPE_FLOAT &&
+        if (Item_type_holder::real_type_handler() == &type_handler_float &&
             max_length > FLT_DIG + 2)
         {
           max_length= MAX_FLOAT_STR_LENGTH;
           decimals= NOT_FIXED_DEC;
         } 
-        else if (Item_type_holder::real_field_type() == MYSQL_TYPE_DOUBLE &&
+        else if (Item_type_holder::real_type_handler() == &type_handler_double &&
                  max_length > DBL_DIG + 2)
         {
           max_length= MAX_DOUBLE_STR_LENGTH;
@@ -10245,8 +10182,9 @@ bool Item_type_holder::join_types(THD *thd, Item *item)
 
   /* Remember decimal integer part to be used in DECIMAL_RESULT handleng */
   prev_decimal_int_part= decimal_int_part();
-  DBUG_PRINT("info", ("become type: %d  len: %u  dec: %u",
-                      (int) real_field_type(), max_length, (uint) decimals));
+  DBUG_PRINT("info", ("become type: %s  len: %u  dec: %u",
+                      real_type_handler()->name().ptr(),
+                      max_length, (uint) decimals));
   DBUG_RETURN(FALSE);
 }
 
@@ -10269,7 +10207,7 @@ Field *Item_type_holder::make_field_by_type(TABLE *table)
   uchar *null_ptr= maybe_null ? (uchar*) "" : 0;
   Field *field;
 
-  switch (Item_type_holder::real_field_type()) {
+  switch (Item_type_holder::real_type_handler()->real_field_type()) {
   case MYSQL_TYPE_ENUM:
     DBUG_ASSERT(enum_set_typelib);
     field= new Field_enum((uchar *) 0, max_length, null_ptr, 0,
@@ -10305,8 +10243,8 @@ Field *Item_type_holder::make_field_by_type(TABLE *table)
 */
 void Item_type_holder::get_full_info(Item *item)
 {
-  if (Item_type_holder::real_field_type() == MYSQL_TYPE_ENUM ||
-      Item_type_holder::real_field_type() == MYSQL_TYPE_SET)
+  if (Item_type_holder::real_type_handler() == &type_handler_enum ||
+      Item_type_holder::real_type_handler() == &type_handler_set)
   {
     if (item->type() == Item::SUM_FUNC_ITEM &&
         (((Item_sum*)item)->sum_func() == Item_sum::MAX_FUNC ||
@@ -10317,11 +10255,11 @@ void Item_type_holder::get_full_info(Item *item)
       field (or MIN|MAX(enum|set field)) and number of NULL fields
     */
     DBUG_ASSERT((enum_set_typelib &&
-                 get_real_type(item) == MYSQL_TYPE_NULL) ||
+                 item->real_type_handler() == &type_handler_null) ||
                 (!enum_set_typelib &&
                  item->real_item()->type() == Item::FIELD_ITEM &&
-                 (get_real_type(item->real_item()) == MYSQL_TYPE_ENUM ||
-                  get_real_type(item->real_item()) == MYSQL_TYPE_SET) &&
+                 (item->real_type_handler() == &type_handler_enum ||
+                  item->real_type_handler() == &type_handler_set) &&
                  ((Field_enum*)((Item_field *) item->real_item())->field)->typelib));
     if (!enum_set_typelib)
     {
diff --git a/sql/item.h b/sql/item.h
index d3b1189..e7086a5 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -699,6 +699,10 @@ class Item: public Value_source,
   {
     return Type_handler::get_handler_by_field_type(field_type());
   }
+  virtual const Type_handler *real_type_handler() const
+  {
+    return type_handler();
+  }
   virtual const Type_handler *cast_to_int_type_handler() const
   {
     return type_handler();
@@ -2409,6 +2413,7 @@ class Item_field :public Item_ident
   {
     return field->type();
   }
+  const Type_handler *real_type_handler() const;
   enum_monotonicity_info get_monotonicity_info() const
   {
     return MONOTONIC_STRICT_INCREASING;
@@ -3355,6 +3360,12 @@ class Item_blob :public Item_partition_func_safe_string
   { max_length= length; }
   enum Type type() const { return TYPE_HOLDER; }
   enum_field_types field_type() const { return MYSQL_TYPE_BLOB; }
+  const Type_handler *real_type_handler() const
+  {
+    // Should not be called, Item_blob is used for SHOW purposes only.
+    DBUG_ASSERT(0);
+    return &type_handler_varchar;
+  }
   Field *create_field_for_schema(THD *thd, TABLE *table)
   { return tmp_table_field_from_field_type(table, false, true); }
 };
@@ -4072,6 +4083,8 @@ class Item_ref :public Item_ident
   { return (*ref)->setup_fast_field_copier(field); }
   enum Item_result result_type () const { return (*ref)->result_type(); }
   enum_field_types field_type() const   { return (*ref)->field_type(); }
+  const Type_handler *real_type_handler() const
+  { return (*ref)->real_type_handler(); }
   Field *get_tmp_table_field()
   { return result_field ? result_field : (*ref)->get_tmp_table_field(); }
   Item *get_tmp_table_item(THD *thd);
@@ -5602,7 +5615,7 @@ class Item_cache_row: public Item_cache
   single SP/PS execution.
 */
 class Item_type_holder: public Item,
-                        public Type_handler_hybrid_real_field_type
+                        public Type_handler_hybrid_field_type
 {
 protected:
   TYPELIB *enum_set_typelib;
@@ -5616,11 +5629,11 @@ class Item_type_holder: public Item,
   Item_type_holder(THD*, Item*);
 
   const Type_handler *type_handler() const
-  { return Type_handler_hybrid_real_field_type::type_handler(); }
+  { return Type_handler_hybrid_field_type::type_handler(); }
   enum_field_types field_type() const
-  { return Type_handler_hybrid_real_field_type::field_type(); }
+  { return Type_handler_hybrid_field_type::field_type(); }
   enum_field_types real_field_type() const
-  { return Type_handler_hybrid_real_field_type::real_field_type(); }
+  { return Type_handler_hybrid_field_type::real_field_type(); }
   enum Item_result result_type () const
   {
     /*
@@ -5635,7 +5648,11 @@ class Item_type_holder: public Item,
       As soon as we get BIT as one of the joined types, the result field
       type cannot be numeric: it's either BIT, or VARBINARY.
     */
-    return Type_handler_hybrid_real_field_type::result_type();
+    return Type_handler_hybrid_field_type::result_type();
+  }
+  const Type_handler *real_type_handler() const
+  {
+    return Item_type_holder::type_handler();
   }
 
   enum Type type() const { return TYPE_HOLDER; }
@@ -5645,7 +5662,6 @@ class Item_type_holder: public Item,
   String *val_str(String*);
   bool join_types(THD *thd, Item *);
   Field *make_field_by_type(TABLE *table);
-  static enum_field_types get_real_type(Item *);
   Field::geometry_type get_geometry_type() const { return geometry_type; };
   Item* get_copy(THD *thd, MEM_ROOT *mem_root) { return 0; }
 };
diff --git a/sql/item_sum.h b/sql/item_sum.h
index 3ceb2a0..ee1c10f 100644
--- a/sql/item_sum.h
+++ b/sql/item_sum.h
@@ -456,7 +456,6 @@ class Item_sum :public Item_func_or_sum
     Updated value is then saved in the field.
   */
   virtual void update_field()=0;
-  virtual bool keep_field_type(void) const { return 0; }
   virtual void fix_length_and_dec() { maybe_null=1; null_value=1; }
   virtual Item *result_item(THD *thd, Field *field);
 
@@ -520,7 +519,7 @@ class Item_sum :public Item_func_or_sum
   st_select_lex *depended_from() 
     { return (nest_level == aggr_level ? 0 : aggr_sel); }
 
-  Item *get_arg(uint i) { return args[i]; }
+  Item *get_arg(uint i) const { return args[i]; }
   Item *set_arg(uint i, THD *thd, Item *new_val);
   uint get_arg_count() const { return arg_count; }
 
@@ -1047,7 +1046,10 @@ class Item_sum_hybrid :public Item_sum, public Type_handler_hybrid_field_type
   my_decimal *val_decimal(my_decimal *);
   void reset_field();
   String *val_str(String *);
-  bool keep_field_type(void) const { return 1; }
+  const Type_handler *real_type_handler() const
+  {
+    return get_arg(0)->real_type_handler();
+  }
   const Type_handler *type_handler() const
   { return Type_handler_hybrid_field_type::type_handler(); }
   enum Item_result result_type () const
diff --git a/sql/item_windowfunc.h b/sql/item_windowfunc.h
index b4953c8..193ac04 100644
--- a/sql/item_windowfunc.h
+++ b/sql/item_windowfunc.h
@@ -303,8 +303,6 @@ class Item_sum_hybrid_simple : public Item_sum,
   my_decimal *val_decimal(my_decimal *);
   void reset_field();
   String *val_str(String *);
-  /* TODO(cvicentiu) copied from Item_sum_hybrid, what does it do? */
-  bool keep_field_type(void) const { return 1; }
   enum Item_result result_type() const
   { return Type_handler_hybrid_field_type::result_type(); }
   enum Item_result cmp_type() const
diff --git a/sql/sql_type.cc b/sql/sql_type.cc
index 7044f7e..7f64072 100644
--- a/sql/sql_type.cc
+++ b/sql/sql_type.cc
@@ -25,7 +25,6 @@ static Type_handler_short       type_handler_short;
 static Type_handler_long        type_handler_long;
 static Type_handler_int24       type_handler_int24;
 static Type_handler_year        type_handler_year;
-static Type_handler_float       type_handler_float;
 static Type_handler_time        type_handler_time;
 static Type_handler_time2       type_handler_time2;
 static Type_handler_date        type_handler_date;
@@ -39,18 +38,19 @@ 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;
 static Type_handler_blob        type_handler_blob;
-static Type_handler_enum        type_handler_enum;
-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;
 Type_handler_longlong    type_handler_longlong;
+Type_handler_float       type_handler_float;
 Type_handler_double      type_handler_double;
 Type_handler_newdecimal  type_handler_newdecimal;
 Type_handler_datetime    type_handler_datetime;
 Type_handler_bit         type_handler_bit;
+Type_handler_enum        type_handler_enum;
+Type_handler_set         type_handler_set;
 
 #ifdef HAVE_SPATIAL
 Type_handler_geometry    type_handler_geometry;
diff --git a/sql/sql_type.h b/sql/sql_type.h
index 3ac9324..02d6d93 100644
--- a/sql/sql_type.h
+++ b/sql/sql_type.h
@@ -1450,29 +1450,19 @@ class Type_handler_hybrid_field_type
 };
 
 
-/**
-  This class is used for Item_type_holder, which preserves real_type.
-*/
-class Type_handler_hybrid_real_field_type:
-  public Type_handler_hybrid_field_type
-{
-public:
-  Type_handler_hybrid_real_field_type(enum_field_types type)
-    :Type_handler_hybrid_field_type(Type_handler::
-                                    get_handler_by_real_type(type))
-  { }
-};
-
-
 extern Type_handler_row   type_handler_row;
 extern Type_handler_null  type_handler_null;
 extern Type_handler_varchar type_handler_varchar;
 extern Type_handler_longlong type_handler_longlong;
+extern Type_handler_float type_handler_float;
 extern Type_handler_double type_handler_double;
 extern Type_handler_newdecimal type_handler_newdecimal;
 extern Type_handler_datetime type_handler_datetime;
 extern Type_handler_longlong type_handler_longlong;
 extern Type_handler_bit type_handler_bit;
+extern Type_handler_enum type_handler_enum;
+extern Type_handler_set type_handler_set;
+
 
 class Type_aggregator
 {