MDEV-8705 Wrong result for SELECT..WHERE latin1_bin_column='a' AND latin1_bin_column='A'

MDEV-8712 Wrong result for SELECT..WHERE latin1_bin_column=_latin1'a' AND latin1_bin_column='A'

Author: Alexander Barkov

mysql-test/r/ctype_latin1.result

@@ -8149,5 +8149,24 @@ Warnings:
 Note	1003	select `test`.`t1`.`a` AS `a` from `test`.`t1` where ((`test`.`t1`.`a` = 'a') and (length(`test`.`t1`.`a`) = 2))
 DROP TABLE t1;
 #
+# MDEV-8712 Wrong result for SELECT..WHERE latin1_bin_column=_latin1'a' AND latin1_bin_column='A'
+#
+SET NAMES latin1;
+CREATE TABLE t1 (a VARCHAR(20) COLLATE latin1_bin);
+INSERT INTO t1 VALUES ('a'),('b');
+SELECT * FROM t1 WHERE a='A';
+a
+SELECT * FROM t1 WHERE a='A' AND a=_latin1'a';
+a
+SELECT * FROM t1 WHERE a=_latin1'a' AND a='A';
+a
+SELECT * FROM t1 WHERE a=_latin1'A';
+a
+SELECT * FROM t1 WHERE a=_latin1'A' AND a=_latin1'a';
+a
+SELECT * FROM t1 WHERE a=_latin1'a' AND a=_latin1'A';
+a
+DROP TABLE t1;
+#
 # End of 10.1 tests
 #

mysql-test/r/ctype_uca.result

@@ -13709,5 +13709,22 @@ SELECT '10' COLLATE utf8_general_ci XOR '20' COLLATE utf8_unicode_ci;
 '10' COLLATE utf8_general_ci XOR '20' COLLATE utf8_unicode_ci
 0
 #
+# MDEV-8705 Wrong result for SELECT..WHERE latin1_bin_column='a' AND latin1_bin_column='A'
+#
+SET NAMES utf8 COLLATE utf8_german2_ci;
+CREATE TABLE t1 (a VARCHAR(10) CHARACTER SET utf8 COLLATE utf8_bin);
+INSERT INTO t1 VALUES ('a'),('A');
+SELECT * FROM t1 WHERE a='a';
+a
+a
+SELECT * FROM t1 WHERE a=_utf8'a';
+a
+a
+SELECT * FROM t1 WHERE a='a' AND a=_utf8'a';
+a
+a
+DROP TABLE t1;
+SET NAMES utf8;
+#
 # End of MariaDB-10.1 tests
 #

mysql-test/t/ctype_latin1.test

@@ -349,6 +349,19 @@ SELECT * FROM t1 WHERE a='a' AND LENGTH(a)=2;
 EXPLAIN EXTENDED SELECT * FROM t1 WHERE a='a' AND LENGTH(a)=2;
 DROP TABLE t1;
 
+--echo #
+--echo # MDEV-8712 Wrong result for SELECT..WHERE latin1_bin_column=_latin1'a' AND latin1_bin_column='A'
+--echo #
+SET NAMES latin1;
+CREATE TABLE t1 (a VARCHAR(20) COLLATE latin1_bin);
+INSERT INTO t1 VALUES ('a'),('b');
+SELECT * FROM t1 WHERE a='A';
+SELECT * FROM t1 WHERE a='A' AND a=_latin1'a';
+SELECT * FROM t1 WHERE a=_latin1'a' AND a='A';
+SELECT * FROM t1 WHERE a=_latin1'A';
+SELECT * FROM t1 WHERE a=_latin1'A' AND a=_latin1'a';
+SELECT * FROM t1 WHERE a=_latin1'a' AND a=_latin1'A';
+DROP TABLE t1;
 
 --echo #
 --echo # End of 10.1 tests

mysql-test/t/ctype_uca.test

@@ -630,6 +630,18 @@ DROP TABLE t1;
 --echo #
 SELECT '10' COLLATE utf8_general_ci XOR '20' COLLATE utf8_unicode_ci;
 
