diff --git a/mysql-test/r/null.result b/mysql-test/r/null.result index e5ce6a10f09..04044602a68 100644 --- a/mysql-test/r/null.result +++ b/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 # diff --git a/mysql-test/t/null.test b/mysql-test/t/null.test index 695c22f3bbd..5347a961c59 100644 --- a/mysql-test/t/null.test +++ b/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 # diff --git a/sql/item.cc b/sql/item.cc index 9762ff3d5e7..3182f9d6a47 100644 --- a/sql/item.cc +++ b/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() { /* diff --git a/sql/item.h b/sql/item.h index 3dfb7d8ec06..d3dbc543e82 100644 --- a/sql/item.h +++ b/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 diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 04afafd2915..7a7fd98dd74 100644 --- a/sql/item_cmpfunc.cc +++ b/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)); } diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 436bbcd259a..cc94a8c08e6 100644 --- a/sql/item_cmpfunc.h +++ b/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; } }; diff --git a/sql/item_func.cc b/sql/item_func.cc index 23fb45fb037..bc4a39b577b 100644 --- a/sql/item_func.cc +++ b/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]); }