A clean-up after the patch for MDEV-8747 and MDEV-8749:

removing IMPOSSIBLE_RESULT from Item_result, as it's not needed any more. The fact that an Item is not in a comparison context is now always designated by IDENTITY_SUBST in Subst_constraint. Previously IMPOSSIBLE_RESULT and IDENTITY_SUBST co-existed but actually meant the same thing.
MariaDB · Sep 6, 2015 · e0df116 · e0df116
1 parent c108019
commit e0df116
Show file tree

Hide file tree

Showing 11 changed files with 47 additions and 100 deletions.
diff --git a/include/mysql.h.pp b/include/mysql.h.pp
@@ -100,7 +100,7 @@
 enum Item_result
 {
   STRING_RESULT=0, REAL_RESULT, INT_RESULT, ROW_RESULT, DECIMAL_RESULT,
-  TIME_RESULT,IMPOSSIBLE_RESULT
+  TIME_RESULT
 };
 typedef struct st_udf_args
 {

diff --git a/include/mysql_com.h b/include/mysql_com.h
@@ -547,7 +547,7 @@ struct my_rnd_struct;
 enum Item_result
 {
   STRING_RESULT=0, REAL_RESULT, INT_RESULT, ROW_RESULT, DECIMAL_RESULT,
-  TIME_RESULT,IMPOSSIBLE_RESULT
+  TIME_RESULT
 };
 
 typedef struct st_udf_args

diff --git a/sql/field.cc b/sql/field.cc
@@ -8616,7 +8616,6 @@ bool Field_enum::can_optimize_keypart_ref(const Item_bool_func *cond,
     return true;
   case STRING_RESULT:
     return charset() == cond->compare_collation();
-  case IMPOSSIBLE_RESULT:
   case ROW_RESULT:
     DBUG_ASSERT(0);
     break;

diff --git a/sql/item.cc b/sql/item.cc
@@ -97,7 +97,6 @@ bool Item::val_bool()
     return val_real() != 0.0;
   case ROW_RESULT:
   case TIME_RESULT:
-  case IMPOSSIBLE_RESULT:
     DBUG_ASSERT(0);
     return 0;                                   // Wrong (but safe)
   }
@@ -543,7 +542,6 @@ void Item::print_value(String *str)
       break;
     case ROW_RESULT:
     case TIME_RESULT:
-    case IMPOSSIBLE_RESULT:
       DBUG_ASSERT(0);
     }
   }
@@ -634,7 +632,7 @@ Item_result Item::cmp_type() const
                            return TIME_RESULT;
   };
   DBUG_ASSERT(0);
-  return IMPOSSIBLE_RESULT;
+  return STRING_RESULT;
 }
 
 /**
@@ -2596,7 +2594,6 @@ bool Item_field::val_bool_result()
     return result_field->val_real() != 0.0;
   case ROW_RESULT:
   case TIME_RESULT:
-  case IMPOSSIBLE_RESULT:
     DBUG_ASSERT(0);
     return 0;                                   // Shut up compiler
   }
@@ -3396,7 +3393,6 @@ bool Item_param::set_from_user_var(THD *thd, const user_var_entry *entry)
     }
     case ROW_RESULT:
     case TIME_RESULT:
-    case IMPOSSIBLE_RESULT:
       DBUG_ASSERT(0);
       set_null();
     }
@@ -4015,7 +4011,6 @@ Item_copy *Item_copy::create(THD *thd, Item *item)
       return new (mem_root) Item_copy_decimal(thd, item);
     case TIME_RESULT:
     case ROW_RESULT:
-  case IMPOSSIBLE_RESULT:
       DBUG_ASSERT (0);
   }
   /* should not happen */
