diff options
| -rw-r--r-- | mysql-test/r/derived.result | 45 | ||||
| -rw-r--r-- | mysql-test/r/group_by.result | 4 | ||||
| -rw-r--r-- | mysql-test/r/view.result | 90 | ||||
| -rw-r--r-- | mysql-test/t/derived.test | 34 | ||||
| -rw-r--r-- | mysql-test/t/view.test | 56 | ||||
| -rw-r--r-- | sql/item.cc | 49 | ||||
| -rw-r--r-- | sql/item.h | 49 | ||||
| -rw-r--r-- | sql/item_cmpfunc.cc | 87 | ||||
| -rw-r--r-- | sql/item_cmpfunc.h | 84 | ||||
| -rw-r--r-- | sql/item_func.h | 2 | ||||
| -rw-r--r-- | sql/sql_select.cc | 11 |
11 files changed, 373 insertions, 138 deletions
diff --git a/mysql-test/r/derived.result b/mysql-test/r/derived.result index bb1e4d51dd6..a59f8a6dfee 100644 --- a/mysql-test/r/derived.result +++ b/mysql-test/r/derived.result @@ -604,3 +604,48 @@ where coalesce(message,0) <> 0; id message drop table t1,t2; set optimizer_switch=@save_derived_optimizer_switch; +# +# Start of 10.1 tests +# +# +# MDEV-8747 Wrong result for SELECT..WHERE derived_table_column='a' AND derived_table_column<>_latin1'A' COLLATE latin1_bin +# +CREATE TABLE t1 (a VARCHAR(10)); +INSERT INTO t1 VALUES ('a'),('A'); +SELECT * FROM t1 WHERE a='a' AND a <> _latin1'A' COLLATE latin1_bin; +a +a +SELECT * FROM (SELECT * FROM t1) AS table1 WHERE a='a' AND a <> _latin1'A' COLLATE latin1_bin; +a +a +DROP TABLE t1; +CREATE TABLE t1 (a ENUM('5','6')); +INSERT INTO t1 VALUES ('5'),('6'); +SELECT * FROM (SELECT * FROM t1) AS table1 WHERE a='5'; +a +5 +SELECT * FROM (SELECT * FROM t1) AS table1 WHERE a=1; +a +5 +SELECT * FROM (SELECT * FROM t1) AS table1 WHERE a='5' AND a=1; +a +5 +DROP TABLE t1; +# +# MDEV-8749 Wrong result for SELECT..WHERE derived_table_enum_column='number' AND derived_table_enum_column OP number2 +# +CREATE TABLE t1 (a ENUM('5','6')); +INSERT INTO t1 VALUES ('5'),('6'); +SELECT * FROM (SELECT * FROM t1) AS table1 WHERE a='5'; +a +5 +SELECT * FROM (SELECT * FROM t1) AS table1 WHERE a=1; +a +5 +SELECT * FROM (SELECT * FROM t1) AS table1 WHERE a='5' AND a=1; +a +5 +DROP TABLE t1; +# +# End of 10.1 tests +# diff --git a/mysql-test/r/group_by.result b/mysql-test/r/group_by.result index 483d4d1ca8e..7f32643b727 100644 --- a/mysql-test/r/group_by.result +++ b/mysql-test/r/group_by.result @@ -2289,20 +2289,16 @@ field1 field2 2004-10-11 18:13:00 1 2009-02-19 02:05:00 5 Warnings: -Warning 1292 Truncated incorrect DOUBLE value: 'x' Warning 1292 Truncated incorrect DOUBLE value: 'g' Warning 1292 Truncated incorrect DOUBLE value: 'o' -Warning 1292 Truncated incorrect DOUBLE value: 'g' Warning 1292 Truncated incorrect DOUBLE value: 'v' SELECT alias2.f3 AS field1 , alias2.f1 AS field2 FROM t1 AS alias1 JOIN t1 AS alias2 ON alias2.f1 = alias1.f2 AND alias2.f1 != alias1.f4 GROUP BY field1 , field2 ; field1 field2 2004-10-11 18:13:00 1 2009-02-19 02:05:00 5 Warnings: -Warning 1292 Truncated incorrect DOUBLE value: 'x' Warning 1292 Truncated incorrect DOUBLE value: 'g' Warning 1292 Truncated incorrect DOUBLE value: 'o' -Warning 1292 Truncated incorrect DOUBLE value: 'g' Warning 1292 Truncated incorrect DOUBLE value: 'v' SET SESSION SQL_MODE=default; drop table t1; diff --git a/mysql-test/r/view.result b/mysql-test/r/view.result index df2b48564ad..4b46a67fc23 100644 --- a/mysql-test/r/view.result +++ b/mysql-test/r/view.result @@ -4430,36 +4430,58 @@ CREATE TABLE t1 (a varchar(10), KEY (a)) ; INSERT INTO t1 VALUES ('DD'), ('ZZ'), ('ZZ'), ('KK'), ('FF'), ('HH'),('MM'); CREATE VIEW v1 AS SELECT * FROM t1; +# t1 and v1 should return the same result set SELECT * FROM v1 WHERE a > 'JJ' OR a <> 0 AND a = 'VV'; a KK MM ZZ ZZ -Warnings: -Warning 1292 Truncated incorrect DOUBLE value: 'VV' +SELECT * FROM t1 WHERE a > 'JJ' OR a <> 0 AND a = 'VV'; +a +KK +MM +ZZ +ZZ +# t1 and v1 should propagate constants in the same way EXPLAIN EXTENDED SELECT * FROM v1 WHERE a > 'JJ' OR a <> 0 AND a = 'VV'; id select_type table type possible_keys key key_len ref rows filtered Extra 1 SIMPLE t1 range a a 13 NULL 4 100.00 Using where; Using index Warnings: -Warning 1292 Truncated incorrect DOUBLE value: 'VV' -Note 1003 select `test`.`t1`.`a` AS `a` from `test`.`t1` where (`test`.`t1`.`a` > 'JJ') +Note 1003 select `test`.`t1`.`a` AS `a` from `test`.`t1` where ((`test`.`t1`.`a` > 'JJ') or ((`test`.`t1`.`a` = 'VV') and (`test`.`t1`.`a` <> 0))) +EXPLAIN EXTENDED +SELECT * FROM t1 WHERE a > 'JJ' OR a <> 0 AND a = 'VV'; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 SIMPLE t1 range a a 13 NULL 4 100.00 Using where; Using index +Warnings: +Note 1003 select `test`.`t1`.`a` AS `a` from `test`.`t1` where ((`test`.`t1`.`a` > 'JJ') or ((`test`.`t1`.`a` = 'VV') and (`test`.`t1`.`a` <> 0))) +# t1 and v1 should return the same result set SELECT * FROM v1 WHERE a > 'JJ' OR a AND a = 'VV'; a KK MM ZZ ZZ -Warnings: -Warning 1292 Truncated incorrect DOUBLE value: 'VV' +SELECT * FROM t1 WHERE a > 'JJ' OR a AND a = 'VV'; +a +KK +MM +ZZ +ZZ +# t1 and v1 should propagate constants in the same way EXPLAIN EXTENDED SELECT * FROM v1 WHERE a > 'JJ' OR a AND a = 'VV'; id select_type table type possible_keys key key_len ref rows filtered Extra 1 SIMPLE t1 range a a 13 NULL 4 100.00 Using where; Using index Warnings: -Warning 1292 Truncated incorrect DOUBLE value: 'VV' -Note 1003 select `test`.`t1`.`a` AS `a` from `test`.`t1` where (`test`.`t1`.`a` > 'JJ') +Note 1003 select `test`.`t1`.`a` AS `a` from `test`.`t1` where ((`test`.`t1`.`a` > 'JJ') or ((`test`.`t1`.`a` = 'VV') and (`test`.`t1`.`a` <> 0))) +EXPLAIN EXTENDED +SELECT * FROM t1 WHERE a > 'JJ' OR a AND a = 'VV'; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 SIMPLE t1 range a a 13 NULL 4 100.00 Using where; Using index +Warnings: +Note 1003 select `test`.`t1`.`a` AS `a` from `test`.`t1` where ((`test`.`t1`.`a` > 'JJ') or ((`test`.`t1`.`a` = 'VV') and (`test`.`t1`.`a` <> 0))) DROP VIEW v1; DROP TABLE t1; # @@ -5571,3 +5593,55 @@ drop view v3; # -- End of 10.0 tests. # ----------------------------------------------------------------- SET optimizer_switch=@save_optimizer_switch; +# +# Start of 10.1 tests +# +# +# MDEV-8747 Wrong result for SELECT..WHERE derived_table_column='a' AND derived_table_column<>_latin1'A' COLLATE latin1_bin +# +CREATE TABLE t1 (a varchar(10) character set cp1251 collate cp1251_ukrainian_ci, KEY (a)) ; +INSERT INTO t1 VALUES ('DD'), ('ZZ'), ('ZZ'), ('KK'), ('FF'), ('HH'),('MM'),('`1'); +CREATE VIEW v1 AS SELECT * FROM t1; +SELECT * FROM t1 WHERE a <> 0 AND a = ' 1'; +a +SELECT * FROM v1 WHERE a <> 0 AND a = ' 1'; +a +DROP VIEW v1; +DROP TABLE t1; +CREATE TABLE t1 (a ENUM('5','6')); +INSERT INTO t1 VALUES ('5'),('6'); +CREATE VIEW v1 AS SELECT * FROM t1; +SELECT * FROM t1 WHERE a='5' AND a<2; +a +5 +SELECT * FROM v1 WHERE a='5' AND a<2; +a +5 +DROP VIEW v1; +DROP TABLE t1; +# +# MDEV-8749 Wrong result for SELECT..WHERE derived_table_enum_column='number' AND derived_table_enum_column OP number2 +# +CREATE TABLE t1 (a varchar(10) character set cp1251 collate cp1251_ukrainian_ci, KEY (a)); +INSERT INTO t1 VALUES ('DD'), ('ZZ'), ('ZZ'), ('KK'), ('FF'), ('HH'),('MM'),('`1'); +CREATE VIEW v1 AS SELECT * FROM t1; +SELECT * FROM t1 WHERE a <> 0 AND a = ' 1'; +a +SELECT * FROM v1 WHERE a <> 0 AND a = ' 1'; +a +DROP VIEW v1; +DROP TABLE t1; +CREATE TABLE t1 (a ENUM('5','6')); +INSERT INTO t1 VALUES ('5'),('6'); +CREATE VIEW v1 AS SELECT * FROM t1; +SELECT * FROM t1 WHERE a='5' AND a<2; +a +5 +SELECT * FROM v1 WHERE a='5' AND a<2; +a +5 +DROP VIEW v1; +DROP TABLE t1; +# +# End of 10.1 tests +# diff --git a/mysql-test/t/derived.test b/mysql-test/t/derived.test index d98e7b56905..38636b0e971 100644 --- a/mysql-test/t/derived.test +++ b/mysql-test/t/derived.test @@ -539,3 +539,37 @@ where coalesce(message,0) <> 0; drop table t1,t2; set optimizer_switch=@save_derived_optimizer_switch; + +--echo # +--echo # Start of 10.1 tests +--echo # + +--echo # +--echo # MDEV-8747 Wrong result for SELECT..WHERE derived_table_column='a' AND derived_table_column<>_latin1'A' COLLATE latin1_bin +--echo # +CREATE TABLE t1 (a VARCHAR(10)); +INSERT INTO t1 VALUES ('a'),('A'); +SELECT * FROM t1 WHERE a='a' AND a <> _latin1'A' COLLATE latin1_bin; +SELECT * FROM (SELECT * FROM t1) AS table1 WHERE a='a' AND a <> _latin1'A' COLLATE latin1_bin; +DROP TABLE t1; + +CREATE TABLE t1 (a ENUM('5','6')); +INSERT INTO t1 VALUES ('5'),('6'); +SELECT * FROM (SELECT * FROM t1) AS table1 WHERE a='5'; +SELECT * FROM (SELECT * FROM t1) AS table1 WHERE a=1; +SELECT * FROM (SELECT * FROM t1) AS table1 WHERE a='5' AND a=1; +DROP TABLE t1; + +--echo # +--echo # MDEV-8749 Wrong result for SELECT..WHERE derived_table_enum_column='number' AND derived_table_enum_column OP number2 +--echo # +CREATE TABLE t1 (a ENUM('5','6')); +INSERT INTO t1 VALUES ('5'),('6'); +SELECT * FROM (SELECT * FROM t1) AS table1 WHERE a='5'; +SELECT * FROM (SELECT * FROM t1) AS table1 WHERE a=1; +SELECT * FROM (SELECT * FROM t1) AS table1 WHERE a='5' AND a=1; +DROP TABLE t1; + +--echo # +--echo # End of 10.1 tests +--echo # diff --git a/mysql-test/t/view.test b/mysql-test/t/view.test index 26faae545d8..d580e98ecb6 100644 --- a/mysql-test/t/view.test +++ b/mysql-test/t/view.test @@ -4318,13 +4318,23 @@ INSERT INTO t1 VALUES CREATE VIEW v1 AS SELECT * FROM t1; +--echo # t1 and v1 should return the same result set SELECT * FROM v1 WHERE a > 'JJ' OR a <> 0 AND a = 'VV'; +SELECT * FROM t1 WHERE a > 'JJ' OR a <> 0 AND a = 'VV'; +--echo # t1 and v1 should propagate constants in the same way EXPLAIN EXTENDED SELECT * FROM v1 WHERE a > 'JJ' OR a <> 0 AND a = 'VV'; +EXPLAIN EXTENDED +SELECT * FROM t1 WHERE a > 'JJ' OR a <> 0 AND a = 'VV'; +--echo # t1 and v1 should return the same result set SELECT * FROM v1 WHERE a > 'JJ' OR a AND a = 'VV'; +SELECT * FROM t1 WHERE a > 'JJ' OR a AND a = 'VV'; +--echo # t1 and v1 should propagate constants in the same way EXPLAIN EXTENDED SELECT * FROM v1 WHERE a > 'JJ' OR a AND a = 'VV'; +EXPLAIN EXTENDED +SELECT * FROM t1 WHERE a > 'JJ' OR a AND a = 'VV'; DROP VIEW v1; DROP TABLE t1; @@ -5450,3 +5460,49 @@ drop view v3; --echo # -- End of 10.0 tests. --echo # ----------------------------------------------------------------- SET optimizer_switch=@save_optimizer_switch; + +--echo # +--echo # Start of 10.1 tests +--echo # + +--echo # +--echo # MDEV-8747 Wrong result for SELECT..WHERE derived_table_column='a' AND derived_table_column<>_latin1'A' COLLATE latin1_bin +--echo # +CREATE TABLE t1 (a varchar(10) character set cp1251 collate cp1251_ukrainian_ci, KEY (a)) ; +INSERT INTO t1 VALUES ('DD'), ('ZZ'), ('ZZ'), ('KK'), ('FF'), ('HH'),('MM'),('`1'); +CREATE VIEW v1 AS SELECT * FROM t1; +SELECT * FROM t1 WHERE a <> 0 AND a = ' 1'; +SELECT * FROM v1 WHERE a <> 0 AND a = ' 1'; +DROP VIEW v1; +DROP TABLE t1; + +CREATE TABLE t1 (a ENUM('5','6')); +INSERT INTO t1 VALUES ('5'),('6'); +CREATE VIEW v1 AS SELECT * FROM t1; +SELECT * FROM t1 WHERE a='5' AND a<2; +SELECT * FROM v1 WHERE a='5' AND a<2; +DROP VIEW v1; +DROP TABLE t1; + +--echo # +--echo # MDEV-8749 Wrong result for SELECT..WHERE derived_table_enum_column='number' AND derived_table_enum_column OP number2 +--echo # +CREATE TABLE t1 (a varchar(10) character set cp1251 collate cp1251_ukrainian_ci, KEY (a)); +INSERT INTO t1 VALUES ('DD'), ('ZZ'), ('ZZ'), ('KK'), ('FF'), ('HH'),('MM'),('`1'); +CREATE VIEW v1 AS SELECT * FROM t1; +SELECT * FROM t1 WHERE a <> 0 AND a = ' 1'; +SELECT * FROM v1 WHERE a <> 0 AND a = ' 1'; +DROP VIEW v1; +DROP TABLE t1; + +CREATE TABLE t1 (a ENUM('5','6')); +INSERT INTO t1 VALUES ('5'),('6'); +CREATE VIEW v1 AS SELECT * FROM t1; +SELECT * FROM t1 WHERE a='5' AND a<2; +SELECT * FROM v1 WHERE a='5' AND a<2; +DROP VIEW v1; +DROP TABLE t1; + +--echo # +--echo # End of 10.1 tests +--echo # diff --git a/sql/item.cc b/sql/item.cc index 823b8470f4a..7d7bced4872 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -422,7 +422,6 @@ Item::Item(THD *thd): maybe_null=null_value=with_sum_func=with_field=0; in_rollup= 0; with_subselect= 0; - cmp_context= IMPOSSIBLE_RESULT; /* Initially this item is not attached to any JOIN_TAB. */ join_tab_idx= MAX_TABLES; @@ -468,8 +467,7 @@ Item::Item(THD *thd, Item *item): with_field(item->with_field), fixed(item->fixed), is_autogenerated_name(item->is_autogenerated_name), - with_subselect(item->has_subquery()), - cmp_context(item->cmp_context) + with_subselect(item->has_subquery()) { next= thd->free_list; // Put in free list thd->free_list= this; @@ -5371,7 +5369,7 @@ 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 (cmp_context == STRING_RESULT && + if (ctx.compare_type() == STRING_RESULT && ctx.compare_collation() != item_equal->compare_collation()) return false; return ctx.subst_constraint() == ANY_SUBST || @@ -5440,31 +5438,33 @@ Item *Item_field::propagate_equal_fields(THD *thd, { if (no_const_subst) return this; - item_equal= find_item_equal((COND_EQUAL *) arg); + item_equal= find_item_equal(arg); Item *item= 0; if (item_equal) { - if (!can_be_substituted_to_equal_item(ctx, 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())) { item_equal= NULL; return this; } item= item_equal->get_const(); } - /* - 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 (!item || !has_compatible_context(item)) + if (!item) item= this; else if (field && (field->flags & ZEROFILL_FLAG) && IS_NUM(field->type())) { - if (item && (cmp_context == STRING_RESULT || cmp_context == IMPOSSIBLE_RESULT)) + DBUG_ASSERT(ctx.compare_type() != STRING_RESULT); + if (item && (ctx.subst_constraint() == IDENTITY_SUBST)) convert_zerofill_number_to_string(thd, &item, (Field_num *)field); else item= this; @@ -5521,8 +5521,19 @@ Item *Item_field::replace_equal_field(THD *thd, uchar *arg) Item *const_item2= item_equal->get_const(); if (const_item2) { - if (!has_compatible_context(const_item2)) - return this; + /* + Currently we don't allow to create Item_equal with compare_type() + different from its Item_field's cmp_type(). + Field_xxx::test_if_equality_guarantees_uniqueness() prevent this. + Also, Item_field::propagate_equal_fields() does not allow to assign + this->item_equal to any instances of Item_equal if "this" is used + in a non-native comparison context, or with an incompatible collation. + So the fact that we have (item_equal != NULL) means that the currently + processed function (the owner of "this") uses the field in its native + comparison context, and it's safe to replace it to the constant from + item_equal. + */ + DBUG_ASSERT(cmp_type() == item_equal->compare_type()); return const_item2; } Item_field *subst= diff --git a/sql/item.h b/sql/item.h index 5b96e93dd1f..f858cc811c1 100644 --- a/sql/item.h +++ b/sql/item.h @@ -693,7 +693,6 @@ public: bool with_subselect; /* If this item is a subselect or some of its arguments is or contains a subselect */ - Item_result cmp_context; /* Comparison context */ // alloc & destruct is done as start of select using sql_alloc Item(THD *thd); /* @@ -1434,8 +1433,6 @@ public: Comparison functions pass their attributes to propagate_equal_fields(). For exmple, for string comparison, the collation of the comparison operation is important inside propagate_equal_fields(). - TODO: We should move Item::cmp_context to from Item to Item::Context - eventually. */ class Context { @@ -1445,18 +1442,44 @@ public: - SUBST_IDENTITY (strict binary equality). */ Subst_constraint m_subst_constraint; + Item_result m_compare_type; /* Collation of the comparison operation. Important only when SUBST_ANY. */ CHARSET_INFO *m_compare_collation; public: - Context(Subst_constraint subst, CHARSET_INFO *cs) - :m_subst_constraint(subst), m_compare_collation(cs) { } - Context(Subst_constraint subst) - :m_subst_constraint(subst), m_compare_collation(&my_charset_bin) { } + Context(Subst_constraint subst, Item_result type, CHARSET_INFO *cs) + :m_subst_constraint(subst), + 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 + { + return m_compare_type == IMPOSSIBLE_RESULT || + m_compare_type == other; + } + }; + 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) { } + }; + class Context_boolean: public Context + { // Use this when an item is [a part of] a boolean expression + public: + Context_boolean() :Context(ANY_SUBST, INT_RESULT, &my_charset_bin) { } }; virtual Item* propagate_equal_fields(THD*, const Context &, COND_EQUAL *) @@ -1621,18 +1644,6 @@ public: return 0; } /** - Check whether this and the given item has compatible comparison context. - 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 *item) const - { - return cmp_context == IMPOSSIBLE_RESULT || item->cmp_context == cmp_context; - } - /** Test whether an expression is expensive to compute. Used during optimization to avoid computing expensive expressions during this phase. Also used to force temp tables when sorting on expensive diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 3e5460ef055..d6a113e0ecb 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -482,9 +482,8 @@ void Item_func::convert_const_compared_to_int_field(THD *thd) { Item_field *field_item= (Item_field*) (args[field]->real_item()); if ((field_item->field_type() == MYSQL_TYPE_LONGLONG || - field_item->field_type() == MYSQL_TYPE_YEAR) && - convert_const_to_int(thd, field_item, &args[!field])) - args[0]->cmp_context= args[1]->cmp_context= INT_RESULT; + field_item->field_type() == MYSQL_TYPE_YEAR)) + convert_const_to_int(thd, field_item, &args[!field]); } } } @@ -499,8 +498,6 @@ bool Item_func::setup_args_and_comparator(THD *thd, Arg_comparator *cmp) agg_arg_charsets_for_comparison(cmp->cmp_collation, args, 2)) return true; - args[0]->cmp_context= args[1]->cmp_context= item_cmp_type(args[0], args[1]); - // Convert constants when compared to int/year field DBUG_ASSERT(functype() != LIKE_FUNC); convert_const_compared_to_int_field(thd); @@ -551,10 +548,8 @@ int Arg_comparator::set_compare_func(Item_func_or_sum *item, Item_result type) my_error(ER_OPERAND_COLUMNS, MYF(0), (*a)->element_index(i)->cols()); return 1; } - if (comparators[i].set_cmp_func_and_arg_cmp_context(owner, - (*a)->addr(i), - (*b)->addr(i), - set_null)) + if (comparators[i].set_cmp_func(owner, (*a)->addr(i), + (*b)->addr(i), set_null)) return 1; } break; @@ -736,6 +731,7 @@ int Arg_comparator::set_cmp_func(Item_func_or_sum *owner_arg, set_null= set_null && owner_arg; a= a1; b= a2; + m_compare_type= type; if (type == STRING_RESULT && (*a)->result_type() == STRING_RESULT && @@ -2199,8 +2195,7 @@ void Item_func_between::fix_length_and_dec() return; if (agg_cmp_type(&m_compare_type, args, 3)) return; - args[0]->cmp_context= args[1]->cmp_context= args[2]->cmp_context= - m_compare_type; + if (m_compare_type == STRING_RESULT && agg_arg_charsets_for_comparison(cmp_collation, args, 3)) return; @@ -3137,26 +3132,6 @@ void Item_func_case::fix_length_and_dec() return; } } - /* - If only one type was found and it matches args[0]->cmp_type(), - set args[0]->cmp_context to this type. This is needed to make sure that - equal field propagation for args[0] works correctly, according to the - type found. - Otherwise, in case of multiple types or in case of a single type that - differs from args[0]->cmp_type(), it's Okey to keep args[0]->cmp_context - equal to IMPOSSIBLE_RESULT, as propagation for args[0] won't be - performed anyway. - */ - if (m_found_types == (1UL << left_cmp_type)) - args[0]->cmp_context= left_cmp_type; - /* - Set cmp_context of all WHEN arguments. This prevents - Item_field::propagate_equal_fields() from transforming a - zerofill argument into a string constant. Such a change would - require rebuilding cmp_items. - */ - for (i= 0; i < ncases; i+= 2) - args[i]->cmp_context= item_cmp_type(left_cmp_type, args[i]); } } @@ -3166,7 +3141,7 @@ Item* Item_func_case::propagate_equal_fields(THD *thd, const Context &ctx, COND_ if (first_expr_num == -1) { // None of the arguments are in a comparison context - Item_args::propagate_equal_fields(thd, IDENTITY_SUBST, cond); + Item_args::propagate_equal_fields(thd, Context_identity(), cond); return this; } @@ -3183,12 +3158,36 @@ Item* Item_func_case::propagate_equal_fields(THD *thd, const Context &ctx, COND_ { /* Cannot replace the CASE (the switch) argument if - there are multiple comparison types were found. + there are multiple comparison types were found, or found a single + comparison type that is not equal to args[0]->cmp_type(). + + - Example: multiple comparison types, can't propagate: + WHERE CASE str_column + WHEN 'string' THEN TRUE + WHEN 1 THEN TRUE + ELSE FALSE END; + + - Example: a single incompatible comparison type, can't propagate: + WHERE CASE str_column + WHEN DATE'2001-01-01' THEN TRUE + ELSE FALSE END; + + - Example: a single incompatible comparison type, can't propagate: + WHERE CASE str_column + WHEN 1 THEN TRUE + ELSE FALSE END; + + - Example: a single compatible comparison type, ok to propagate: + WHERE CASE str_column + WHEN 'str1' THEN TRUE + WHEN 'str2' THEN TRUE + ELSE FALSE END; */ if (m_found_types == (1UL << left_cmp_type)) new_item= args[i]->propagate_equal_fields(thd, Context( ANY_SUBST, + left_cmp_type, cmp_collation.collation), cond); } @@ -3197,16 +3196,21 @@ Item* Item_func_case::propagate_equal_fields(THD *thd, const Context &ctx, COND_ /* These arguments are in comparison. Allow invariants of the same value during propagation. + Note, as we pass ANY_SUBST, none of the WHEN arguments will be + replaced to zero-filled constants (only IDENTITY_SUBST allows this). + Such a change for WHEN arguments would require rebuilding cmp_items. */ + Item_result tmp_cmp_type= item_cmp_type(args[first_expr_num], args[i]); new_item= args[i]->propagate_equal_fields(thd, Context( ANY_SUBST, + tmp_cmp_type, cmp_collation.collation), cond); } else // THEN and ELSE arguments (they are not in comparison) { - new_item= args[i]->propagate_equal_fields(thd, IDENTITY_SUBST, cond); + new_item= args[i]->propagate_equal_fields(thd, Context_identity(), cond); } if (new_item && new_item != args[i]) thd->change_item_tree(&args[i], new_item); @@ -4066,7 +4070,6 @@ void Item_func_in::fix_length_and_dec() if (m_compare_type == STRING_RESULT && agg_arg_charsets_for_comparison(cmp_collation, args, arg_count)) return; - args[0]->cmp_context= m_compare_type; arg_types_compatible= TRUE; if (m_compare_type == ROW_RESULT) @@ -4198,16 +4201,6 @@ void Item_func_in::fix_length_and_dec() } } } - /* - Set cmp_context of all arguments. This prevents - Item_field::propagate_equal_fields() from transforming a zerofill integer - argument into a string constant. Such a change would require rebuilding - cmp_itmes. - */ - for (arg= args + 1, arg_end= args + arg_count; arg != arg_end ; arg++) - { - arg[0]->cmp_context= item_cmp_type(left_cmp_type, arg[0]); - } max_length= 1; } @@ -4675,7 +4668,7 @@ Item *Item_cond::propagate_equal_fields(THD *thd, is not important at this point. Item_func derivants will create and pass their own context to the arguments. */ - Item *new_item= item->propagate_equal_fields(thd, ANY_SUBST, cond); + Item *new_item= item->propagate_equal_fields(thd, Context_boolean(), cond); if (new_item && new_item != item) thd->change_item_tree(li.ref(), new_item); } @@ -5876,6 +5869,7 @@ Item_equal::Item_equal(THD *thd, Item *f1, Item *f2, bool with_const_item): equal_items.push_back(f1, thd->mem_root); equal_items.push_back(f2, thd->mem_root); compare_as_dates= with_const_item && f2->cmp_type() == TIME_RESULT; + cmp.set_compare_type(item_cmp_type(f1, f2)); upper_levels= NULL; } @@ -5905,6 +5899,7 @@ Item_equal::Item_equal(THD *thd, Item_equal *item_equal): } with_const= item_equal->with_const; compare_as_dates= item_equal->compare_as_dates; + cmp.set_compare_type(item_equal->cmp.compare_type()); cond_false= item_equal->cond_false; upper_levels= item_equal->upper_levels; } diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 45783c041bc..df0fc248f25 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -47,6 +47,7 @@ typedef int (*Item_field_cmpfunc)(Item *f1, Item *f2, void *arg); class Arg_comparator: public Sql_alloc { Item **a, **b; + Item_result m_compare_type; arg_cmp_func func; Item_func_or_sum *owner; bool set_null; // TRUE <=> set owner->null_value @@ -68,9 +69,11 @@ public: /* Allow owner function to use string buffers. */ String value1, value2; - Arg_comparator(): set_null(TRUE), comparators(0), thd(0), + Arg_comparator(): m_compare_type(IMPOSSIBLE_RESULT), + set_null(TRUE), comparators(0), thd(0), a_cache(0), b_cache(0) {}; - Arg_comparator(Item **a1, Item **a2): a(a1), b(a2), set_null(TRUE), + Arg_comparator(Item **a1, Item **a2): a(a1), b(a2), + m_compare_type(IMPOSSIBLE_RESULT), set_null(TRUE), comparators(0), thd(0), a_cache(0), b_cache(0) {}; private: @@ -78,23 +81,16 @@ private: Item **a1, Item **a2, Item_result type); public: + void set_compare_type(Item_result type) + { + m_compare_type= type; + } inline int set_cmp_func(Item_func_or_sum *owner_arg, Item **a1, Item **a2, bool set_null_arg) { set_null= set_null_arg; return set_cmp_func(owner_arg, a1, a2, item_cmp_type(*a1, *a2)); } - int set_cmp_func_and_arg_cmp_context(Item_func_or_sum *owner_arg, - Item **a1, Item **a2, - bool set_null_arg) - { - set_null= set_null_arg; - Item_result type= item_cmp_type(*a1, *a2); - int rc= set_cmp_func(owner_arg, a1, a2, type); - if (!rc) - (*a1)->cmp_context= (*a2)->cmp_context= type; - return rc; - } inline int compare() { return (this->*func)(); } int compare_string(); // compare args[0] & args[1] @@ -128,6 +124,7 @@ public: return (owner->type() == Item::FUNC_ITEM && ((Item_func*)owner)->functype() == Item_func::EQUAL_FUNC); } + Item_result compare_type() const { return m_compare_type; } void cleanup() { delete [] comparators; @@ -367,6 +364,20 @@ public: COND *remove_eq_conds(THD *thd, Item::cond_result *cond_value, bool top_level); bool count_sargable_conds(uchar *arg); + /* + Specifies which result type the function uses to compare its arguments. + This method is used in equal field propagation. + */ + virtual Item_result compare_type() const + { + /* + Have STRING_RESULT by default, which means the function compares + val_str() results of the arguments. This is suitable for Item_func_like + and for Item_func_spatial_rel. + Note, Item_bool_rowready_func2 overrides this default behaviour. + */ + return STRING_RESULT; + } }; class Item_bool_rowready_func2 :public Item_bool_func2 @@ -388,7 +399,9 @@ public: Item* propagate_equal_fields(THD *thd, const Context &ctx, COND_EQUAL *cond) { Item_args::propagate_equal_fields(thd, - Context(ANY_SUBST, compare_collation()), + Context(ANY_SUBST, + cmp.compare_type(), + compare_collation()), cond); return this; } @@ -397,16 +410,9 @@ public: { return cmp.set_cmp_func(this, tmp_arg, tmp_arg + 1, true); } - bool set_cmp_func_and_arg_cmp_context() - { - if (set_cmp_func()) - return true; - tmp_arg[0]->cmp_context= tmp_arg[1]->cmp_context= - item_cmp_type(tmp_arg[0], tmp_arg[1]); - return false; - } CHARSET_INFO *compare_collation() const { return cmp.cmp_collation.collation; } + Item_result compare_type() const { return cmp.compare_type(); } Arg_comparator *get_comparator() { return &cmp; } void cleanup() { @@ -439,14 +445,9 @@ public: Item *neg_transformer(THD *thd); Item* propagate_equal_fields(THD *thd, const Context &ctx, COND_EQUAL *cond) { - Item_args::propagate_equal_fields(thd, ANY_SUBST, cond); + Item_args::propagate_equal_fields(thd, Context_boolean(), cond); return this; } - void fix_length_and_dec() - { - Item_bool_func::fix_length_and_dec(); - args[0]->cmp_context= args[1]->cmp_context= INT_RESULT; - } }; class Item_func_not :public Item_bool_func @@ -760,6 +761,7 @@ public: { Item_args::propagate_equal_fields(thd, Context(ANY_SUBST, + m_compare_type, compare_collation()), cond); return this; @@ -933,6 +935,15 @@ public: 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); + return this; + } }; @@ -1420,16 +1431,16 @@ public: Item* propagate_equal_fields(THD *thd, const Context &ctx, COND_EQUAL *cond) { /* - In case when arg_types_compatible is false, - fix_length_and_dec() leaves args[0]->cmp_context equal to - IMPOSSIBLE_CONTEXT. In this case it's not safe to replace the field to an - equal constant, because Item_field::can_be_substituted_to_equal_item() - won't be able to check compatibility between - Item_equal->compare_collation() and this->compare_collation(). + TODO: enable propagation of the args[i>0] arguments even if + !arg_types_compatible. See MDEV-8755. + Note, we pass ANY_SUBST, this makes sure that non of the args + will be replaced to a zero-filled Item_string. + Such a change would require rebuilding of cmp_items. */ if (arg_types_compatible) Item_args::propagate_equal_fields(thd, Context(ANY_SUBST, + m_compare_type, compare_collation()), cond); return this; @@ -1691,7 +1702,8 @@ public: if ((flags & MY_CS_NOPAD) && !(flags & MY_CS_NON1TO1)) Item_args::propagate_equal_fields(thd, Context(ANY_SUBST, - compare_collation()), + STRING_RESULT, + compare_collation()), cond); return this; } @@ -1700,7 +1712,6 @@ public: void fix_length_and_dec() { max_length= 1; - args[0]->cmp_context= args[1]->cmp_context= STRING_RESULT; agg_arg_charsets_for_comparison(cmp_collation, args, 2); } void cleanup(); @@ -2091,6 +2102,7 @@ public: bool walk(Item_processor processor, bool walk_subquery, uchar *arg); Item *transform(THD *thd, Item_transformer transformer, uchar *arg); virtual void print(String *str, enum_query_type query_type); + Item_result compare_type() const { return cmp.compare_type(); } CHARSET_INFO *compare_collation() const; void set_context_field(Item_field *ctx_field) { context_field= ctx_field; } diff --git a/sql/item_func.h b/sql/item_func.h index d5ae2668cde..f8fd26d129c 100644 --- a/sql/item_func.h +++ b/sql/item_func.h @@ -348,7 +348,7 @@ public: are never equal is allowed in the arguments of a function. This is overruled for the direct arguments of comparison functions. */ - Item_args::propagate_equal_fields(thd, IDENTITY_SUBST, cond); + Item_args::propagate_equal_fields(thd, Context_identity(), cond); return this; } diff --git a/sql/sql_select.cc b/sql/sql_select.cc index b489756c784..3f2c2ead09c 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -12762,7 +12762,7 @@ static bool check_row_equality(THD *thd, Item *left_row, Item_row *right_row, { Item_func_eq *eq_item; if (!(eq_item= new (thd->mem_root) Item_func_eq(thd, left_item, right_item)) || - eq_item->set_cmp_func_and_arg_cmp_context()) + eq_item->set_cmp_func()) return FALSE; eq_item->quick_fix_field(); eq_list->push_back(eq_item, thd->mem_root); @@ -13096,7 +13096,7 @@ COND *Item_func::build_equal_items(THD *thd, COND_EQUAL *inherited, as soon the field is not of a string type or the field reference is an argument of a comparison predicate. */ - COND *cond= propagate_equal_fields(thd, ANY_SUBST, inherited); + COND *cond= propagate_equal_fields(thd, Context_boolean(), inherited); cond->update_used_tables(); DBUG_ASSERT(cond == this); DBUG_ASSERT(!cond_equal_ref || !cond_equal_ref[0]); @@ -13875,9 +13875,9 @@ can_change_cond_ref_to_const(Item_bool_func2 *target, { if (!target_expr->eq(source_expr,0) || target_value == source_const || - target_expr->cmp_context != source_expr->cmp_context) + target->compare_type() != source->compare_type()) return false; - if (target_expr->cmp_context == STRING_RESULT) + if (target->compare_type() == STRING_RESULT) { /* In this example: @@ -14921,7 +14921,8 @@ void propagate_new_equalities(THD *thd, Item *cond, } else { - cond= cond->propagate_equal_fields(thd, Item::ANY_SUBST, inherited); + cond= cond->propagate_equal_fields(thd, + Item::Context_boolean(), inherited); cond->update_used_tables(); } } |