MDEV-8816 Equal field propagation is not applied for WHERE varbinary_column>=_utf8'a' COLLATE utf8_general_ci AND varbinary_column='A';

1. Removing the legacy code that disabled equal field propagation in cases when comparison is done as VARBINARY. This is now correctly handled by the new propagation code in Item_xxx::propagate_equal_fields() and Field_str::can_be_substituted_to_equal_item (the bug fix). 2. Also, removing legacy (pre-MySQL-4.1) Arg_comparator methods compare_binary_string() and compare_e_binary_string(), as VARBINARY comparison is correcty handled in compare_string() and compare_e_string() by the corresponding VARBINARY collation handler implemented in my_charset_bin. (not really a part of the bug fix)

Author: Alexander Barkov

mysql-test/r/ctype_utf8.result

@@ -10198,5 +10198,20 @@ Warnings:
 Note	1003	select `test`.`t1`.`a` AS `a` from `test`.`t1` where ((`test`.`t1`.`a` = '1ë1') and (`test`.`t1`.`a` in (1,2,'x')))
 DROP TABLE IF EXISTS t1;
 #
+# MDEV-8816 Equal field propagation is not applied for WHERE varbinary_column>=_utf8'a' COLLATE utf8_swedish_ci AND varbinary_column='A';
+#
+CREATE TABLE t1 (c VARBINARY(10));
+INSERT INTO t1 VALUES ('a'),('A');
+SELECT * FROM t1 WHERE c>=_utf8'a' COLLATE utf8_general_ci AND c='A';
+c
+A
+EXPLAIN EXTENDED
+SELECT * FROM t1 WHERE c>=_utf8'a' COLLATE utf8_general_ci AND c='A';
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	filtered	Extra
+1	SIMPLE	t1	ALL	NULL	NULL	NULL	NULL	2	100.00	Using where
+Warnings:
+Note	1003	select `test`.`t1`.`c` AS `c` from `test`.`t1` where (`test`.`t1`.`c` = 'A')
+DROP TABLE t1;
+#
 # End of 10.1 tests
 #

mysql-test/t/ctype_utf8.test

@@ -1833,6 +1833,17 @@ EXPLAIN EXTENDED SELECT * FROM t1 WHERE a IN (1,2,'x') AND a='1ë1';
 DROP TABLE IF EXISTS t1;
 
 
+--echo #
+--echo # MDEV-8816 Equal field propagation is not applied for WHERE varbinary_column>=_utf8'a' COLLATE utf8_swedish_ci AND varbinary_column='A';
+--echo #
+CREATE TABLE t1 (c VARBINARY(10));
+INSERT INTO t1 VALUES ('a'),('A');
+SELECT * FROM t1 WHERE c>=_utf8'a' COLLATE utf8_general_ci AND c='A';
+EXPLAIN EXTENDED
+SELECT * FROM t1 WHERE c>=_utf8'a' COLLATE utf8_general_ci AND c='A';
+DROP TABLE t1;
+
+
 --echo #
 --echo # End of 10.1 tests
 --echo #

sql/item.cc

@@ -2140,8 +2140,6 @@ bool agg_item_set_converter(DTCollation &coll, const char *fname,
  res= TRUE;
  break; // we cannot return here, we need to restore "arena".
  }