@@ -5369,13 +5364,27 @@ Item_equal *Item_field::find_item_equal(COND_EQUAL *cond_equal)
 bool Item_field::can_be_substituted_to_equal_item(const Context &ctx,
                                                   const Item_equal *item_equal)
 {
-  if (ctx.compare_type() == STRING_RESULT &&
-      ctx.compare_collation() != item_equal->compare_collation())
-    return false;
-  return ctx.subst_constraint() == ANY_SUBST ||
-         field->cmp_type() != STRING_RESULT ||
-         ((field->charset()->state & MY_CS_BINSORT) &&
-          (field->charset()->state & MY_CS_NOPAD));
+  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;
 }
 
 
@@ -5442,17 +5451,7 @@ Item *Item_field::propagate_equal_fields(THD *thd,
   Item *item= 0;
   if (item_equal)
   {
-    /*
-      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').
-    */
-    if (!can_be_substituted_to_equal_item(ctx, item_equal) ||
-        !ctx.has_compatible_context(item_equal->compare_type()))
+    if (!can_be_substituted_to_equal_item(ctx, item_equal))
     {
       item_equal= NULL;
       return this;
@@ -5463,11 +5462,13 @@ Item *Item_field::propagate_equal_fields(THD *thd,
     item= this;
   else if (field && (field->flags & ZEROFILL_FLAG) && IS_NUM(field->type()))
   {
-    DBUG_ASSERT(ctx.compare_type() != STRING_RESULT);
-    if (item && (ctx.subst_constraint() == IDENTITY_SUBST))
+    if (ctx.subst_constraint() == IDENTITY_SUBST)
       convert_zerofill_number_to_string(thd, &item, (Field_num *)field);
     else
+    {
+      DBUG_ASSERT(ctx.compare_type() != STRING_RESULT);
       item= this;
+    }
   }
   return item;
 }
@@ -5599,7 +5600,6 @@ enum_field_types Item::field_type() const
   case REAL_RESULT:    return MYSQL_TYPE_DOUBLE;
   case ROW_RESULT:
   case TIME_RESULT:
-  case IMPOSSIBLE_RESULT:
     DBUG_ASSERT(0);
     return MYSQL_TYPE_VARCHAR;
   }
@@ -7384,7 +7384,6 @@ bool Item_ref::val_bool_result()
       return result_field->val_real() != 0.0;
     case ROW_RESULT:
     case TIME_RESULT:
-    case IMPOSSIBLE_RESULT:
       DBUG_ASSERT(0);
     }
   }
@@ -8748,9 +8747,6 @@ void resolve_const_item(THD *thd, Item **ref, Item *comp_item)
                (Item*) new (mem_root) Item_decimal(thd, name, result, length, decimals));
     break;
   }
-  case IMPOSSIBLE_RESULT:
-    DBUG_ASSERT(0);
-    break;
   }
   if (new_item)
     thd->change_item_tree(ref, new_item);
@@ -8902,9 +8898,6 @@ Item_cache* Item_cache::get_cache(THD *thd, const Item *item,
     return new (mem_root) Item_cache_row(thd);
   case TIME_RESULT:
     return new (mem_root) Item_cache_temporal(thd, item->field_type());
-  case IMPOSSIBLE_RESULT:
-    DBUG_ASSERT(0);
-    break;
   }
   return 0;                                     // Impossible
 }
@@ -9538,7 +9531,6 @@ enum_field_types Item_type_holder::get_real_type(Item *item)
         return MYSQL_TYPE_NEWDECIMAL;
       case ROW_RESULT:
       case TIME_RESULT:
-      case IMPOSSIBLE_RESULT:
         DBUG_ASSERT(0);
         return MYSQL_TYPE_VAR_STRING;
       }