+--echo #
+--echo # MDEV-8705 Wrong result for SELECT..WHERE latin1_bin_column='a' AND latin1_bin_column='A'
+--echo #
+SET NAMES utf8 COLLATE utf8_german2_ci;
+CREATE TABLE t1 (a VARCHAR(10) CHARACTER SET utf8 COLLATE utf8_bin);
+INSERT INTO t1 VALUES ('a'),('A');
+SELECT * FROM t1 WHERE a='a';
+SELECT * FROM t1 WHERE a=_utf8'a';
+# Make sure this does not return "Illegal mix of collations"
+SELECT * FROM t1 WHERE a='a' AND a=_utf8'a';
+DROP TABLE t1;
+SET NAMES utf8;
 
 --echo #
 --echo # End of MariaDB-10.1 tests

sql/item_cmpfunc.cc

@@ -5920,7 +5920,7 @@ Item_equal::Item_equal(THD *thd, Item *f1, Item *f2, bool with_const_item):
  with_const= with_const_item;
  equal_items.push_back(f1, thd->mem_root);
  equal_items.push_back(f2, thd->mem_root);
- compare_as_dates= with_const_item && f2->cmp_type() == TIME_RESULT;
+ cmp.cmp_collation.set(f2->collation);
  cmp.set_compare_type(item_cmp_type(f1, f2));
  upper_levels= NULL;
 }
@@ -5950,7 +5950,7 @@ Item_equal::Item_equal(THD *thd, Item_equal *item_equal):
  equal_items.push_back(item, thd->mem_root);
  }
  with_const= item_equal->with_const;
- compare_as_dates= item_equal->compare_as_dates;
+ cmp.cmp_collation.set(item_equal->cmp.cmp_collation);
  cmp.set_compare_type(item_equal->cmp.compare_type());
  cond_false= item_equal->cond_false;
  upper_levels= item_equal->upper_levels;
@@ -5971,41 +5971,85 @@ Item_equal::Item_equal(THD *thd, Item_equal *item_equal):
  the list. Otherwise the value of c is compared with the value of the
  constant item from equal_items. If they are not equal cond_false is set
  to TRUE. This serves as an indicator that this Item_equal is always FALSE.
- The optional parameter f is used to adjust the flag compare_as_dates.
 */
 
