MDEV-8740 Wrong result for SELECT..WHERE year_field=10 AND NULLIF(year_field,2011.1)='2011'

MDEV-8754 Wrong result for SELECT..WHERE year_field=2020 AND NULLIF(year_field,2010)='2020'
Problems:
1. Item_func_nullif stored a copy of args[0] in a private member m_args0_copy,
   which was invisible for the inherited Item_func menthods, like
   update_used_tables(). As a result, after equal field propagation
   things like Item_func_nullif::const_item() could return wrong result
   and a non-constant NULLIF() was erroneously treated as a constant
   at optimize_cond() time.
   Solution: removing m_args0_copy and storing the return value item
   in args[2] instead.
2. Equal field propagation did not work well for Item_fun_nullif.
   Solution: using ANY_SUBST for args[0] and args[1], as they are in
   comparison, and IDENTITY_SUBST for args[2], as it's not in comparison.

mysql-test/r/null.result

@@ -1409,5 +1409,61 @@ Warnings:
 Note	1003	select isnull((case when convert(`test`.`t1`.`a` using utf8) = (_utf8'a' collate utf8_bin) then NULL else `test`.`t1`.`a` end)) AS `expr` from `test`.`t1`
 DROP TABLE t1;
 #
+# MDEV-8740 Wrong result for SELECT..WHERE year_field=10 AND NULLIF(year_field,2011.1)='2011'
+#
+CREATE TABLE t1 (a YEAR);
+INSERT INTO t1 VALUES (2010),(2011);
+SELECT a=10 AND NULLIF(a,2011.1)='2011' AS cond FROM t1;
+cond
+0
+0
+SELECT * FROM t1 WHERE a=10;
+a
+2010
+SELECT * FROM t1 WHERE NULLIF(a,2011.1)='2011';
+a
+SELECT * FROM t1 WHERE a=10 AND NULLIF(a,2011.1)='2011';
+a
+EXPLAIN EXTENDED
+SELECT * FROM t1 WHERE a=10 AND NULLIF(a,2011.1)='2011';
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	filtered	Extra
+1	SIMPLE	NULL	NULL	NULL	NULL	NULL	NULL	NULL	NULL	Impossible WHERE
+Warnings:
+Note	1003	select `test`.`t1`.`a` AS `a` from `test`.`t1` where 0
+EXPLAIN EXTENDED
+SELECT * FROM t1 WHERE a=10 AND NULLIF(a,2011.1)=CONCAT('2011',RAND());
+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`.`a` AS `a` from `test`.`t1` where ((`test`.`t1`.`a` = 2010) and (<cache>((case when 2010 = 2011 then NULL else '2010' end)) = concat('2011',rand())))
+DROP TABLE t1;
+#
+# MDEV-8754 Wrong result for SELECT..WHERE year_field=2020 AND NULLIF(year_field,2010)='2020'
+#
+CREATE TABLE t1 (a YEAR);
+INSERT INTO t1 VALUES (2010),(2020);
+SELECT * FROM t1 WHERE a=2020;
+a
+2020
+SELECT * FROM t1 WHERE NULLIF(a,2010)='2020';
+a
+2020
+SELECT * FROM t1 WHERE a=2020 AND NULLIF(a,2010)='2020';
+a
+2020
+EXPLAIN EXTENDED
+SELECT * FROM t1 WHERE a=2020 AND NULLIF(a,2010)='2020';
+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`.`a` AS `a` from `test`.`t1` where (`test`.`t1`.`a` = 2020)
+EXPLAIN EXTENDED
+SELECT * FROM t1 WHERE a=2020 AND NULLIF(a,2010)=CONCAT('2020',RAND());
+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`.`a` AS `a` from `test`.`t1` where ((`test`.`t1`.`a` = 2020) and (<cache>((case when 2020 = 2010 then NULL else '2020' end)) = concat('2020',rand())))
+DROP TABLE t1;
+#
 # End of 10.1 tests
 #

mysql-test/t/null.test

@@ -879,6 +879,37 @@ EXPLAIN EXTENDED SELECT NULLIF(a,_utf8'a' COLLATE utf8_bin) IS NULL AS expr FROM
 DROP TABLE t1;
 
 
+--echo #
+--echo # MDEV-8740 Wrong result for SELECT..WHERE year_field=10 AND NULLIF(year_field,2011.1)='2011'
+--echo #
+CREATE TABLE t1 (a YEAR);
+INSERT INTO t1 VALUES (2010),(2011);
+SELECT a=10 AND NULLIF(a,2011.1)='2011' AS cond FROM t1;
+SELECT * FROM t1 WHERE a=10;
+SELECT * FROM t1 WHERE NULLIF(a,2011.1)='2011';
+SELECT * FROM t1 WHERE a=10 AND NULLIF(a,2011.1)='2011';
+EXPLAIN EXTENDED
+SELECT * FROM t1 WHERE a=10 AND NULLIF(a,2011.1)='2011';
+EXPLAIN EXTENDED
+SELECT * FROM t1 WHERE a=10 AND NULLIF(a,2011.1)=CONCAT('2011',RAND());
+DROP TABLE t1;
+
+
+--echo #
+--echo # MDEV-8754 Wrong result for SELECT..WHERE year_field=2020 AND NULLIF(year_field,2010)='2020'
+--echo #
+CREATE TABLE t1 (a YEAR);
+INSERT INTO t1 VALUES (2010),(2020);
+SELECT * FROM t1 WHERE a=2020;
+SELECT * FROM t1 WHERE NULLIF(a,2010)='2020';
+SELECT * FROM t1 WHERE a=2020 AND NULLIF(a,2010)='2020';
+EXPLAIN EXTENDED
+SELECT * FROM t1 WHERE a=2020 AND NULLIF(a,2010)='2020';
+EXPLAIN EXTENDED
+SELECT * FROM t1 WHERE a=2020 AND NULLIF(a,2010)=CONCAT('2020',RAND());
+DROP TABLE t1;
+
+
 --echo #
 --echo # End of 10.1 tests
 --echo #

sql/item.cc

@@ -6683,6 +6683,18 @@ bool Item_field::send(Protocol *protocol, String *buffer)
 }
 
 
+Item* Item::propagate_equal_fields_and_change_item_tree(THD *thd,
+                                                        const Context &ctx,
+                                                        COND_EQUAL *cond,
+                                                        Item **place)
+{
+  Item *item= propagate_equal_fields(thd, ctx, cond);
+  if (item && item != this)
+    thd->change_item_tree(place, item);
+  return item;
+}
+
+
 void Item_field::update_null_value() 
 { 
   /* 

sql/item.h

@@ -1455,6 +1455,11 @@ public:
     return this;
   };
 
+  Item* propagate_equal_fields_and_change_item_tree(THD *thd,
+                                                    const Context &ctx,
+                                                    COND_EQUAL *cond,
+                                                    Item **place);
+
   /*
     @brief
     Processor used to check acceptability of an item in the defining

sql/item_cmpfunc.cc

@@ -473,7 +473,7 @@ static bool convert_const_to_int(THD *thd, Item_field *field_item,
 */
 void Item_func::convert_const_compared_to_int_field(THD *thd)
 {
-  DBUG_ASSERT(arg_count == 2);
+  DBUG_ASSERT(arg_count >= 2); // Item_func_nullif has arg_count == 3
   if (!thd->lex->is_ps_or_view_context_analysis())
   {
     int field;
@@ -491,7 +491,7 @@ void Item_func::convert_const_compared_to_int_field(THD *thd)
 
 bool Item_func::setup_args_and_comparator(THD *thd, Arg_comparator *cmp)
 {
-  DBUG_ASSERT(arg_count == 2);
+  DBUG_ASSERT(arg_count >= 2); // Item_func_nullif has arg_count == 3
 
   if (args[0]->cmp_type() == STRING_RESULT &&
       args[1]->cmp_type() == STRING_RESULT &&
@@ -502,7 +502,7 @@ bool Item_func::setup_args_and_comparator(THD *thd, Arg_comparator *cmp)
   DBUG_ASSERT(functype() != LIKE_FUNC);
   convert_const_compared_to_int_field(thd);
 
-  return cmp->set_cmp_func(this, tmp_arg, tmp_arg + 1, true);
+  return cmp->set_cmp_func(this, &args[0], &args[1], true);
 }
 
 
@@ -2663,15 +2663,15 @@ bool Item_func_if::date_op(MYSQL_TIME *ltime, uint fuzzydate)
 void
 Item_func_nullif::fix_length_and_dec()
 {
-  if (!m_args0_copy)					// Only false if EOM
+  if (!args[2])					// Only false if EOM
     return;
 
-  cached_result_type= m_args0_copy->result_type();
-  cached_field_type= m_args0_copy->field_type();
-  collation.set(m_args0_copy->collation);
-  decimals= m_args0_copy->decimals;
-  unsigned_flag= m_args0_copy->unsigned_flag;
-  fix_char_length(m_args0_copy->max_char_length());
+  cached_result_type= args[2]->result_type();
+  cached_field_type= args[2]->field_type();
+  collation.set(args[2]->collation);
+  decimals= args[2]->decimals;
+  unsigned_flag= args[2]->unsigned_flag;
+  fix_char_length(args[2]->max_char_length());
   maybe_null=1;
   setup_args_and_comparator(current_thd, &cmp);
 }
@@ -2683,16 +2683,16 @@ void Item_func_nullif::print(String *str, enum_query_type query_type)
     NULLIF(a,b) is implemented according to the SQL standard as a short for
     CASE WHEN a=b THEN NULL ELSE a END
 
-    The constructor of Item_func_nullif sets args[0] and m_args0_copy to the
+    The constructor of Item_func_nullif sets args[0] and args[2] to the
     same item "a", and sets args[1] to "b".
 
     If "this" is a part of a WHERE or ON condition, then:
     - the left "a" is a subject to equal field propagation with ANY_SUBST.
     - the right "a" is a subject to equal field propagation with IDENTITY_SUBST.
-    Therefore, after equal field propagation args[0] and m_args0_copy can point
+    Therefore, after equal field propagation args[0] and args[2] can point
     to different items.
   */
-  if (!(query_type & QT_ITEM_FUNC_NULLIF_TO_CASE) || args[0] == m_args0_copy)
+  if (!(query_type & QT_ITEM_FUNC_NULLIF_TO_CASE) || args[0] == args[2])
   {
     /*
       If no QT_ITEM_FUNC_NULLIF_TO_CASE is requested,
@@ -2701,7 +2701,7 @@ void Item_func_nullif::print(String *str, enum_query_type query_type)
         SHOW CREATE {VIEW|FUNCTION|PROCEDURE}
 
       The original representation is possible only if
-      args[0] and m_args0_copy still point to the same Item.
+      args[0] and args[2] still point to the same Item.
 
       The caller must pass call print() with QT_ITEM_FUNC_NULLIF_TO_CASE
       if an expression has undergone some optimization
@@ -2713,10 +2713,10 @@ void Item_func_nullif::print(String *str, enum_query_type query_type)
       Note, the EXPLAIN EXTENDED and EXPLAIN FORMAT=JSON routines
       do pass QT_ITEM_FUNC_NULLIF_TO_CASE to print().
     */
-    DBUG_ASSERT(args[0] == m_args0_copy);
+    DBUG_ASSERT(args[0] == args[2]);
     str->append(func_name());
     str->append('(');
-    m_args0_copy->print(str, query_type);
+    args[2]->print(str, query_type);
     str->append(',');
     args[1]->print(str, query_type);
     str->append(')');
@@ -2724,7 +2724,7 @@ void Item_func_nullif::print(String *str, enum_query_type query_type)
   else
   {
     /*
-      args[0] and m_args0_copy are different items.
+      args[0] and args[2] are different items.
       This is possible after WHERE optimization (equal fields propagation etc),
       e.g. in EXPLAIN EXTENDED or EXPLAIN FORMAT=JSON.
       As it's not possible to print as a function with 2 arguments any more,
@@ -2735,7 +2735,7 @@ void Item_func_nullif::print(String *str, enum_query_type query_type)
     str->append(STRING_WITH_LEN(" = "));
     args[1]->print(str, query_type);
     str->append(STRING_WITH_LEN(" then NULL else "));
-    m_args0_copy->print(str, query_type);
+    args[2]->print(str, query_type);
     str->append(STRING_WITH_LEN(" end)"));
   }
 }
@@ -2761,8 +2761,8 @@ Item_func_nullif::real_op()
     null_value=1;
     return 0.0;
   }
-  value= m_args0_copy->val_real();
-  null_value=m_args0_copy->null_value;
+  value= args[2]->val_real();
+  null_value= args[2]->null_value;
   return value;
 }
 
@@ -2776,8 +2776,8 @@ Item_func_nullif::int_op()
     null_value=1;
     return 0;
   }
-  value=m_args0_copy->val_int();
-  null_value=m_args0_copy->null_value;
+  value= args[2]->val_int();
+  null_value= args[2]->null_value;
   return value;
 }
 
@@ -2791,8 +2791,8 @@ Item_func_nullif::str_op(String *str)
     null_value=1;
     return 0;
   }
-  res=m_args0_copy->val_str(str);
-  null_value=m_args0_copy->null_value;
+  res= args[2]->val_str(str);
+  null_value= args[2]->null_value;
   return res;
 }
 
@@ -2807,8 +2807,8 @@ Item_func_nullif::decimal_op(my_decimal * decimal_value)
     null_value=1;
     return 0;
   }
-  res= m_args0_copy->val_decimal(decimal_value);
-  null_value= m_args0_copy->null_value;
+  res= args[2]->val_decimal(decimal_value);
+  null_value= args[2]->null_value;
   return res;
 }
 
@@ -2819,14 +2819,14 @@ Item_func_nullif::date_op(MYSQL_TIME *ltime, uint fuzzydate)
   DBUG_ASSERT(fixed == 1);
   if (!cmp.compare())
     return (null_value= true);
-  return (null_value= m_args0_copy->get_date(ltime, fuzzydate));
+  return (null_value= args[2]->get_date(ltime, fuzzydate));
 }
 
 
 bool
 Item_func_nullif::is_null()
 {
-  return (null_value= (!cmp.compare() ? 1 : m_args0_copy->null_value)); 
+  return (null_value= (!cmp.compare() ? 1 : args[2]->null_value));
 }
 
 

sql/item_cmpfunc.h

@@ -907,18 +907,26 @@ class Item_func_nullif :public Item_func_hybrid_field_type
 {
   Arg_comparator cmp;
   /*
-    Remember the first argument in case it will be substituted by either of:
-    - convert_const_compared_to_int_field()
+    NULLIF(a,b) is a short for:
+      CASE WHEN a=b THEN NULL ELSE a END
+
+    The left "a" is for comparison purposes.
+    The right "a" is for return value purposes.
+    These are two different "a" and they can be replaced to different items.
+
+    The left "a" is in a comparison and can be replaced by:
+    - Item_func::convert_const_compared_to_int_field()
     - agg_item_set_converter() in set_cmp_func()
-    - cache_converted_constant() in set_cmp_func()
-    The original item will be stored in m_arg0_copy, to return result.
-    The substituted item will be stored in args[0], for comparison purposes.
+    - Arg_comparator::cache_converted_constant() in set_cmp_func()
+
+    Both "a"s are subject to equal fields propagation and can be replaced by:
+    - Item_field::propagate_equal_fields(ANY_SUBST) for the left "a"
+    - Item_field::propagate_equal_fields(IDENTITY_SUBST) for the right "a"
   */
-  Item *m_args0_copy;
 public:
+  // Put "a" to args[0] for comparison and to args[2] for the returned value.
   Item_func_nullif(THD *thd, Item *a, Item *b):
-    Item_func_hybrid_field_type(thd, a, b),
-     m_args0_copy(a)
+    Item_func_hybrid_field_type(thd, a, b, a)
   {}
   bool date_op(MYSQL_TIME *ltime, uint fuzzydate);
   double real_op();
@@ -926,18 +934,21 @@ public:
   String *str_op(String *str);
   my_decimal *decimal_op(my_decimal *);
   void fix_length_and_dec();
-  uint decimal_precision() const { return m_args0_copy->decimal_precision(); }
+  uint decimal_precision() const { return args[2]->decimal_precision(); }
   const char *func_name() const { return "nullif"; }
   void print(String *str, enum_query_type query_type);
   table_map not_null_tables() const { return 0; }
   bool is_null();
   Item* propagate_equal_fields(THD *thd, const Context &ctx, COND_EQUAL *cond)
   {
-    Item_args::propagate_equal_fields(thd,
-                                      Context(ANY_SUBST,
-                                              cmp.compare_type(),
-                                              cmp.cmp_collation.collation),
-                                      cond);
+    Context cmpctx(ANY_SUBST, cmp.compare_type(), cmp.cmp_collation.collation);
+    args[0]->propagate_equal_fields_and_change_item_tree(thd, cmpctx,
+                                                         cond, &args[0]);
+    args[1]->propagate_equal_fields_and_change_item_tree(thd, cmpctx,
+                                                         cond, &args[1]);
+    args[2]->propagate_equal_fields_and_change_item_tree(thd,
+                                                         Context_identity(),
+                                                         cond, &args[2]);
     return this;
   }
 };

sql/item_func.cc

@@ -420,13 +420,10 @@ void Item_args::propagate_equal_fields(THD *thd,
                                        const Item::Context &ctx,
                                        COND_EQUAL *cond)
 {
-  Item **arg,**arg_end;
-  for (arg= args, arg_end= args + arg_count; arg != arg_end; arg++)
-  {
-    Item *new_item= (*arg)->propagate_equal_fields(thd, ctx, cond);
-    if (new_item && *arg != new_item)
-      thd->change_item_tree(arg, new_item);
-  }
+  uint i;
+  for (i= 0; i < arg_count; i++)
+    args[i]->propagate_equal_fields_and_change_item_tree(thd, ctx, cond,
+                                                         &args[i]);
 }