- if ((*arg)->type() == Item::FIELD_ITEM)
- ((Item_field *)(*arg))->no_const_subst= 1;
  /*
  If in statement prepare, then we create a converter for two
  constant items, do it once and then reuse it.
@@ -2226,7 +2224,7 @@ void Item_ident_for_show::make_field(Send_field *tmp_field)
 
 Item_field::Item_field(THD *thd, Field *f)
  :Item_ident(thd, 0, NullS, *f->table_name, f->field_name),
- item_equal(0), no_const_subst(0),
+ item_equal(0),
  have_privileges(0), any_privileges(0)
 {
  set_field(f);
@@ -2249,7 +2247,7 @@ Item_field::Item_field(THD *thd, Field *f)
 Item_field::Item_field(THD *thd, Name_resolution_context *context_arg,
  Field *f)
  :Item_ident(thd, context_arg, f->table->s->db.str, *f->table_name, f->field_name),
- item_equal(0), no_const_subst(0),
+ item_equal(0),
  have_privileges(0), any_privileges(0)
 {
  /*
@@ -2292,7 +2290,7 @@ Item_field::Item_field(THD *thd, Name_resolution_context *context_arg,
  const char *db_arg,const char *table_name_arg,
  const char *field_name_arg)
  :Item_ident(thd, context_arg, db_arg, table_name_arg, field_name_arg),
- field(0), item_equal(0), no_const_subst(0),
+ field(0), item_equal(0),
  have_privileges(0), any_privileges(0)
 {
  SELECT_LEX *select= thd->lex->current_select;
@@ -2310,7 +2308,6 @@ Item_field::Item_field(THD *thd, Item_field *item)
  :Item_ident(thd, item),
  field(item->field),
  item_equal(item->item_equal),
- no_const_subst(item->no_const_subst),
  have_privileges(item->have_privileges),
  any_privileges(item->any_privileges)
 {
@@ -5335,7 +5332,7 @@ Item *Item_field::propagate_equal_fields(THD *thd,
  const Context &ctx,
  COND_EQUAL *arg)
 {
- if (no_const_subst || !(item_equal= find_item_equal(arg)))
+ if (!(item_equal= find_item_equal(arg)))
  return this;
  if (!field->can_be_substituted_to_equal_item(ctx, item_equal))
  {
@@ -5372,20 +5369,6 @@ Item *Item_field::propagate_equal_fields(THD *thd,
 }
 
 
-/**
- Mark the item to not be part of substitution if it's not a binary item.
-
- See comments in Arg_comparator::set_compare_func() for details.
-*/
-
-bool Item_field::set_no_const_sub(uchar *arg)
-{
- if (field->charset() != &my_charset_bin)
- no_const_subst=1;
- return FALSE;
-}
-
-
 /**
  Replace an Item_field for an equal Item_field that evaluated earlier
  (if any).

sql/item.h

@@ -1520,7 +1520,6 @@ class Item: public Value_source, public Type_std_attributes
  return trace_unsupported_by_check_vcol_func_processor(full_name());
  }
 
- virtual bool set_no_const_sub(uchar *arg) { return FALSE; }
  /* arg points to REPLACE_EQUAL_FIELD_ARG object */
  virtual Item *replace_equal_field(THD *thd, uchar *arg) { return this; }
  /*
@@ -2333,7 +2332,6 @@ class Item_field :public Item_ident
 public:
  Field *field;
  Item_equal *item_equal;
- bool no_const_subst;
  /*
  if any_privileges set to TRUE then here real effective privileges will
  be stored
@@ -2465,7 +2463,6 @@ class Item_field :public Item_ident
  void set_item_equal(Item_equal *item_eq) { item_equal= item_eq; }
  Item_equal *find_item_equal(COND_EQUAL *cond_equal);
  Item* propagate_equal_fields(THD *, const Context &, COND_EQUAL *);
- bool set_no_const_sub(uchar *arg);
  Item *replace_equal_field(THD *thd, uchar *arg);
  inline uint32 max_disp_length() { return field->max_display_length(); }
  Item_field *field_for_view_update() { return this; }

sql/item_cmpfunc.cc

@@ -554,32 +554,6 @@ int Arg_comparator::set_compare_func(Item_func_or_sum *item, Item_result type)
  }
  break;
  }
- case STRING_RESULT:
- {
- if (cmp_collation.collation == &my_charset_bin)
- {
- /*
-We are using BLOB/BINARY/VARBINARY, change to compare byte by byte,
-without removing end space
- */
- if (func == &Arg_comparator::compare_string)
-func= &Arg_comparator::compare_binary_string;
- else if (func == &Arg_comparator::compare_e_string)
-func= &Arg_comparator::compare_e_binary_string;
-
- /*
- As this is binary compassion, mark all fields that they can't be
- transformed. Otherwise we would get into trouble with comparisons
- like:
- WHERE col= 'j' AND col LIKE BINARY 'j'
- which would be transformed to:
- WHERE col= 'j'
- */
- (*a)->walk(&Item::set_no_const_sub, FALSE, (uchar*) 0);
- (*b)->walk(&Item::set_no_const_sub, FALSE, (uchar*) 0);
- }
- break;
- }
  case INT_RESULT:
  {
  if (func == &Arg_comparator::compare_int_signed)
@@ -598,6 +572,7 @@ int Arg_comparator::set_compare_func(Item_func_or_sum *item, Item_result type)
  }
  break;
  }
+ case STRING_RESULT:
  case DECIMAL_RESULT:
  break;
  case REAL_RESULT:
@@ -943,38 +918,6 @@ int Arg_comparator::compare_string()
 }
 
 
-/**
- Compare strings byte by byte. End spaces are also compared.
-
- @retval
- <0 *a < *b
- @retval
- 0 *b == *b
- @retval
- >0 *a > *b
-*/
-
-int Arg_comparator::compare_binary_string()
-{
- String *res1,*res2;
- if ((res1= (*a)->val_str(&value1)))
- {
- if ((res2= (*b)->val_str(&value2)))
- {
- if (set_null)
- owner->null_value= 0;
- uint res1_length= res1->length();
- uint res2_length= res2->length();
- int cmp= memcmp(res1->ptr(), res2->ptr(), MY_MIN(res1_length,res2_length));
- return cmp ? cmp : (int) (res1_length - res2_length);
- }
- }
- if (set_null)
- owner->null_value= 1;
- return -1;
-}
-
-
 /**
  Compare strings, but take into account that NULL == NULL.
 */
@@ -991,17 +934,6 @@ int Arg_comparator::compare_e_string()
 }
 
 
-int Arg_comparator::compare_e_binary_string()
-{
- String *res1,*res2;
- res1= (*a)->val_str(&value1);
- res2= (*b)->val_str(&value2);
- if (!res1 || !res2)
- return MY_TEST(res1 == res2);
- return MY_TEST(stringcmp(res1, res2) == 0);
-}
-
-
 int Arg_comparator::compare_real()
 {
  /*

sql/item_cmpfunc.h

@@ -86,7 +86,6 @@ class Arg_comparator: public Sql_alloc
  inline int compare() { return (this->*func)(); }
 
  int compare_string(); // compare args[0] & args[1]
- int compare_binary_string(); // compare args[0] & args[1]
  int compare_real(); // compare args[0] & args[1]
  int compare_decimal(); // compare args[0] & args[1]
  int compare_int_signed(); // compare args[0] & args[1]
@@ -95,7 +94,6 @@ class Arg_comparator: public Sql_alloc
  int compare_int_unsigned();
  int compare_row(); // compare args[0] & args[1]
  int compare_e_string(); // compare args[0] & args[1]
- int compare_e_binary_string(); // compare args[0] & args[1]
  int compare_e_real(); // compare args[0] & args[1]
  int compare_e_decimal(); // compare args[0] & args[1]
  int compare_e_int(); // compare args[0] & args[1]