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.

Author: Alexander Barkov

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 @@ class Item: public Value_source, public Type_std_attributes
  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,18 +2713,18 @@ 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(')');
  }
  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,37 +907,48 @@ 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();
  longlong int_op();
  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]);
 }