From 060d4861b95d02bc8b971b9d62103d019c37799e Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Fri, 30 Dec 2016 21:13:34 +0400 Subject: [PATCH 1/3] Fixing minor problems in the patch for MDEV-11478 (shortint vs smallint) - Fixing Type_handler_short::name() to return "smallint" instead of "shortint". - Fixing test markers "End of 10.2 tests" and "End of 10.3 test" --- mysql-test/r/gis.result | 15 +++++++++------ mysql-test/t/gis.test | 6 +++++- sql/sql_type.cc | 2 +- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/mysql-test/r/gis.result b/mysql-test/r/gis.result index 7f620e67b0e70..601590c88c153 100644 --- a/mysql-test/r/gis.result +++ b/mysql-test/r/gis.result @@ -2199,8 +2199,11 @@ c 1 DROP TABLE t1; # +# Start of 10.3 tests # # +# MDEV-11478 Result data type aggregation for pluggable data types +# CREATE PROCEDURE p2(query TEXT) BEGIN DECLARE errcount INT DEFAULT 0; @@ -2451,23 +2454,23 @@ CREATE TABLE t1 (a SMALLINT, b Point) CREATE TABLE t2 AS SELECT CASE WHEN TRUE THEN a ELSE b END FROM t1 ERROR: -Illegal parameter data types shortint and geometry for operation 'case' +Illegal parameter data types smallint and geometry for operation 'case' CREATE TABLE t2 AS SELECT COALESCE(a,b) FROM t1 ERROR: -Illegal parameter data types shortint and geometry for operation 'coalesce' +Illegal parameter data types smallint and geometry for operation 'coalesce' CREATE TABLE t2 AS SELECT IF(TRUE,a,b) FROM t1 ERROR: -Illegal parameter data types shortint and geometry for operation 'if' +Illegal parameter data types smallint and geometry for operation 'if' CREATE TABLE t2 AS SELECT IFNULL(a,b) FROM t1 ERROR: -Illegal parameter data types shortint and geometry for operation 'ifnull' +Illegal parameter data types smallint and geometry for operation 'ifnull' CREATE TABLE t2 AS SELECT a FROM t1 UNION SELECT b FROM t1 ERROR: -Illegal parameter data types shortint and geometry for operation 'UNION' +Illegal parameter data types smallint and geometry for operation 'UNION' ------------------------------------- CREATE TABLE t1 (a MEDIUMINT, b Point) @@ -2811,5 +2814,5 @@ DROP TABLE t1; DROP PROCEDURE p1; DROP PROCEDURE p2; # -# End of 10.2 tests +# End of 10.3 tests # diff --git a/mysql-test/t/gis.test b/mysql-test/t/gis.test index b11fe18ec6df5..282005da62413 100644 --- a/mysql-test/t/gis.test +++ b/mysql-test/t/gis.test @@ -1717,7 +1717,11 @@ SELECT c FROM t1; DROP TABLE t1; --echo # +--echo # Start of 10.3 tests --echo # + +--echo # +--echo # MDEV-11478 Result data type aggregation for pluggable data types --echo # DELIMITER $$; @@ -1796,5 +1800,5 @@ DROP PROCEDURE p2; --echo # ---echo # End of 10.2 tests +--echo # End of 10.3 tests --echo # diff --git a/sql/sql_type.cc b/sql/sql_type.cc index 6f20a6c17b6ef..acd8cf99b0fd8 100644 --- a/sql/sql_type.cc +++ b/sql/sql_type.cc @@ -200,7 +200,7 @@ const Name const Name Type_handler_tiny::m_name_tiny(C_STRING_WITH_LEN("tinyint")), - Type_handler_short::m_name_short(C_STRING_WITH_LEN("shortint")), + Type_handler_short::m_name_short(C_STRING_WITH_LEN("smallint")), Type_handler_long::m_name_int(C_STRING_WITH_LEN("int")), Type_handler_longlong::m_name_longlong(C_STRING_WITH_LEN("bigint")), Type_handler_int24::m_name_mediumint(C_STRING_WITH_LEN("mediumint")), From 306ce497a7b541d301b56fe39eae53aed1bff259 Mon Sep 17 00:00:00 2001 From: Oleksandr Byelkin Date: Tue, 3 Jan 2017 14:26:04 +0100 Subject: [PATCH 2/3] Fixed typo in comments --- storage/heap/hp_block.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/storage/heap/hp_block.c b/storage/heap/hp_block.c index aa5343a071769..6ecab0d08c4d1 100644 --- a/storage/heap/hp_block.c +++ b/storage/heap/hp_block.c @@ -71,9 +71,9 @@ int hp_get_new_block(HP_SHARE *info, HP_BLOCK *block, size_t *alloc_length) lower levels. For example, for level 0, we allocate data for X rows. - When level 0 is full, we allocate data for HPTRS_IN_NODE + X rows. + When level 0 is full, we allocate data for HP_PTRS_IN_NOD + X rows. Next time we allocate data for X rows. - When level 1 is full, we allocate data for HPTRS_IN_NODE at level 2 and 1 + When level 1 is full, we allocate data for HP_PTRS_IN_NOD at level 2 and 1 + X rows at level 0. */ *alloc_length= (sizeof(HP_PTRS) * ((i == block->levels) ? i : i - 1) + From 59c58f6ec3c1dcba9b396fed45f6dc206c7ffb3e Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Fri, 13 Jan 2017 17:25:16 +0400 Subject: [PATCH 3/3] 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 119193165000c..7d15fc0a1c045 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -392,7 +392,7 @@ class Item_bool_func2 :public Item_bool_func 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 @@ class Item_bool_func2 :public Item_bool_func 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 @@ class Item_bool_rowready_func2 :public Item_bool_func2_with_rev 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 2889e1e1549f2..310f0673156f2 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 acd8cf99b0fd8..d43dd2c2c597e 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 b9c2dd096dd23..29e1a2946bb47 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 @@ class Type_handler 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 @@ class Type_handler_row: public Type_handler 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 @@ class Type_handler_numeric: public Type_handler 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 @@ class Type_handler_temporal_result: public Type_handler 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 @@ class Type_handler_string_result: public Type_handler 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,