diff --git a/sql/item.h b/sql/item.h
@@ -1438,14 +1438,18 @@ class Item: public Type_std_attributes
   {
     /*
       Which type of propagation is allowed:
-      - SUBST_ANY (loose equality, according to the collation), or
-      - SUBST_IDENTITY (strict binary equality).
+      - 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 SUBST_ANY.
+      Important only when ANY_SUBST.
     */
     CHARSET_INFO *m_compare_collation;
   public:
@@ -1454,27 +1458,22 @@ class Item: public Type_std_attributes
        m_compare_type(type),
        m_compare_collation(cs) { }
     Subst_constraint subst_constraint() const { return m_subst_constraint; }
-    Item_result compare_type() const { return m_compare_type; }
-    CHARSET_INFO *compare_collation() const { return m_compare_collation; }
-    /**
-      Check whether this and the given comparison type are compatible.
-      Used by the equality propagation. See Item_field::propagate_equal_fields()
-
-      @return
-        TRUE  if the context is the same
-        FALSE otherwise.
-    */
-    inline bool has_compatible_context(Item_result other) const
+    Item_result compare_type() const
+    {
+      DBUG_ASSERT(m_subst_constraint == ANY_SUBST);
+      return m_compare_type;
+    }
+    CHARSET_INFO *compare_collation() const
     {
-      return m_compare_type == IMPOSSIBLE_RESULT ||
-             m_compare_type == other;
+      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, IMPOSSIBLE_RESULT, &my_charset_bin) { }
+      :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

diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
@@ -612,9 +612,6 @@ int Arg_comparator::set_compare_func(Item_func_or_sum *item, Item_result type)
     }
     break;
   }
-  case IMPOSSIBLE_RESULT:
-    DBUG_ASSERT(0);
-    break;
   }
   return 0;
 }
@@ -2358,7 +2355,6 @@ longlong Item_func_between::val_int()
     break;
   }
   case ROW_RESULT:
-  case IMPOSSIBLE_RESULT:
     DBUG_ASSERT(0);
     null_value= 1;
     return 0;
@@ -2417,7 +2413,6 @@ Item_func_case_abbreviation2::fix_length_and_dec2(Item **args)
     break;
   case ROW_RESULT:
   case TIME_RESULT:
-  case IMPOSSIBLE_RESULT:
     DBUG_ASSERT(0);
   }
   fix_char_length(char_length);
@@ -3375,7 +3370,6 @@ void Item_func_coalesce::fix_length_and_dec()
     break;
   case ROW_RESULT:
   case TIME_RESULT:
-  case IMPOSSIBLE_RESULT:
     DBUG_ASSERT(0);
   }
 }
@@ -3739,9 +3733,6 @@ cmp_item* cmp_item::get_comparator(Item_result type, Item *warn_item,
   case TIME_RESULT:
     DBUG_ASSERT(warn_item);
     return new cmp_item_datetime(warn_item);
-  case IMPOSSIBLE_RESULT:
-    DBUG_ASSERT(0);
-    break;
   }
   return 0; // to satisfy compiler :)
 }
@@ -4164,9 +4155,6 @@ void Item_func_in::fix_length_and_dec()
       date_arg= find_date_time_item(args, arg_count, 0);
       array= new (thd->mem_root) in_datetime(date_arg, arg_count - 1);
       break;
-    case IMPOSSIBLE_RESULT:
-      DBUG_ASSERT(0);
-      break;
     }
     if (array && !(thd->is_fatal_error))		// If not EOM
     {

diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h
@@ -69,11 +69,11 @@ class Arg_comparator: public Sql_alloc
   /* Allow owner function to use string buffers. */
   String value1, value2;
 
-  Arg_comparator(): m_compare_type(IMPOSSIBLE_RESULT),
+  Arg_comparator(): m_compare_type(STRING_RESULT),
     set_null(TRUE), comparators(0), thd(0),
     a_cache(0), b_cache(0) {};
   Arg_comparator(Item **a1, Item **a2): a(a1), b(a2),
-    m_compare_type(IMPOSSIBLE_RESULT), set_null(TRUE),
+    m_compare_type(STRING_RESULT), set_null(TRUE),
     comparators(0), thd(0), a_cache(0), b_cache(0) {};
 
 private: