From 7c87385e3075143de18e50c1d327eeb2e224603a Mon Sep 17 00:00:00 2001 From: Igor Babaev Date: Thu, 10 Oct 2013 10:08:26 -0700 Subject: [PATCH] Fixed bug mdev-5105. The bug caused a memory overwrite in the function update_ref_and_keys() It happened due to a wrong value of SELECT_LEX::cond_count. This value historically was calculated by the fix_fields method. Now the logic of calling this method became too complicated and, as a result, this value is calculated not always correctly. The patch changes the way how and when the values of SELECT_LEX::cond_count and of SELECT_LEX::between_count are calculated. The new code does it just at the beginning of update_ref_and_keys(). --- mysql-test/r/derived_view.result | 11 +++++++++++ mysql-test/t/derived_view.test | 14 ++++++++++++++ sql/item.h | 1 + sql/item_cmpfunc.cc | 25 ++++++++++++++++++++++++ sql/item_cmpfunc.h | 4 +++- sql/item_func.cc | 10 ++++++++++ sql/item_func.h | 5 ++++- sql/sql_base.cc | 1 - sql/sql_derived.cc | 2 -- sql/sql_select.cc | 33 ++++++++++++++++++++++++++------ 10 files changed, 95 insertions(+), 11 deletions(-) diff --git a/mysql-test/r/derived_view.result b/mysql-test/r/derived_view.result index eb2200c7720..0278c36b557 100644 --- a/mysql-test/r/derived_view.result +++ b/mysql-test/r/derived_view.result @@ -2223,6 +2223,17 @@ a (SELECT SUM(a + c) FROM (SELECT b as c FROM t2) AS v1) DROP VIEW v; DROP TABLE t1,t2; # +# mdev-5105: memory overwrite in multi-table update +# using natuaral join with a view +# +create table t1(a int,b tinyint,c tinyint)engine=myisam; +create table t2(a tinyint,b float,c int, d int, e int, f int, key (b), key(c), key(d), key(e), key(f))engine=myisam; +create table t3(a int,b int,c int, d int, e int, f int, key(a), key(b), key(c), key(d), key(e), key(f))engine=myisam; +create view v1 as select t2.b a, t1.b b, t2.c c, t2.d d, t2.e e, t2.f f from t1,t2 where t1.a=t2.a; +update t3 natural join v1 set a:=1; +drop view v1; +drop table t1,t2,t3; +# # end of 5.3 tests # set optimizer_switch=@exit_optimizer_switch; diff --git a/mysql-test/t/derived_view.test b/mysql-test/t/derived_view.test index 254f97ebe6a..8d1b3109d20 100644 --- a/mysql-test/t/derived_view.test +++ b/mysql-test/t/derived_view.test @@ -1564,6 +1564,20 @@ SELECT a, (SELECT SUM(a + c) FROM (SELECT b as c FROM t2) AS v1) FROM t1; DROP VIEW v; DROP TABLE t1,t2; +--echo # +--echo # mdev-5105: memory overwrite in multi-table update +--echo # using natuaral join with a view +--echo # + +create table t1(a int,b tinyint,c tinyint)engine=myisam; +create table t2(a tinyint,b float,c int, d int, e int, f int, key (b), key(c), key(d), key(e), key(f))engine=myisam; +create table t3(a int,b int,c int, d int, e int, f int, key(a), key(b), key(c), key(d), key(e), key(f))engine=myisam; +create view v1 as select t2.b a, t1.b b, t2.c c, t2.d d, t2.e e, t2.f f from t1,t2 where t1.a=t2.a; + +update t3 natural join v1 set a:=1; +drop view v1; +drop table t1,t2,t3; + --echo # --echo # end of 5.3 tests --echo # diff --git a/sql/item.h b/sql/item.h index fd19180f3be..4f03d588bbc 100644 --- a/sql/item.h +++ b/sql/item.h @@ -1054,6 +1054,7 @@ public: virtual bool view_used_tables_processor(uchar *arg) { return 0; } virtual bool eval_not_null_tables(uchar *opt_arg) { return 0; } virtual bool is_subquery_processor (uchar *opt_arg) { return 0; } + virtual bool count_sargable_conds(uchar *arg) { return 0; } virtual bool limit_index_condition_pushdown_processor(uchar *opt_arg) { return FALSE; diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index a6587fd4c3d..514b0fe4580 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -539,6 +539,8 @@ void Item_bool_func2::fix_length_and_dec() to the collation of A. */ + sargable= true; + DTCollation coll; if (args[0]->result_type() == STRING_RESULT && args[1]->result_type() == STRING_RESULT && @@ -1433,6 +1435,7 @@ bool Item_in_optimizer::eval_not_null_tables(uchar *opt_arg) return FALSE; } + bool Item_in_optimizer::fix_left(THD *thd, Item **ref) { if ((!args[0]->fixed && args[0]->fix_fields(thd, args)) || @@ -2160,6 +2163,15 @@ bool Item_func_between::eval_not_null_tables(uchar *opt_arg) } +bool Item_func_between::count_sargable_conds(uchar *arg) +{ + SELECT_LEX *sel= (SELECT_LEX *) arg; + sel->cond_count++; + sel->between_count++; + return 0; +} + + void Item_func_between::fix_after_pullout(st_select_lex *new_parent, Item **ref) { /* This will re-calculate attributes of the arguments */ @@ -2173,6 +2185,7 @@ void Item_func_between::fix_length_and_dec() THD *thd= current_thd; max_length= 1; compare_as_dates= 0; + sargable= true; /* As some compare functions are generated after sql_yacc, @@ -3852,6 +3865,7 @@ void Item_func_in::fix_length_and_dec() uint found_types= 0; uint type_cnt= 0, i; Item_result cmp_type= STRING_RESULT; + sargable= true; left_result_type= args[0]->cmp_type(); if (!(found_types= collect_cmp_types(args, arg_count, true))) return; @@ -4638,6 +4652,7 @@ longlong Item_func_isnull::val_int() return args[0]->is_null() ? 1: 0; } + longlong Item_is_not_null_test::val_int() { DBUG_ASSERT(fixed == 1); @@ -4841,6 +4856,7 @@ bool Item_func_like::fix_fields(THD *thd, Item **ref) return FALSE; } + void Item_func_like::cleanup() { canDoTurboBM= FALSE; @@ -5868,6 +5884,14 @@ void Item_equal::update_used_tables() } +bool Item_equal::count_sargable_conds(uchar *arg) +{ + SELECT_LEX *sel= (SELECT_LEX *) arg; + uint m= equal_items.elements; + sel->cond_count+= m*(m-1); + return 0; +} + /** @brief @@ -5920,6 +5944,7 @@ void Item_equal::fix_length_and_dec() Item *item= get_first(NO_PARTICULAR_TAB, NULL); eval_item= cmp_item::get_comparator(item->cmp_type(), item, item->collation.collation); + sargable= true; } diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 99d89e16532..4deda7679ac 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -118,7 +118,7 @@ public: Item_bool_func(Item *a,Item *b) :Item_int_func(a,b) {} Item_bool_func(THD *thd, Item_bool_func *item) :Item_int_func(thd, item) {} bool is_bool_func() { return 1; } - void fix_length_and_dec() { decimals=0; max_length=1; } + void fix_length_and_dec() { decimals=0; max_length=1; sargable= true;} uint decimal_precision() const { return 1; } }; @@ -681,6 +681,7 @@ public: uint decimal_precision() const { return 1; } bool eval_not_null_tables(uchar *opt_arg); void fix_after_pullout(st_select_lex *new_parent, Item **ref); + bool count_sargable_conds(uchar *arg); }; @@ -1747,6 +1748,7 @@ public: CHARSET_INFO *compare_collation(); void set_context_field(Item_field *ctx_field) { context_field= ctx_field; } + bool count_sargable_conds(uchar *arg); friend class Item_equal_iterator; friend class Item_equal_iterator; friend Item *eliminate_item_equal(COND *cond, COND_EQUAL *upper_levels, diff --git a/sql/item_func.cc b/sql/item_func.cc index c729893dae0..9416d05cb2f 100644 --- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -733,6 +733,16 @@ double Item_int_func::val_real() return unsigned_flag ? (double) ((ulonglong) val_int()) : (double) val_int(); } +bool Item_int_func::count_sargable_conds(uchar *arg) +{ + if (sargable) + { + SELECT_LEX *sel= (SELECT_LEX *) arg; + sel->cond_count++; + } + return 0; +} + String *Item_int_func::val_str(String *str) { diff --git a/sql/item_func.h b/sql/item_func.h index 80b794f30d5..310b312d1e4 100644 --- a/sql/item_func.h +++ b/sql/item_func.h @@ -485,6 +485,8 @@ class Item_num_op :public Item_func_numhybrid class Item_int_func :public Item_func { +protected: + bool sargable; public: Item_int_func() :Item_func() { max_length= 21; } Item_int_func(Item *a) :Item_func(a) { max_length= 21; } @@ -496,7 +498,8 @@ public: double val_real(); String *val_str(String*str); enum Item_result result_type () const { return INT_RESULT; } - void fix_length_and_dec() {} + void fix_length_and_dec() { sargable= false; } + bool count_sargable_conds(uchar *arg); }; diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 22dc09351c7..130763de76a 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -8549,7 +8549,6 @@ int setup_conds(THD *thd, TABLE_LIST *tables, List &leaves, embedded->on_expr->fix_fields(thd, &embedded->on_expr)) || embedded->on_expr->check_cols(1)) goto err_no_arena; - select_lex->cond_count++; } /* If it's a semi-join nest, fix its "left expression", as it is used by diff --git a/sql/sql_derived.cc b/sql/sql_derived.cc index a8bfc8a11a7..db6ab1fb269 100644 --- a/sql/sql_derived.cc +++ b/sql/sql_derived.cc @@ -394,8 +394,6 @@ bool mysql_derived_merge(THD *thd, LEX *lex, TABLE_LIST *derived) if (dt_select->options & OPTION_SCHEMA_TABLE) parent_lex->options |= OPTION_SCHEMA_TABLE; - parent_lex->cond_count+= dt_select->cond_count; - if (!derived->get_unit()->prepared) { dt_select->leaf_tables.empty(); diff --git a/sql/sql_select.cc b/sql/sql_select.cc index bca53bef878..072f10a4d08 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -4733,6 +4733,32 @@ update_ref_and_keys(THD *thd, DYNAMIC_ARRAY *keyuse,JOIN_TAB *join_tab, KEY_FIELD *key_fields, *end, *field; uint sz; uint m= max(select_lex->max_equal_elems,1); + + SELECT_LEX *sel=thd->lex->current_select; + sel->cond_count= 0; + sel->between_count= 0; + if (cond) + cond->walk(&Item::count_sargable_conds, 0, (uchar*) sel); + for (i=0 ; i < tables ; i++) + { + if (*join_tab[i].on_expr_ref) + (*join_tab[i].on_expr_ref)->walk(&Item::count_sargable_conds, + 0, (uchar*) sel); + } + { + List_iterator li(*join_tab->join->join_list); + TABLE_LIST *table; + while ((table= li++)) + { + if (table->nested_join) + { + if (table->on_expr) + table->on_expr->walk(&Item::count_sargable_conds, 0, (uchar*) sel); + if (table->sj_on_expr) + table->sj_on_expr->walk(&Item::count_sargable_conds, 0, (uchar*) sel); + } + } + } /* We use the same piece of memory to store both KEY_FIELD @@ -4756,8 +4782,7 @@ update_ref_and_keys(THD *thd, DYNAMIC_ARRAY *keyuse,JOIN_TAB *join_tab, substitutions. */ sz= max(sizeof(KEY_FIELD),sizeof(SARGABLE_PARAM))* - (((thd->lex->current_select->cond_count+1)*2 + - thd->lex->current_select->between_count)*m+1); + ((sel->cond_count*2 + sel->between_count)*m+1); if (!(key_fields=(KEY_FIELD*) thd->alloc(sz))) return TRUE; /* purecov: inspected */ and_level= 0; @@ -11219,13 +11244,10 @@ static bool check_row_equality(THD *thd, Item *left_row, Item_row *right_row, (Item_row *) left_item, (Item_row *) right_item, cond_equal, eq_list); - if (!is_converted) - thd->lex->current_select->cond_count++; } else { is_converted= check_simple_equality(left_item, right_item, 0, cond_equal); - thd->lex->current_select->cond_count++; } if (!is_converted) @@ -11284,7 +11306,6 @@ static bool check_equality(THD *thd, Item *item, COND_EQUAL *cond_equal, if (left_item->type() == Item::ROW_ITEM && right_item->type() == Item::ROW_ITEM) { - thd->lex->current_select->cond_count--; return check_row_equality(thd, (Item_row *) left_item, (Item_row *) right_item,