maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

Hello Sanja,

Please review MDEV-11365.

Thanks!

commit aa55bda2506daa56adbeca5351502a6a1b0934af
Author: Alexander Barkov <bar@xxxxxxxxxxx>
Date:   Mon Nov 28 10:21:01 2016 +0400

    MDEV-11365 Split the data type and attribute related code in Item_sum_hybrid::fix_fields into Type_handler::Item_sum_hybrid_fix_length_and_dec()
    
    This patch:
    - Implements the task according to the description
    - Adds a new class Type_handler_numeric as a common parent
      for Type_handler_real_result, Type_handler_int_result,
      Type_handler_decimal_result, to share the common code
      between numeric data type handlers.
    - Removes the dedundant call for collation.set(item->collation) in
      Item_sum_hybrid::setup_hybrid(), because setup_hybrid() is called
      either after fix_length_and_dec() or afte ther constructor
      Item_sum_hybrid(THD *thd, Item_sum_hybrid *item),
      so the collation is already properly set in all cases.

diff --git a/sql/item_sum.cc b/sql/item_sum.cc
index 9ffb056..879115d 100644
--- a/sql/item_sum.cc
+++ b/sql/item_sum.cc
@@ -1126,36 +1126,11 @@ Item_sum_hybrid::fix_fields(THD *thd, Item **ref)
   if ((!item->fixed && item->fix_fields(thd, args)) ||
       (item= args[0])->check_cols(1))
     return TRUE;
-  Type_std_attributes::set(args[0]);
   with_subselect= args[0]->with_subselect;
 
-  Item *item2= item->real_item();
-  if (item2->type() == Item::FIELD_ITEM)
-    set_handler_by_field_type(((Item_field*) item2)->field->type());
-  else if (item->cmp_type() == TIME_RESULT)
-    set_handler_by_field_type(item2->field_type());
-  else
-    set_handler_by_result_type(item2->result_type(),
-                               max_length, collation.collation);
-
-  switch (Item_sum_hybrid::result_type()) {
-  case INT_RESULT:
-  case DECIMAL_RESULT:
-  case STRING_RESULT:
-    break;
-  case REAL_RESULT:
-    max_length= float_length(decimals);
-    break;
-  case ROW_RESULT:
-  case TIME_RESULT:
-    DBUG_ASSERT(0);
-  };
+  fix_length_and_dec();
   setup_hybrid(thd, args[0], NULL);
-  /* MIN/MAX can return NULL for empty set indepedent of the used column */
-  maybe_null= 1;
   result_field=0;
-  null_value=1;
-  fix_length_and_dec();
 
   if (check_sum_func(thd, ref))
     return TRUE;
@@ -1166,6 +1141,14 @@ Item_sum_hybrid::fix_fields(THD *thd, Item **ref)
 }
 
 
+void Item_sum_hybrid::fix_length_and_dec()
+{
+  DBUG_ASSERT(args[0]->field_type() == args[0]->real_item()->field_type());
+  DBUG_ASSERT(args[0]->result_type() == args[0]->real_item()->result_type());
+  (void) args[0]->type_handler()->Item_sum_hybrid_fix_length_and_dec(this);
+}
+
+
 /**
   MIN/MAX function setup.
 
@@ -1201,7 +1184,6 @@ void Item_sum_hybrid::setup_hybrid(THD *thd, Item *item, Item *value_arg)
   cmp= new Arg_comparator();
   if (cmp)
     cmp->set_cmp_func(this, (Item**)&arg_cache, (Item**)&value, FALSE);
-  collation.set(item->collation);
 }
 
 
diff --git a/sql/item_sum.h b/sql/item_sum.h
index 76cc983..59d7587 100644
--- a/sql/item_sum.h
+++ b/sql/item_sum.h
@@ -1006,6 +1006,7 @@ class Item_sum_hybrid :public Item_sum, public Type_handler_hybrid_field_type
     cmp_sign(item->cmp_sign), was_values(item->was_values)
   { }
   bool fix_fields(THD *, Item **);
+  void fix_length_and_dec();
   void setup_hybrid(THD *thd, Item *item, Item *value_arg);
   void clear();
   double val_real();
diff --git a/sql/sql_type.cc b/sql/sql_type.cc
index f94e18d..12fd73d 100644
--- a/sql/sql_type.cc
+++ b/sql/sql_type.cc
@@ -761,3 +761,101 @@ Type_handler_temporal_result::Item_get_cache(THD *thd, const Item *item) const
 
 /*************************************************************************/
 
+/**
+  MAX/MIN for the traditional numeric types preserve the exact data type
+  from Fields, but do not preserve the exact type from Items:
+    MAX(float_field)              -> FLOAT
+    MAX(smallint_field)           -> LONGLONG
+    MAX(COALESCE(float_field))    -> DOUBLE
+    MAX(COALESCE(smallint_field)) -> LONGLONG
+  QQ: Items should probably be fixed to preserve the exact type.
+*/
+bool Type_handler_numeric::
+       Item_sum_hybrid_fix_length_and_dec_numeric(Item_sum_hybrid *func,
+                                                  const Type_handler *handler)
+                                                  const
+{
+  Item *item= func->arguments()[0];
+  Item *item2= item->real_item();
+  func->Type_std_attributes::set(item);
+  /* MIN/MAX can return NULL for empty set indepedent of the used column */
+  func->maybe_null= func->null_value= true;
+  if (item2->type() == Item::FIELD_ITEM)
+    func->set_handler_by_field_type(item2->field_type());
+  else
+    func->set_handler(handler);
+  return false;
+}
+
+
+bool Type_handler_int_result::
+       Item_sum_hybrid_fix_length_and_dec(Item_sum_hybrid *func) const
+{
+  return Item_sum_hybrid_fix_length_and_dec_numeric(func,
+                                                    &type_handler_longlong);
+}
+
+
+bool Type_handler_real_result::
+       Item_sum_hybrid_fix_length_and_dec(Item_sum_hybrid *func) const
+{
+  (void) Item_sum_hybrid_fix_length_and_dec_numeric(func,
+                                                    &type_handler_double);
+  func->max_length= func->float_length(func->decimals);
+  return false;
+}
+
+
+bool Type_handler_decimal_result::
+       Item_sum_hybrid_fix_length_and_dec(Item_sum_hybrid *func) const
+{
+  return Item_sum_hybrid_fix_length_and_dec_numeric(func,
+                                                    &type_handler_newdecimal);
+}
+
+
+/**
+   MAX(str_field) converts ENUM/SET to CHAR, and preserve all other types
+   for Fields.
+   QQ: This works differently from UNION, which preserve the exact data
+   type for ENUM/SET if the joined ENUM/SET fields are equally defined.
+   Perhaps should be fixed.
+   MAX(str_item) chooses the best suitable string type.
+*/
+bool Type_handler_string_result::
+       Item_sum_hybrid_fix_length_and_dec(Item_sum_hybrid *func) const
+{
+  Item *item= func->arguments()[0];
+  Item *item2= item->real_item();
+  func->Type_std_attributes::set(item);
+  func->maybe_null= func->null_value= true;
+  if (item2->type() == Item::FIELD_ITEM)
+  {
+    // Fields: convert ENUM/SET to CHAR, preserve the type otherwise.
+    func->set_handler_by_field_type(item->field_type());
+  }
+  else
+  {
+    // Items: choose VARCHAR/BLOB/MEDIUMBLOB/LONGBLOB, depending on length.
+    func->set_handler(type_handler_varchar.
+          type_handler_adjusted_to_max_octet_length(func->max_length,
+                                                    func->collation.collation));
+  }
+  return false;
+}
+
+
+/**
+  Traditional temporal types always preserve the type of the argument.
+*/
+bool Type_handler_temporal_result::
+       Item_sum_hybrid_fix_length_and_dec(Item_sum_hybrid *func) const
+{
+  Item *item= func->arguments()[0];
+  func->Type_std_attributes::set(item);
+  func->maybe_null= func->null_value= true;
+  func->set_handler(item->type_handler());
+  return false;
+}
+
+/*************************************************************************/
diff --git a/sql/sql_type.h b/sql/sql_type.h
index 87cd5c4..ef85f3d 100644
--- a/sql/sql_type.h
+++ b/sql/sql_type.h
@@ -26,6 +26,7 @@
 class Field;
 class Item;
 class Item_cache;
+class Item_sum_hybrid;
 class Type_std_attributes;
 class Sort_param;
 class Arg_comparator;
@@ -289,6 +290,7 @@ class Type_handler
                                  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;
+  virtual bool Item_sum_hybrid_fix_length_and_dec(Item_sum_hybrid *) const= 0;
 };
 
 
@@ -342,12 +344,31 @@ class Type_handler_row: public Type_handler
   }
   Item_cache *Item_get_cache(THD *thd, const Item *item) const;
   bool set_comparator_func(Arg_comparator *cmp) const;
+  bool Item_sum_hybrid_fix_length_and_dec(Item_sum_hybrid *func) const
+  {
+    DBUG_ASSERT(0);
+    return true;
+  }
+};
+
+
+/*
+  A common parent class for numeric data type handlers
+*/
+class Type_handler_numeric: public Type_handler
+{
+protected:
+  bool Item_sum_hybrid_fix_length_and_dec_numeric(Item_sum_hybrid *func,
+                                                  const Type_handler *handler)
+                                                  const;
+public:
+  virtual ~Type_handler_numeric() { }
 };
 
 
 /*** Abstract classes for every XXX_RESULT */
 
-class Type_handler_real_result: public Type_handler
+class Type_handler_real_result: public Type_handler_numeric
 {
 public:
   Item_result result_type() const { return REAL_RESULT; }
@@ -361,10 +382,11 @@ class Type_handler_real_result: public Type_handler
   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;
+  bool Item_sum_hybrid_fix_length_and_dec(Item_sum_hybrid *func) const;
 };
 
 
-class Type_handler_decimal_result: public Type_handler
+class Type_handler_decimal_result: public Type_handler_numeric
 {
 public:
   Item_result result_type() const { return DECIMAL_RESULT; }
@@ -379,10 +401,11 @@ class Type_handler_decimal_result: public Type_handler
   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;
+  bool Item_sum_hybrid_fix_length_and_dec(Item_sum_hybrid *func) const;
 };
 
 
-class Type_handler_int_result: public Type_handler
+class Type_handler_int_result: public Type_handler_numeric
 {
 public:
   Item_result result_type() const { return INT_RESULT; }
@@ -397,6 +420,7 @@ class Type_handler_int_result: public Type_handler
   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;
+  bool Item_sum_hybrid_fix_length_and_dec(Item_sum_hybrid *func) const;
 };
 
 
@@ -413,6 +437,7 @@ class Type_handler_temporal_result: public Type_handler
                   SORT_FIELD_ATTR *attr) const;
   Item_cache *Item_get_cache(THD *thd, const Item *item) const;
   bool set_comparator_func(Arg_comparator *cmp) const;
+  bool Item_sum_hybrid_fix_length_and_dec(Item_sum_hybrid *func) const;
 };
 
 
@@ -433,6 +458,7 @@ class Type_handler_string_result: public Type_handler
   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;
+  bool Item_sum_hybrid_fix_length_and_dec(Item_sum_hybrid *func) const;
 };