MDEV-8729 Wrong result for SELECT..WHERE HEX(enum_column)='61' AND enum_column='a '

4 files changed, 210 insertions, 173 deletions

diff --git a/sql/field.cc b/sql/field.cc
index a472ce473bf..b9ded9605c8 100644
--- a/sql/field.cc
+++ b/sql/field.cc
@@ -1261,6 +1261,61 @@ bool Field::test_if_equality_guarantees_uniqueness(const Item *item) const
 }
 
 
+/**
+  Check whether a field item can be substituted for an equal item
+
+  @details
+  The function checks whether a substitution of a field item for
+  an equal item is valid.
+
+  @param arg   *arg != NULL <-> the field is in the context
+               where substitution for an equal item is valid
+
+  @note
+    The following statement is not always true:
+  @n
+    x=y => F(x)=F(x/y).
+  @n
+    This means substitution of an item for an equal item not always
+    yields an equavalent condition. Here's an example:
+    @code
+    'a'='a '
+    (LENGTH('a')=1) != (LENGTH('a ')=2)
+  @endcode
+    Such a substitution is surely valid if either the substituted
+    field is not of a STRING type or if it is an argument of
+    a comparison predicate.
+
+  @retval
+    TRUE   substitution is valid
+  @retval
+    FALSE  otherwise
+*/
+
+bool Field::can_be_substituted_to_equal_item(const Context &ctx,
+                                             const Item_equal *item_equal)
+{
+  DBUG_ASSERT(item_equal->compare_type() != STRING_RESULT);
+  DBUG_ASSERT(cmp_type() != STRING_RESULT);
+  switch (ctx.subst_constraint()) {
+  case ANY_SUBST:
+    /*
+      Disable const propagation for items used in different comparison contexts.
+      This must be done because, for example, Item_hex_string->val_int() is not
+      the same as (Item_hex_string->val_str() in BINARY column)->val_int().
+      We cannot simply disable the replacement in a particular context (
+      e.g. <bin_col> = <int_col> AND <bin_col> = <hex_string>) since
+      Items don't know the context they are in and there are functions like
+      IF (<hex_string>, 'yes', 'no').
+    */
+    return ctx.compare_type() == item_equal->compare_type();
+  case IDENTITY_SUBST:
+    return true;
+  }
+  return false;
+}
+
+
 /*
   This handles all numeric and BIT data types.
 */ 
@@ -1303,7 +1358,7 @@ Field_num::Field_num(uchar *ptr_arg,uint32 len_arg, uchar *null_ptr_arg,
 }
 
 
-void Field_num::prepend_zeros(String *value)
+void Field_num::prepend_zeros(String *value) const
 {
   int diff;
   if ((diff= (int) (field_length - value->length())) > 0)
@@ -1317,6 +1372,36 @@ void Field_num::prepend_zeros(String *value)
   }
 }
 
+
+/**
+  Convert a numeric value to a zero-filled string
+
+  @param[in]  thd    current thread
+  @param[in]  item   the item to convert
+
+  This function converts a numeric value to a string. In this conversion
+  the zero-fill flag of the field is taken into account.
+  This is required so the resulting string value can be used instead of
+  the field reference when propagating equalities.
+*/
+
+Item *Field_num::convert_zerofill_number_to_string(THD *thd, Item *item) const
+{
+  char buff[MAX_FIELD_WIDTH],*pos;
+  String tmp(buff,sizeof(buff),Field_num::charset()), *res;
+
+  res= item->val_str(&tmp);
+  if (item->is_null())
+    return new (thd->mem_root) Item_null(thd);
+  else
+  {
+    prepend_zeros(res);
+    pos= (char *) sql_strmake (res->ptr(), res->length());
+    return new (thd->mem_root) Item_string(thd, pos, res->length(), Field_num::charset());
+  }
+}
+
+
 /**
   Test if given number is a int.
 
@@ -1884,6 +1969,23 @@ bool Field_str::test_if_equality_guarantees_uniqueness(const Item *item) const
 }
 
 
+bool Field_str::can_be_substituted_to_equal_item(const Context &ctx,
+                                                  const Item_equal *item_equal)
+{
+  DBUG_ASSERT(item_equal->compare_type() == STRING_RESULT);
+  switch (ctx.subst_constraint()) {
+  case ANY_SUBST:
+    return ctx.compare_type() == item_equal->compare_type() &&
+          (ctx.compare_type() != STRING_RESULT ||
+           ctx.compare_collation() == item_equal->compare_collation());
+  case IDENTITY_SUBST:
+    return ((charset()->state & MY_CS_BINSORT) &&
+            (charset()->state & MY_CS_NOPAD));
+  }
+  return false;
+}
+
+
 void Field_num::make_field(Send_field *field)
 {
   Field::make_field(field);
diff --git a/sql/field.h b/sql/field.h
index ba44ee28cc6..1f8c09b3627 100644
--- a/sql/field.h
+++ b/sql/field.h
@@ -41,6 +41,7 @@ class Column_statistics;
 class Column_statistics_collected;
 class Item_func;
 class Item_bool_func;
+class Item_equal;
 
 enum enum_check_fields
 {
@@ -50,6 +51,77 @@ enum enum_check_fields
 };
 
 
+/*
+  Common declarations for Field and Item
+*/
+class Value_source
+{
+public:
+  /*
+    The enumeration Subst_constraint is currently used only in implementations
+    of the virtual function subst_argument_checker.
+  */
+  enum Subst_constraint
+  {
+    ANY_SUBST,           /* Any substitution for a field is allowed  */
+    IDENTITY_SUBST       /* Substitution for a field is allowed if any two
+                            different values of the field type are not equal */
+  };
+  /*
+    Item context attributes.
+    Comparison functions pass their attributes to propagate_equal_fields().
+    For exmple, for string comparison, the collation of the comparison
+    operation is important inside propagate_equal_fields().
+  */
+  class Context
+  {
+    /*
+      Which type of propagation is allowed:
+      - ANY_SUBST (loose equality, according to the collation), or
+      - IDENTITY_SUBST (strict binary equality).
+    */
+    Subst_constraint m_subst_constraint;
+    /*
+      Comparison type.
+      Impostant only when ANY_SUBSTS.
+    */
+    Item_result m_compare_type;
+    /*
+      Collation of the comparison operation.
+      Important only when ANY_SUBST.
+    */
+    CHARSET_INFO *m_compare_collation;
+  public:
+    Context(Subst_constraint subst, Item_result type, CHARSET_INFO *cs)
+      :m_subst_constraint(subst),
+       m_compare_type(type),
+       m_compare_collation(cs) { }
+    Subst_constraint subst_constraint() const { return m_subst_constraint; }
+    Item_result compare_type() const
+    {
+      DBUG_ASSERT(m_subst_constraint == ANY_SUBST);
+      return m_compare_type;
+    }
+    CHARSET_INFO *compare_collation() const
+    {
+      DBUG_ASSERT(m_subst_constraint == ANY_SUBST);
+      return m_compare_collation;
+    }
+  };
+  class Context_identity: public Context
+  { // Use this to request only exact value, no invariants.
+  public:
+     Context_identity()
+      :Context(IDENTITY_SUBST, STRING_RESULT, &my_charset_bin) { }
+  };
+  class Context_boolean: public Context
+  { // Use this when an item is [a part of] a boolean expression
+  public:
+    Context_boolean() :Context(ANY_SUBST, INT_RESULT, &my_charset_bin) { }
+  };
+};
+
+
 enum Derivation
 {
   DERIVATION_IGNORABLE= 6,
@@ -61,6 +133,7 @@ enum Derivation
   DERIVATION_EXPLICIT= 0
 };
 
+
 #define STORAGE_TYPE_MASK 7
 #define COLUMN_FORMAT_MASK 7
 #define COLUMN_FORMAT_SHIFT 3
@@ -287,11 +360,18 @@ public:
   }
 };
 
