From 8e553c455c4740a51d2a7d0e23c3c79863b5df22 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Thu, 10 Sep 2015 15:01:44 +0400 Subject: [PATCH] MDEV-8785 Wrong results for EXPLAIN EXTENDED...WHERE NULLIF(latin1_col, _utf8'a' COLLATE utf8_bin) IS NOT NULL --- mysql-test/r/explain_json.result | 20 ++++++++++ mysql-test/r/null.result | 19 ++++++++++ mysql-test/t/explain_json.test | 8 ++++ mysql-test/t/null.test | 10 +++++ sql/item_cmpfunc.cc | 64 ++++++++++++++++++++++++++++++++ sql/item_cmpfunc.h | 7 +--- sql/mysqld.h | 11 +++++- 7 files changed, 132 insertions(+), 7 deletions(-) diff --git a/mysql-test/r/explain_json.result b/mysql-test/r/explain_json.result index d741c1e5362..07ff72b4208 100644 --- a/mysql-test/r/explain_json.result +++ b/mysql-test/r/explain_json.result @@ -1090,3 +1090,23 @@ EXPLAIN } } DROP TABLE t1; +# +# MDEV-8785 Wrong results for EXPLAIN EXTENDED...WHERE NULLIF(latin1_col, _utf8'a' COLLATE utf8_bin) IS NOT NULL +# +CREATE TABLE t1 (a VARCHAR(10) CHARACTER SET latin1); +INSERT INTO t1 VALUES ('a'),('A'); +EXPLAIN FORMAT=JSON SELECT * FROM t1 WHERE NULLIF(a,_utf8'a' COLLATE utf8_bin); +EXPLAIN +{ + "query_block": { + "select_id": 1, + "table": { + "table_name": "t1", + "access_type": "ALL", + "rows": 2, + "filtered": 100, + "attached_condition": "(case when convert(t1.a using utf8) = ((_utf8'a' collate utf8_bin)) then NULL else t1.a end)" + } + } +} +DROP TABLE t1; diff --git a/mysql-test/r/null.result b/mysql-test/r/null.result index debb3c05681..e5ce6a10f09 100644 --- a/mysql-test/r/null.result +++ b/mysql-test/r/null.result @@ -1390,5 +1390,24 @@ Warning 1292 Incorrect datetime value: '1' Warning 1292 Incorrect datetime value: '1' DROP TABLE t1; # +# MDEV-8785 Wrong results for EXPLAIN EXTENDED...WHERE NULLIF(latin1_col, _utf8'a' COLLATE utf8_bin) IS NOT NULL +# +CREATE TABLE t1 (a VARCHAR(10) CHARACTER SET latin1); +INSERT INTO t1 VALUES ('a'),('A'); +SELECT a, NULLIF(a,_utf8'a' COLLATE utf8_bin) IS NULL FROM t1; +a NULLIF(a,_utf8'a' COLLATE utf8_bin) IS NULL +a 1 +A 0 +SELECT CHARSET(NULLIF(a,_utf8'a' COLLATE utf8_bin)) FROM t1; +CHARSET(NULLIF(a,_utf8'a' COLLATE utf8_bin)) +latin1 +latin1 +EXPLAIN EXTENDED SELECT NULLIF(a,_utf8'a' COLLATE utf8_bin) IS NULL AS expr FROM t1; +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 +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; +# # End of 10.1 tests # diff --git a/mysql-test/t/explain_json.test b/mysql-test/t/explain_json.test index e12cbed6d52..1078222725e 100644 --- a/mysql-test/t/explain_json.test +++ b/mysql-test/t/explain_json.test @@ -286,3 +286,11 @@ CREATE TABLE t1 (a VARCHAR(10) CHARACTER SET latin1); INSERT INTO t1 VALUES ('a'),('b'); EXPLAIN FORMAT=JSON SELECT * FROM t1 WHERE a=_latin1 0xDF; DROP TABLE t1; + +--echo # +--echo # MDEV-8785 Wrong results for EXPLAIN EXTENDED...WHERE NULLIF(latin1_col, _utf8'a' COLLATE utf8_bin) IS NOT NULL +--echo # +CREATE TABLE t1 (a VARCHAR(10) CHARACTER SET latin1); +INSERT INTO t1 VALUES ('a'),('A'); +EXPLAIN FORMAT=JSON SELECT * FROM t1 WHERE NULLIF(a,_utf8'a' COLLATE utf8_bin); +DROP TABLE t1; diff --git a/mysql-test/t/null.test b/mysql-test/t/null.test index 65a45b9a21c..695c22f3bbd 100644 --- a/mysql-test/t/null.test +++ b/mysql-test/t/null.test @@ -868,6 +868,16 @@ CREATE TABLE t1 AS SELECT END AS b; DROP TABLE t1; +--echo # +--echo # MDEV-8785 Wrong results for EXPLAIN EXTENDED...WHERE NULLIF(latin1_col, _utf8'a' COLLATE utf8_bin) IS NOT NULL +--echo # +CREATE TABLE t1 (a VARCHAR(10) CHARACTER SET latin1); +INSERT INTO t1 VALUES ('a'),('A'); +SELECT a, NULLIF(a,_utf8'a' COLLATE utf8_bin) IS NULL FROM t1; +SELECT CHARSET(NULLIF(a,_utf8'a' COLLATE utf8_bin)) FROM t1; +EXPLAIN EXTENDED SELECT NULLIF(a,_utf8'a' COLLATE utf8_bin) IS NULL AS expr FROM t1; +DROP TABLE t1; + --echo # --echo # End of 10.1 tests diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 3bfee49560c..04afafd2915 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -2677,6 +2677,70 @@ Item_func_nullif::fix_length_and_dec() } +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 + 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 + to different items. + */ + if (!(query_type & QT_ITEM_FUNC_NULLIF_TO_CASE) || args[0] == m_args0_copy) + { + /* + If no QT_ITEM_FUNC_NULLIF_TO_CASE is requested, + that means we want the original NULLIF() representation, + e.g. when we are in: + SHOW CREATE {VIEW|FUNCTION|PROCEDURE} + + The original representation is possible only if + args[0] and m_args0_copy 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 + (e.g. equal field propagation done in optimize_cond()) already and + NULLIF() potentially has two different representations of "a": + - one "a" for comparison + - another "a" for the returned value! + + 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); + str->append(func_name()); + str->append('('); + m_args0_copy->print(str, query_type); + str->append(','); + args[1]->print(str, query_type); + str->append(')'); + } + else + { + /* + args[0] and m_args0_copy 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, + do it in the CASE style. + */ + str->append(STRING_WITH_LEN("(case when ")); + args[0]->print(str, 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); + str->append(STRING_WITH_LEN(" end)")); + } +} + + /** @note Note that we have to evaluate the first argument twice as the compare diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index e5d20e6e33c..436bbcd259a 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -928,12 +928,7 @@ public: void fix_length_and_dec(); uint decimal_precision() const { return m_args0_copy->decimal_precision(); } const char *func_name() const { return "nullif"; } - - virtual inline void print(String *str, enum_query_type query_type) - { - Item_func::print(str, query_type); - } - + 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) diff --git a/sql/mysqld.h b/sql/mysqld.h index f19c322c96b..bf1af3afb94 100644 --- a/sql/mysqld.h +++ b/sql/mysqld.h @@ -654,9 +654,18 @@ enum enum_query_type /// If Item_subselect should print as just "(subquery#1)" /// rather than display the subquery body QT_ITEM_SUBSELECT_ID_ONLY= (1 << 5), + /// If NULLIF(a,b) should print itself as + /// CASE WHEN a_for_comparison=b THEN NULL ELSE a_for_return_value END + /// when "a" was replaced to two different items + /// (e.g. by equal fields propagation in optimize_cond()). + /// The default behaviour is to print as NULLIF(a_for_return, b) + /// which should be Ok for SHOW CREATE {VIEW|PROCEDURE|FUNCTION} + /// as they are not affected by WHERE optimization. + QT_ITEM_FUNC_NULLIF_TO_CASE= (1 <<6), /// This value means focus on readability, not on ability to parse back, etc. QT_EXPLAIN= QT_TO_SYSTEM_CHARSET | + QT_ITEM_FUNC_NULLIF_TO_CASE | QT_ITEM_IDENT_SKIP_CURRENT_DATABASE | QT_ITEM_CACHE_WRAPPER_SKIP_DETAILS | QT_ITEM_SUBSELECT_ID_ONLY, @@ -665,7 +674,7 @@ enum enum_query_type /// Be more detailed than QT_EXPLAIN. /// Perhaps we should eventually include QT_ITEM_IDENT_SKIP_CURRENT_DATABASE /// here, as it would give better readable results - QT_EXPLAIN_EXTENDED= QT_TO_SYSTEM_CHARSET + QT_EXPLAIN_EXTENDED= QT_TO_SYSTEM_CHARSET | QT_ITEM_FUNC_NULLIF_TO_CASE };