MDEV-8816 Equal field propagation is not applied for WHERE...

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)

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]