-void Item_equal::add_const(THD *thd, Item *c, Item *f)
+void Item_equal::add_const(THD *thd, Item *c)
 {
  if (cond_false)
  return;
  if (!with_const)
  {
  with_const= TRUE;
- if (f)
- compare_as_dates= f->cmp_type() == TIME_RESULT;
  equal_items.push_front(c, thd->mem_root);
  return;
  }
  Item *const_item= get_const();
- if (compare_as_dates)
- {
- cmp.set_datetime_cmp_func(this, &c, &const_item);
- cond_false= cmp.compare();
- }
- else
- {
- Item_func_eq *func= new (thd->mem_root) Item_func_eq(thd, c, const_item);
- if (func->set_cmp_func())
+ switch (cmp.compare_type()) {
+ case TIME_RESULT:
  {
+ cmp.set_datetime_cmp_func(this, &c, &const_item);
+ cond_false= cmp.compare();
+ break;
+ }
+ case STRING_RESULT:
+ {
+ String *str1, *str2;
  /*
- Setting a comparison function fails when trying to compare
- incompatible charsets. Charset compatibility is checked earlier,
- except for constant subqueries where we may do it here.
+ Suppose we have an expression (with a string type field) like this:
+ WHERE field=const1 AND field=const2 ...
+
+ For all pairs field=constXXX we know that:
+
+ - Item_func_eq::fix_length_and_dec() performed collation and character
+ set aggregation and added character set converters when needed.
+ Note, the case like:
+ WHERE field=const1 COLLATE latin1_bin AND field=const2
+ is not handled here, because the field would be replaced to
+ Item_func_set_collation, which cannot get into Item_equal.
+ So all constXXX that are handled by Item_equal
+ already have compatible character sets with "field".
+
+ - Also, Field_str::test_if_equality_guarantees_uniqueness() guarantees
+ that the comparison collation of all equalities handled by Item_equal
+ match the the collation of the field.
+
+ Therefore, at Item_equal::add_const() time all constants constXXX
+ should be directly comparable to each other without an additional
+ character set conversion.
+ It's safe to do val_str() for "const_item" and "c" and compare
+ them according to the collation of the *field*.
+
+ So in a script like this:
+ CREATE TABLE t1 (a VARCHAR(10) COLLATE xxx);
+ INSERT INTO t1 VALUES ('a'),('A');
+ SELECT * FROM t1 WHERE a='a' AND a='A';
+ Item_equal::add_const() effectively rewrites the condition to:
+ SELECT * FROM t1 WHERE a='a' AND 'a' COLLATE xxx='A';
+ and then to:
+ SELECT * FROM t1 WHERE a='a'; // if the two constants were equal
+ // e.g. in case of latin1_swedish_ci
+ or to:
+ SELECT * FROM t1 WHERE FALSE; // if the two constants were not equal
+ // e.g. in case of latin1_bin
+
+ Note, both "const_item" and "c" can return NULL, e.g.:
+ SELECT * FROM t1 WHERE a=NULL AND a='const';
+ SELECT * FROM t1 WHERE a='const' AND a=NULL;
+ SELECT * FROM t1 WHERE a='const' AND a=(SELECT MAX(a) FROM t2)
  */
- return;
+ cond_false= !(str1= const_item->val_str(&cmp.value1)) ||
+ !(str2= c->val_str(&cmp.value2)) ||
+ !str1->eq(str2, compare_collation());
+ break;
+ }
+ default:
+ {
+ Item_func_eq *func= new (thd->mem_root) Item_func_eq(thd, c, const_item);
+ if (func->set_cmp_func())
+ return;
+ func->quick_fix_field();
+ cond_false= !func->val_int();
  }
- func->quick_fix_field();
- cond_false= !func->val_int();
  }
  if (with_const && equal_items.elements == 1)
  cond_true= TRUE;
@@ -6482,14 +6526,6 @@ void Item_equal::print(String *str, enum_query_type query_type)
 }
 
 
-CHARSET_INFO *Item_equal::compare_collation() const
-{ 
- Item_equal_fields_iterator it(*((Item_equal*) this));
- Item *item= it++;
- return item->collation.collation;
-}
-
-
 /*
  @brief Get the first equal field of multiple equality.
  @param[in] field the field to get equal field to

sql/item_cmpfunc.h

@@ -2052,12 +2052,6 @@ class Item_equal: public Item_bool_func
  the equal_items should be ignored.
  */
  bool cond_true;
- /* 
- compare_as_dates=TRUE <-> constants equal to fields from equal_items
- must be compared as datetimes and not as strings.
- compare_as_dates can be TRUE only if with_const=TRUE 
- */
- bool compare_as_dates;
  /* 
  The comparator used to compare constants equal to fields from equal_items
  as datetimes. The comparator is used only if compare_as_dates=TRUE
@@ -2080,7 +2074,7 @@ class Item_equal: public Item_bool_func
  Item_equal(THD *thd, Item_equal *item_equal);
  /* Currently the const item is always the first in the list of equal items */
  inline Item* get_const() { return with_const ? equal_items.head() : NULL; }
- void add_const(THD *thd, Item *c, Item *f = NULL);
+ void add_const(THD *thd, Item *c);
  /** Add a non-constant item to the multiple equality */
  void add(Item *f, MEM_ROOT *root) { equal_items.push_back(f, root); }
  bool contains(Field *field);
@@ -2110,7 +2104,8 @@ class Item_equal: public Item_bool_func
  Item *transform(THD *thd, Item_transformer transformer, uchar *arg);
  virtual void print(String *str, enum_query_type query_type);
  Item_result compare_type() const { return cmp.compare_type(); }
- CHARSET_INFO *compare_collation() const;
+ CHARSET_INFO *compare_collation() const
+ { return cmp.cmp_collation.collation; }
 
  void set_context_field(Item_field *ctx_field) { context_field= ctx_field; }
  void set_link_equal_fields(bool flag) { link_equal_fields= flag; }

sql/sql_select.cc

@@ -12699,7 +12699,7 @@ static bool check_simple_equality(THD *thd, const Item::Context &ctx,
  already contains a constant and its value is not equal to
  the value of const_item.
  */
- item_equal->add_const(thd, const_item2, orig_field_item);
+ item_equal->add_const(thd, const_item2);
  }
  else
  {