From 59c58f6ec3c1dcba9b396fed45f6dc206c7ffb3e Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Fri, 13 Jan 2017 17:25:16 +0400 Subject: [PATCH] MDEV-9522 Split sql_select.cc:can_change_cond_ref_to_const into virtual methods in Type_handler --- sql/item_cmpfunc.h | 9 +++-- sql/sql_select.cc | 72 +++-------------------------------- sql/sql_type.cc | 93 ++++++++++++++++++++++++++++++++++++++++++++++ sql/sql_type.h | 46 +++++++++++++++++++++++ 4 files changed, 151 insertions(+), 69 deletions(-) diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 11919316500..7d15fc0a1c0 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -392,7 +392,7 @@ public: 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 + virtual const Type_handler *compare_type_handler() const { /* Have STRING_RESULT by default, which means the function compares @@ -400,7 +400,7 @@ public: and for Item_func_spatial_rel. Note, Item_bool_rowready_func2 overrides this default behaviour. */ - return STRING_RESULT; + return &type_handler_varchar; } SEL_TREE *get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr) { @@ -508,7 +508,10 @@ public: return cmp.set_cmp_func(this, tmp_arg, tmp_arg + 1, true); } CHARSET_INFO *compare_collation() const { return cmp.compare_collation(); } - Item_result compare_type() const { return cmp.compare_type(); } + const Type_handler *compare_type_handler() const + { + return cmp.compare_type_handler(); + } Arg_comparator *get_comparator() { return &cmp; } void cleanup() { diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 2889e1e1549..310f0673156 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -12927,7 +12927,7 @@ bool Item_func_eq::check_equality(THD *thd, COND_EQUAL *cond_equal, } return check_simple_equality(thd, Context(ANY_SUBST, - compare_type(), + compare_type_handler()->cmp_type(), compare_collation()), left_item, right_item, cond_equal); } @@ -13995,71 +13995,11 @@ can_change_cond_ref_to_const(Item_bool_func2 *target, Item_bool_func2 *source, Item *source_expr, Item *source_const) { - if (!target_expr->eq(source_expr,0) || - target_value == source_const || - target->compare_type() != source->compare_type()) - return false; - if (target->compare_type() == STRING_RESULT) - { - /* - In this example: - SET NAMES utf8 COLLATE utf8_german2_ci; - DROP TABLE IF EXISTS t1; - CREATE TABLE t1 (a CHAR(10) CHARACTER SET utf8); - INSERT INTO t1 VALUES ('o-umlaut'),('oe'); - SELECT * FROM t1 WHERE a='oe' COLLATE utf8_german2_ci AND a='oe'; - - the query should return only the row with 'oe'. - It should not return 'o-umlaut', because 'o-umlaut' does not match - the right part of the condition: a='oe' - ('o-umlaut' is not equal to 'oe' in utf8_general_ci, - which is the collation of the field "a"). - - If we change the right part from: - ... AND a='oe' - to - ... AND 'oe' COLLATE utf8_german2_ci='oe' - it will be evalulated to TRUE and removed from the condition, - so the overall query will be simplified to: - - SELECT * FROM t1 WHERE a='oe' COLLATE utf8_german2_ci; - - which will erroneously start to return both 'oe' and 'o-umlaut'. - So changing "expr" to "const" is not possible if the effective - collations of "target" and "source" are not exactly the same. - - Note, the code before the fix for MDEV-7152 only checked that - collations of "source_const" and "target_value" are the same. - This was not enough, as the bug report demonstrated. - */ - return - target->compare_collation() == source->compare_collation() && - target_value->collation.collation == source_const->collation.collation; - } - if (target->compare_type() == TIME_RESULT) - { - if (target_value->cmp_type() != TIME_RESULT) - { - /* - Can't rewrite: - WHERE COALESCE(time_column)='00:00:00' - AND COALESCE(time_column)=DATE'2015-09-11' - to - WHERE DATE'2015-09-11'='00:00:00' - AND COALESCE(time_column)=DATE'2015-09-11' - because the left part will erroneously try to parse '00:00:00' - as DATE, not as TIME. - - TODO: It could still be rewritten to: - WHERE DATE'2015-09-11'=TIME'00:00:00' - AND COALESCE(time_column)=DATE'2015-09-11' - i.e. we need to replace both target_expr and target_value - at the same time. This is not supported yet. - */ - return false; - } - } - return true; // Non-string comparison + return target_expr->eq(source_expr,0) && + target_value != source_const && + target->compare_type_handler()-> + can_change_cond_ref_to_const(target, target_expr, target_value, + source, source_expr, source_const); } diff --git a/sql/sql_type.cc b/sql/sql_type.cc index acd8cf99b0f..d43dd2c2c59 100644 --- a/sql/sql_type.cc +++ b/sql/sql_type.cc @@ -1057,6 +1057,99 @@ bool Type_handler_temporal_result::set_comparator_func(Arg_comparator *cmp) cons } +/*************************************************************************/ + +bool Type_handler_temporal_result:: + can_change_cond_ref_to_const(Item_bool_func2 *target, + Item *target_expr, Item *target_value, + Item_bool_func2 *source, + Item *source_expr, Item *source_const) + const +{ + if (source->compare_type_handler()->cmp_type() != TIME_RESULT) + return false; + + /* + Can't rewrite: + WHERE COALESCE(time_column)='00:00:00' + AND COALESCE(time_column)=DATE'2015-09-11' + to + WHERE DATE'2015-09-11'='00:00:00' + AND COALESCE(time_column)=DATE'2015-09-11' + because the left part will erroneously try to parse '00:00:00' + as DATE, not as TIME. + + TODO: It could still be rewritten to: + WHERE DATE'2015-09-11'=TIME'00:00:00' + AND COALESCE(time_column)=DATE'2015-09-11' + i.e. we need to replace both target_expr and target_value + at the same time. This is not supported yet. + */ + return target_value->cmp_type() == TIME_RESULT; +} + + +bool Type_handler_string_result:: + can_change_cond_ref_to_const(Item_bool_func2 *target, + Item *target_expr, Item *target_value, + Item_bool_func2 *source, + Item *source_expr, Item *source_const) + const +{ + if (source->compare_type_handler()->cmp_type() != STRING_RESULT) + return false; + /* + In this example: + SET NAMES utf8 COLLATE utf8_german2_ci; + DROP TABLE IF EXISTS t1; + CREATE TABLE t1 (a CHAR(10) CHARACTER SET utf8); + INSERT INTO t1 VALUES ('o-umlaut'),('oe'); + SELECT * FROM t1 WHERE a='oe' COLLATE utf8_german2_ci AND a='oe'; + + the query should return only the row with 'oe'. + It should not return 'o-umlaut', because 'o-umlaut' does not match + the right part of the condition: a='oe' + ('o-umlaut' is not equal to 'oe' in utf8_general_ci, + which is the collation of the field "a"). + + If we change the right part from: + ... AND a='oe' + to + ... AND 'oe' COLLATE utf8_german2_ci='oe' + it will be evalulated to TRUE and removed from the condition, + so the overall query will be simplified to: + + SELECT * FROM t1 WHERE a='oe' COLLATE utf8_german2_ci; + + which will erroneously start to return both 'oe' and 'o-umlaut'. + So changing "expr" to "const" is not possible if the effective + collations of "target" and "source" are not exactly the same. + + Note, the code before the fix for MDEV-7152 only checked that + collations of "source_const" and "target_value" are the same. + This was not enough, as the bug report demonstrated. + */ + return + target->compare_collation() == source->compare_collation() && + target_value->collation.collation == source_const->collation.collation; +} + + +bool Type_handler_numeric:: + can_change_cond_ref_to_const(Item_bool_func2 *target, + Item *target_expr, Item *target_value, + Item_bool_func2 *source, + Item *source_expr, Item *source_const) + const +{ + /* + The collations of "target" and "source" do not make sense for numeric + data types. + */ + return target->compare_type_handler() == source->compare_type_handler(); +} + + /*************************************************************************/ Item_cache * diff --git a/sql/sql_type.h b/sql/sql_type.h index b9c2dd096dd..29e1a2946bb 100644 --- a/sql/sql_type.h +++ b/sql/sql_type.h @@ -32,6 +32,7 @@ class Item_func_hex; class Item_hybrid_func; class Item_func_min_max; class Item_func_hybrid_field_type; +class Item_bool_func2; class Item_func_between; class Item_func_in; class cmp_item; @@ -346,6 +347,31 @@ public: virtual uint32 max_display_length(const Item *item) const= 0; virtual int Item_save_in_field(Item *item, Field *field, bool no_conversions) const= 0; + /** + Check if + WHERE expr=value AND expr=const + can be rewritten as: + WHERE const=value AND expr=const + + "this" is the comparison handler that is used by "target". + + @param target - the predicate expr=value, + whose "expr" argument will be replaced to "const". + @param target_expr - the target's "expr" which will be replaced to "const". + @param target_value - the target's second argument, it will remain unchanged. + @param source - the equality predicate expr=const (or expr<=>const) + that can be used to rewrite the "target" part + (under certain conditions, see the code). + @param source_expr - the source's "expr". It should be exactly equal to + the target's "expr" to make condition rewrite possible. + @param source_const - the source's "const" argument, it will be inserted + into "target" instead of "expr". + */ + virtual bool + can_change_cond_ref_to_const(Item_bool_func2 *target, + Item *target_expr, Item *target_value, + Item_bool_func2 *source, + Item *source_expr, Item *source_const) const= 0; virtual Item_cache *Item_get_cache(THD *thd, const Item *item) const= 0; virtual bool set_comparator_func(Arg_comparator *cmp) const= 0; virtual bool Item_hybrid_func_fix_attributes(THD *thd, Item_hybrid_func *func, @@ -455,6 +481,14 @@ public: DBUG_ASSERT(0); return 1; } + bool can_change_cond_ref_to_const(Item_bool_func2 *target, + Item *target_expr, Item *target_value, + Item_bool_func2 *source, + Item *source_expr, Item *source_const) const + { + DBUG_ASSERT(0); + return false; + } Item_cache *Item_get_cache(THD *thd, const Item *item) const; bool set_comparator_func(Arg_comparator *cmp) const; bool Item_hybrid_func_fix_attributes(THD *thd, Item_hybrid_func *func, @@ -558,6 +592,10 @@ public: bool Item_func_min_max_get_date(Item_func_min_max*, MYSQL_TIME *, ulonglong fuzzydate) const; virtual ~Type_handler_numeric() { } + bool can_change_cond_ref_to_const(Item_bool_func2 *target, + Item *target_expr, Item *target_value, + Item_bool_func2 *source, + Item *source_expr, Item *source_const) const; }; @@ -699,6 +737,10 @@ public: const Type_std_attributes *item, SORT_FIELD_ATTR *attr) const; uint32 max_display_length(const Item *item) const; + bool can_change_cond_ref_to_const(Item_bool_func2 *target, + Item *target_expr, Item *target_value, + Item_bool_func2 *source, + Item *source_expr, Item *source_const) const; Item_cache *Item_get_cache(THD *thd, const Item *item) const; bool set_comparator_func(Arg_comparator *cmp) const; bool Item_sum_hybrid_fix_length_and_dec(Item_sum_hybrid *func) const; @@ -746,6 +788,10 @@ public: SORT_FIELD_ATTR *attr) const; uint32 max_display_length(const Item *item) const; int Item_save_in_field(Item *item, Field *field, bool no_conversions) const; + bool can_change_cond_ref_to_const(Item_bool_func2 *target, + Item *target_expr, Item *target_value, + Item_bool_func2 *source, + Item *source_expr, Item *source_const) const; Item_cache *Item_get_cache(THD *thd, const Item *item) const; bool set_comparator_func(Arg_comparator *cmp) const; bool Item_hybrid_func_fix_attributes(THD *thd, Item_hybrid_func *func,