MDEV-7366 SELECT 'a' = BINARY 'A' returns 1 (utf8 charset, utf8_unicode_ci collation)

Fixing a wrong assymetric code in Arg_comparator::set_cmp_func().
It existed for a long time, but showed up in 10.0.14 after the fix
for "MDEV-6666 Malformed result for CONCAT(utf8_column, binary_string)".

mysql-test/r/ctype_uca.result

@@ -13126,5 +13126,15 @@ DROP TABLE t1;
 # END of ctype_myanmar.inc
 #
 #
+# MDEV-7366 SELECT 'a' = BINARY 'A' returns 1 (utf8 charset, utf8_unicode_ci collation)
+#
+SET NAMES utf8 COLLATE utf8_unicode_ci;
+SELECT 'a' = BINARY 'A';
+'a' = BINARY 'A'
+0
+SELECT BINARY 'A' = 'a';
+BINARY 'A' = 'a'
+0
+#
 # End of MariaDB-10.0 tests
 #

mysql-test/t/ctype_uca.test

@@ -587,6 +587,13 @@ SET NAMES utf8 COLLATE utf8_myanmar_ci;
 SET collation_connection=ucs2_myanmar_ci;
 --source include/ctype_myanmar.inc
 
+--echo #
+--echo # MDEV-7366 SELECT 'a' = BINARY 'A' returns 1 (utf8 charset, utf8_unicode_ci collation)
+--echo #
+SET NAMES utf8 COLLATE utf8_unicode_ci; 
+SELECT 'a' = BINARY 'A';
+SELECT BINARY 'A' = 'a';
+
 --echo #
 --echo # End of MariaDB-10.0 tests
 --echo #

sql/item_cmpfunc.cc

@@ -621,17 +621,6 @@ int Arg_comparator::set_compare_func(Item_result_field *item, Item_result type)
   }
   case STRING_RESULT:
   {
-    /*
-      We must set cmp_charset here as we may be called from for an automatic
-      generated item, like in natural join
-    */
-    if (cmp_collation.set((*a)->collation, (*b)->collation) || 
-	cmp_collation.derivation == DERIVATION_NONE)
-    {
-      my_coll_agg_error((*a)->collation, (*b)->collation,
-                        owner->func_name());
-      return 1;
-    }
     if (cmp_collation.collation == &my_charset_bin)
     {
       /*
@@ -754,6 +743,37 @@ 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()
+{
+  if (cmp_collation.set((*a)->collation, (*b)->collation, MY_COLL_CMP_CONV) ||
+      cmp_collation.derivation == DERIVATION_NONE)
+  {
+    my_coll_agg_error((*a)->collation, (*b)->collation, owner->func_name());
+    return true;
+  }
+  if (agg_item_set_converter(cmp_collation, owner->func_name(),
+                             a, 1, MY_COLL_CMP_CONV, 1) ||
+      agg_item_set_converter(cmp_collation, owner->func_name(),
+                             b, 1, MY_COLL_CMP_CONV, 1))
+    return true;
+  return false;
+}
+
+
 /**
   Prepare the comparator (set the comparison function) for comparing
   items *a1 and *a2 in the context of 'type'.
@@ -781,10 +801,11 @@ int Arg_comparator::set_cmp_func(Item_result_field *owner_arg,
       (*a)->result_type() == STRING_RESULT &&
       (*b)->result_type() == STRING_RESULT)
   {
-    DTCollation coll;
-    coll.set((*a)->collation.collation);
-    if (agg_item_set_converter(coll, owner->func_name(),
-                               b, 1, MY_COLL_CMP_CONV, 1))
+    /*
+      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())
       return 1;
   }
   if (type == INT_RESULT &&

sql/item_cmpfunc.h

@@ -48,6 +48,14 @@ class Arg_comparator: public Sql_alloc
   THD *thd;
   Item *a_cache, *b_cache;         // Cached values of a and b items
                                    //   when one of arguments is NULL.
+  int set_compare_func(Item_result_field *owner, Item_result type);
+  inline int set_compare_func(Item_result_field *owner_arg)
+  {
+    return set_compare_func(owner_arg, item_cmp_type((*a)->result_type(),
+                                                     (*b)->result_type()));
+  }
+  bool agg_arg_charsets_for_comparison();
+
 public:
   DTCollation cmp_collation;
   /* Allow owner function to use string buffers. */
@@ -58,12 +66,6 @@ class Arg_comparator: public Sql_alloc
   Arg_comparator(Item **a1, Item **a2): a(a1), b(a2),  set_null(TRUE),
     comparators(0), thd(0), a_cache(0), b_cache(0) {};
 
-  int set_compare_func(Item_result_field *owner, Item_result type);
-  inline int set_compare_func(Item_result_field *owner_arg)
-  {
-    return set_compare_func(owner_arg, item_cmp_type((*a)->result_type(),
-                                                     (*b)->result_type()));
-  }
   int set_cmp_func(Item_result_field *owner_arg,
 			  Item **a1, Item **a2,
 			  Item_result type);