-class Field
+class Field: public Value_source
 {
   Field(const Item &);				/* Prevent use of these */
   void operator=(Field &);
 public:
+  virtual bool can_be_substituted_to_equal_item(const Context &ctx,
+                                        const Item_equal *item);
+  virtual Item *get_equal_const_item(THD *thd, const Context &ctx,
+                                     Item_field *field_item, Item *const_item)
+  {
+    return const_item;
+  }
   static void *operator new(size_t size, MEM_ROOT *mem_root) throw ()
   { return alloc_root(mem_root, size); }
   static void *operator new(size_t size) throw ()
@@ -1126,7 +1206,11 @@ protected:
 
 
 class Field_num :public Field {
+protected:
+  Item *convert_zerofill_number_to_string(THD *thd, Item *item) const;
 public:
+  Item *get_equal_const_item(THD *thd, const Context &ctx,
+                             Item_field *field_item, Item *const_item);
   const uint8 dec;
   bool zerofill,unsigned_flag;	// Purify cannot handle bit fields
   Field_num(uchar *ptr_arg,uint32 len_arg, uchar *null_ptr_arg,
@@ -1137,7 +1221,7 @@ public:
   enum Derivation derivation(void) const { return DERIVATION_NUMERIC; }
   uint repertoire(void) const { return MY_REPERTOIRE_NUMERIC; }
   CHARSET_INFO *charset(void) const { return &my_charset_numeric; }
-  void prepend_zeros(String *value);
+  void prepend_zeros(String *value) const;
   void add_zerofill_and_unsigned(String &res) const;
   friend class Create_field;
   void make_field(Send_field *);
@@ -1172,6 +1256,8 @@ protected:
   CHARSET_INFO *field_charset;
   enum Derivation field_derivation;
 public:
+  bool can_be_substituted_to_equal_item(const Context &ctx,
+                                        const Item_equal *item_equal);
   Field_str(uchar *ptr_arg,uint32 len_arg, uchar *null_ptr_arg,
 	    uchar null_bit_arg, utype unireg_check_arg,
 	    const char *field_name_arg, CHARSET_INFO *charset);
diff --git a/sql/item.cc b/sql/item.cc
index cfbbcd136ac..2dfca76fbbd 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -5331,93 +5331,6 @@ Item_equal *Item_field::find_item_equal(COND_EQUAL *cond_equal)
 
 
 /**
-  Check whether a field item can be substituted for an equal item
-
-  @details
-  The function checks whether a substitution of a field item for
-  an equal item is valid.
-
-  @param arg   *arg != NULL <-> the field is in the context
-               where substitution for an equal item is valid
-
-  @note
-    The following statement is not always true:
-  @n
-    x=y => F(x)=F(x/y).
-  @n
-    This means substitution of an item for an equal item not always
-    yields an equavalent condition. Here's an example:
-    @code
-    'a'='a '
-    (LENGTH('a')=1) != (LENGTH('a ')=2)
-  @endcode
-    Such a substitution is surely valid if either the substituted
-    field is not of a STRING type or if it is an argument of
-    a comparison predicate.
-
-  @retval
-    TRUE   substitution is valid
-  @retval
-    FALSE  otherwise
-*/
-
-bool Item_field::can_be_substituted_to_equal_item(const Context &ctx,
-                                                  const Item_equal *item_equal)
-{
-  switch (ctx.subst_constraint()) {
-  case ANY_SUBST:
-    /*
-      Disable const propagation for items used in different comparison contexts.
-      This must be done because, for example, Item_hex_string->val_int() is not
-      the same as (Item_hex_string->val_str() in BINARY column)->val_int().
-      We cannot simply disable the replacement in a particular context (
-      e.g. <bin_col> = <int_col> AND <bin_col> = <hex_string>) since
-      Items don't know the context they are in and there are functions like
-      IF (<hex_string>, 'yes', 'no').
-    */
-    return
-      ctx.compare_type() == item_equal->compare_type() &&
-      (ctx.compare_type() != STRING_RESULT ||
-       ctx.compare_collation() == item_equal->compare_collation());
-  case IDENTITY_SUBST:
-    return field->cmp_type() != STRING_RESULT ||
-           ((field->charset()->state & MY_CS_BINSORT) &&
-            (field->charset()->state & MY_CS_NOPAD));
-  }
-  return false;
-}
-
-
-/**
-  Convert a numeric value to a zero-filled string
-
-  @param[in,out]  item   the item to operate on
-  @param          field  The field that this value is equated to
-
-  This function converts a numeric value to a string. In this conversion
-  the zero-fill flag of the field is taken into account.
-  This is required so the resulting string value can be used instead of
-  the field reference when propagating equalities.
-*/
-
-static void convert_zerofill_number_to_string(THD *thd, Item **item, Field_num *field)
-{
-  char buff[MAX_FIELD_WIDTH],*pos;
-  String tmp(buff,sizeof(buff), field->charset()), *res;
-
-  res= (*item)->val_str(&tmp);
-  if ((*item)->is_null())
-    *item= new (thd->mem_root) Item_null(thd);
-  else
-  {
-    field->prepend_zeros(res);
-    pos= (char *) sql_strmake (res->ptr(), res->length());
-    *item= new (thd->mem_root) Item_string(thd, pos, res->length(), field->charset());
-  }
-}
-
-
-/**
   Set a pointer to the multiple equality the field reference belongs to
   (if any).
 
@@ -5445,32 +5358,34 @@ Item *Item_field::propagate_equal_fields(THD *thd,
                                          const Context &ctx,
                                          COND_EQUAL *arg)
 {
-  if (no_const_subst)
+  if (no_const_subst || !(item_equal= find_item_equal(arg)))
     return this;
-  item_equal= find_item_equal(arg);
-  Item *item= 0;
-  if (item_equal)
+  if (!field->can_be_substituted_to_equal_item(ctx, item_equal))
   {
-    if (!can_be_substituted_to_equal_item(ctx, item_equal))
-    {
-      item_equal= NULL;
-      return this;
-    }
-    item= item_equal->get_const();
+    item_equal= NULL;
+    return this;
   }
-  if (!item)
-    item= this;
-  else if (field && (field->flags & ZEROFILL_FLAG) && IS_NUM(field->type()))
+  Item *item= item_equal->get_const();
+  return item ? field->get_equal_const_item(thd, ctx, this, item) : this;
+}
+
+
+Item *Field_num::get_equal_const_item(THD *thd, const Context &ctx,
+                                  Item_field *field_item,
+                                  Item *const_item)
+{
+  DBUG_ASSERT(const_item->const_item());
+  if ((flags & ZEROFILL_FLAG) && IS_NUM(type()))
   {
     if (ctx.subst_constraint() == IDENTITY_SUBST)
-      convert_zerofill_number_to_string(thd, &item, (Field_num *)field);
+      return convert_zerofill_number_to_string(thd, const_item);
     else
     {
       DBUG_ASSERT(ctx.compare_type() != STRING_RESULT);
-      item= this;
+      return field_item;
     }
   }
-  return item;
+  return const_item;
 }
 
 
diff --git a/sql/item.h b/sql/item.h
index 1fd26f144d4..9d4d2aee0c7 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -602,7 +602,7 @@ public:
 };
 
 
-class Item: public Type_std_attributes
+class Item: public Value_source, public Type_std_attributes
 {
   Item(const Item &);			/* Prevent use of these */
   void operator=(Item &);
@@ -1417,70 +1417,6 @@ public:
     return FALSE;
   }
 
-  /*
-    The enumeration Subst_constraint is currently used only in implementations
-    of the virtual function subst_argument_checker.
-  */ 
-  enum Subst_constraint 
-  { 
-    ANY_SUBST,           /* Any substitution for a field is allowed  */ 
-    IDENTITY_SUBST       /* Substitution for a field is allowed if any two
-                            different values of the field type are not equal */
-  };
-
-  /*
-    Item context attributes.
-    Comparison functions pass their attributes to propagate_equal_fields().
-    For exmple, for string comparison, the collation of the comparison
-    operation is important inside propagate_equal_fields().
-  */
-  class Context
-  {
-    /*
-      Which type of propagation is allowed:
-      - ANY_SUBST (loose equality, according to the collation), or
-      - IDENTITY_SUBST (strict binary equality).
-    */
-    Subst_constraint m_subst_constraint;
-    /*
-      Comparison type.
-      Impostant only when ANY_SUBSTS.
-    */
-    Item_result m_compare_type;
-    /*
-      Collation of the comparison operation.
-      Important only when ANY_SUBST.
-    */
-    CHARSET_INFO *m_compare_collation;
-  public:
-    Context(Subst_constraint subst, Item_result type, CHARSET_INFO *cs)
-      :m_subst_constraint(subst),
-       m_compare_type(type),
-       m_compare_collation(cs) { }
-    Subst_constraint subst_constraint() const { return m_subst_constraint; }
-    Item_result compare_type() const
-    {
-      DBUG_ASSERT(m_subst_constraint == ANY_SUBST);
-      return m_compare_type;
-    }
-    CHARSET_INFO *compare_collation() const
-    {
-      DBUG_ASSERT(m_subst_constraint == ANY_SUBST);
-      return m_compare_collation;
-    }
-  };
-  class Context_identity: public Context
-  { // Use this to request only exact value, no invariants.
-  public:
-     Context_identity()
-      :Context(IDENTITY_SUBST, STRING_RESULT, &my_charset_bin) { }
-  };
-  class Context_boolean: public Context
-  { // Use this when an item is [a part of] a boolean expression
-  public:
-    Context_boolean() :Context(ANY_SUBST, INT_RESULT, &my_charset_bin) { }
-  };
-
   virtual Item* propagate_equal_fields(THD*, const Context &, COND_EQUAL *)
   {
     return this;
@@ -2312,8 +2248,6 @@ public:
 
 class Item_field :public Item_ident
 {
-  bool can_be_substituted_to_equal_item(const Context &ctx,
-                                        const Item_equal *item);
 protected:
   void set_field(Field *field);
 public: