Changing a number of functions to aggregate argument character sets

and collations from the global name space into private and protected
methods in Item_func_or_sum.

Author: Alexander Barkov

sql/item.cc

@@ -2035,8 +2035,9 @@ void my_coll_agg_error(Item** args, uint count, const char *fname,
 }
 
 
-bool agg_item_collations(DTCollation &c, const char *fname,
-                         Item **av, uint count, uint flags, int item_sep)
+bool Item_func_or_sum::agg_item_collations(DTCollation &c, const char *fname,
+                                           Item **av, uint count,
+                                           uint flags, int item_sep)
 {
   uint i;
   Item **arg;
@@ -2081,16 +2082,10 @@ bool agg_item_collations(DTCollation &c, const char *fname,
 }
 
 
-bool agg_item_collations_for_comparison(DTCollation &c, const char *fname,
-                                        Item **av, uint count, uint flags)
-{
-  return (agg_item_collations(c, fname, av, count,
-                              flags | MY_COLL_DISALLOW_NONE, 1));
-}
-
-
-bool agg_item_set_converter(DTCollation &coll, const char *fname,
-                            Item **args, uint nargs, uint flags, int item_sep)
+bool Item_func_or_sum::agg_item_set_converter(const DTCollation &coll,
+                                              const char *fname,
+                                              Item **args, uint nargs,
+                                              uint flags, int item_sep)
 {
   Item **arg, *safe_args[2]= {NULL, NULL};
 
@@ -2167,46 +2162,6 @@ bool agg_item_set_converter(DTCollation &coll, const char *fname,
 }
 
 
-/* 
-  Collect arguments' character sets together.
-  We allow to apply automatic character set conversion in some cases.
-  The conditions when conversion is possible are:
-  - arguments A and B have different charsets
-  - A wins according to coercibility rules
-    (i.e. a column is stronger than a string constant,
-     an explicit COLLATE clause is stronger than a column)
-  - character set of A is either superset for character set of B,
-    or B is a string constant which can be converted into the
-    character set of A without data loss.
-    
-  If all of the above is true, then it's possible to convert
-  B into the character set of A, and then compare according
-  to the collation of A.
-  
-  For functions with more than two arguments:
-
-    collect(A,B,C) ::= collect(collect(A,B),C)
-
-  Since this function calls THD::change_item_tree() on the passed Item **
-  pointers, it is necessary to pass the original Item **'s, not copies.
-  Otherwise their values will not be properly restored (see BUG#20769).
-  If the items are not consecutive (eg. args[2] and args[5]), use the
-  item_sep argument, ie.
-
-    agg_item_charsets(coll, fname, &args[2], 2, flags, 3)
-
-*/
-
-bool agg_item_charsets(DTCollation &coll, const char *fname,
-                       Item **args, uint nargs, uint flags, int item_sep)
-{
-  if (agg_item_collations(coll, fname, args, nargs, flags, item_sep))
-    return TRUE;
-
-  return agg_item_set_converter(coll, fname, args, nargs, flags, item_sep);
-}
-
-
 void Item_ident_for_show::make_field(Send_field *tmp_field)
 {
   tmp_field->table_name= tmp_field->org_table_name= table_name;

sql/item.h

@@ -2153,38 +2153,6 @@ class Item_name_const : public Item
   }
 };
 
-bool agg_item_collations(DTCollation &c, const char *name,
-                         Item **items, uint nitems, uint flags, int item_sep);
-bool agg_item_collations_for_comparison(DTCollation &c, const char *name,
-                                        Item **items, uint nitems, uint flags);
-bool agg_item_set_converter(DTCollation &coll, const char *fname,
-                            Item **args, uint nargs, uint flags, int item_sep);
-bool agg_item_charsets(DTCollation &c, const char *name,
-                       Item **items, uint nitems, uint flags, int item_sep);
-inline bool
-agg_item_charsets_for_comparison(DTCollation &c, const char *name,
-                                 Item **items, uint nitems,
-                                 int item_sep= 1)
-{
-  uint flags= MY_COLL_ALLOW_SUPERSET_CONV |
-              MY_COLL_ALLOW_COERCIBLE_CONV |
-              MY_COLL_DISALLOW_NONE;
-  return agg_item_charsets(c, name, items, nitems, flags, item_sep);
-}
-inline bool
-agg_item_charsets_for_string_result_with_comparison(DTCollation &c,
-                                                    const char *name,
-                                                    Item **items, uint nitems,
-                                                    int item_sep= 1)
-{
-  uint flags= MY_COLL_ALLOW_SUPERSET_CONV |
-              MY_COLL_ALLOW_COERCIBLE_CONV |
-              MY_COLL_ALLOW_NUMERIC_CONV |
-              MY_COLL_DISALLOW_NONE;
-  return agg_item_charsets(c, name, items, nitems, flags, item_sep);
-}
-
-
 class Item_num: public Item_basic_constant
 {
 public:
@@ -3616,7 +3584,50 @@ class Used_tables_and_const_cache
 */
 class Item_func_or_sum: public Item_result_field, public Item_args
 {
+  bool agg_item_collations(DTCollation &c, const char *name,
+                           Item **items, uint nitems,
+                           uint flags, int item_sep);
+  bool agg_item_set_converter(const DTCollation &coll, const char *fname,
+                              Item **args, uint nargs,
+                              uint flags, int item_sep);
 protected:
+  /*
+    Collect arguments' character sets together.
+    We allow to apply automatic character set conversion in some cases.
+    The conditions when conversion is possible are:
+    - arguments A and B have different charsets
+    - A wins according to coercibility rules
+      (i.e. a column is stronger than a string constant,
+       an explicit COLLATE clause is stronger than a column)
+    - character set of A is either superset for character set of B,
+      or B is a string constant which can be converted into the
+      character set of A without data loss.
+
+    If all of the above is true, then it's possible to convert
+    B into the character set of A, and then compare according
+    to the collation of A.
+
+    For functions with more than two arguments:
+
+      collect(A,B,C) ::= collect(collect(A,B),C)
+
+    Since this function calls THD::change_item_tree() on the passed Item **
+    pointers, it is necessary to pass the original Item **'s, not copies.
+    Otherwise their values will not be properly restored (see BUG#20769).
+    If the items are not consecutive (eg. args[2] and args[5]), use the
+    item_sep argument, ie.
+
+      agg_item_charsets(coll, fname, &args[2], 2, flags, 3)
+  */
+  bool agg_arg_charsets(DTCollation &c, Item **items, uint nitems,
+                        uint flags, int item_sep)
+  {
+    if (agg_item_collations(c, func_name(), items, nitems, flags, item_sep))
+      return true;
+
+    return agg_item_set_converter(c, func_name(), items, nitems,
+                                  flags, item_sep);
+  }
   /*
     Aggregate arguments for string result, e.g: CONCAT(a,b)
     - convert to @@character_set_connection if all arguments are numbers
@@ -3629,7 +3640,7 @@ class Item_func_or_sum: public Item_result_field, public Item_args
     uint flags= MY_COLL_ALLOW_SUPERSET_CONV |
                 MY_COLL_ALLOW_COERCIBLE_CONV |
                 MY_COLL_ALLOW_NUMERIC_CONV;
-    return agg_item_charsets(c, func_name(), items, nitems, flags, item_sep);
+    return agg_arg_charsets(c, items, nitems, flags, item_sep);
   }
   /*
     Aggregate arguments for string result, when some comparison
@@ -3646,7 +3657,48 @@ class Item_func_or_sum: public Item_result_field, public Item_args
                 MY_COLL_ALLOW_COERCIBLE_CONV |
                 MY_COLL_ALLOW_NUMERIC_CONV |
                 MY_COLL_DISALLOW_NONE;
-    return agg_item_charsets(c, func_name(), items, nitems, flags, item_sep);
+    return agg_arg_charsets(c, items, nitems, flags, item_sep);
+  }
+
+  /*
+    Aggregate arguments for comparison, e.g: a=b, a LIKE b, a RLIKE b
+    - don't convert to @@character_set_connection if all arguments are numbers
+    - don't allow DERIVATION_NONE
+  */
+  bool agg_arg_charsets_for_comparison(DTCollation &c,
+                                       Item **items, uint nitems,
+                                       int item_sep= 1)
+  {
+    uint flags= MY_COLL_ALLOW_SUPERSET_CONV |
+                MY_COLL_ALLOW_COERCIBLE_CONV |
+                MY_COLL_DISALLOW_NONE;
+    return agg_arg_charsets(c, items, nitems, flags, item_sep);
+  }
+
+
+public:
+  // This method is used by Arg_comparator
+  bool agg_arg_charsets_for_comparison(CHARSET_INFO **cs, Item **a, Item **b)
+  {
+    DTCollation tmp;
+    if (tmp.set((*a)->collation, (*b)->collation, MY_COLL_CMP_CONV) ||
+        tmp.derivation == DERIVATION_NONE)
+    {
+      my_error(ER_CANT_AGGREGATE_2COLLATIONS,MYF(0),
+               (*a)->collation.collation->name,
+               (*a)->collation.derivation_name(),
+               (*b)->collation.collation->name,
+               (*b)->collation.derivation_name(),
+               func_name());
+      return true;
+    }
+    if (agg_item_set_converter(tmp, func_name(),
+                               a, 1, MY_COLL_CMP_CONV, 1) ||
+        agg_item_set_converter(tmp, func_name(),
+                               b, 1, MY_COLL_CMP_CONV, 1))
+      return true;
+    *cs= tmp.collation;
+    return false;
   }
 
 public:

sql/item_cmpfunc.cc

@@ -253,15 +253,6 @@ static uint collect_cmp_types(Item **items, uint nitems, bool skip_nulls= FALSE)
   return found_types;
 }
 
-static void my_coll_agg_error(DTCollation &c1, DTCollation &c2,
-                              const char *fname)
-{
-  my_error(ER_CANT_AGGREGATE_2COLLATIONS, MYF(0),
-           c1.collation->name,c1.derivation_name(),
-           c2.collation->name,c2.derivation_name(),
-           fname);
-}
-
 
 /*
   Test functions
@@ -653,39 +644,6 @@ bool get_mysql_time_from_str(THD *thd, String *str, timestamp_type warn_type,
 }
 
 
-/**
-  Aggregate comparator argument charsets for comparison.
-  One of the arguments ("a" or "b") can be replaced,
-  typically by Item_string or Item_func_conv_charset.
-
-  @return Aggregation result
-  @retval false - if no conversion is needed,
-                  or if one of the arguments was converted
-  @retval true  - on error, if arguments are not comparable.
-
-  TODO: get rid of this method eventually and refactor the calling code.
-  Argument conversion should happen on the Item_func level.
-  Arg_comparator should get comparable arguments.
-*/
-bool Arg_comparator::agg_arg_charsets_for_comparison()
-{
-  DTCollation tmp;
-  if (tmp.set((*a)->collation, (*b)->collation, MY_COLL_CMP_CONV) ||
-      tmp.derivation == DERIVATION_NONE)
-  {
-    my_coll_agg_error((*a)->collation, (*b)->collation, owner->func_name());
-    return true;
-  }
-  if (agg_item_set_converter(tmp, owner->func_name(),
-                             a, 1, MY_COLL_CMP_CONV, 1) ||
-      agg_item_set_converter(tmp, owner->func_name(),
-                             b, 1, MY_COLL_CMP_CONV, 1))
-    return true;
-  m_compare_collation= tmp.collation;
-  return false;
-}
-
-
 /**
   Prepare the comparator (set the comparison function) for comparing
   items *a1 and *a2 in the context of 'type'.
@@ -717,7 +675,7 @@ int Arg_comparator::set_cmp_func(Item_func_or_sum *owner_arg,
       We must set cmp_collation here as we may be called from for an automatic
       generated item, like in natural join
     */
-    if (agg_arg_charsets_for_comparison())
+    if (owner->agg_arg_charsets_for_comparison(&m_compare_collation, a, b))
       return 1;
   }
 

sql/item_cmpfunc.h

@@ -60,7 +60,6 @@ class Arg_comparator: public Sql_alloc
                                    //   when one of arguments is NULL.
   int set_compare_func(Item_func_or_sum *owner, Item_result type);
   int set_cmp_func(Item_func_or_sum *owner_arg, Item **a1, Item **a2);
-  bool agg_arg_charsets_for_comparison();
 
   int compare_temporal(enum_field_types type);
   int compare_e_temporal(enum_field_types type);

sql/item_func.h

@@ -187,23 +187,6 @@ class Item_func :public Item_func_or_sum, public Used_tables_and_const_cache
     else
       max_length= (uint32) max_result_length;
   }
-  bool agg_arg_charsets(DTCollation &c, Item **items, uint nitems,
-                        uint flags, int item_sep)
-  {
-    return agg_item_charsets(c, func_name(), items, nitems, flags, item_sep);
-  }
-  /*
-    Aggregate arguments for comparison, e.g: a=b, a LIKE b, a RLIKE b
-    - don't convert to @@character_set_connection if all arguments are numbers
-    - don't allow DERIVATION_NONE
-  */
-  bool agg_arg_charsets_for_comparison(DTCollation &c,
-                                       Item **items, uint nitems,
-                                       int item_sep= 1)
-  {
-    return agg_item_charsets_for_comparison(c, func_name(),
-                                            items, nitems, item_sep);
-  }
   Item *transform(THD *thd, Item_transformer transformer, uchar *arg);
   Item* compile(THD *thd, Item_analyzer analyzer, uchar **arg_p,
                 Item_transformer transformer, uchar *arg_t);