From 102a85f9f30cdf8c3baa3893c68932617240bfa6 Mon Sep 17 00:00:00 2001 From: Oleksandr Byelkin Date: Thu, 3 Sep 2015 18:00:43 +0200 Subject: MDEV-8663: IF Statement returns multiple values erroneously (or Assertion `!null_value' failed in Item::send(Protocol*, String*)) Postreview addons by Bar Fix: keeping contract: NULL value mean NULL pointer in val_str and val_deciman. --- sql/item_cmpfunc.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'sql/item_cmpfunc.cc') diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 998cb1cbd64..4f07318d1fd 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -2692,7 +2692,8 @@ Item_func_if::str_op(String *str) String *res=arg->val_str(str); if (res) res->set_charset(collation.collation); - null_value=arg->null_value; + if (null_value=arg->null_value) + res= NULL; return res; } @@ -2703,7 +2704,8 @@ Item_func_if::decimal_op(my_decimal *decimal_value) DBUG_ASSERT(fixed == 1); Item *arg= args[0]->val_bool() ? args[1] : args[2]; my_decimal *value= arg->val_decimal(decimal_value); - null_value= arg->null_value; + if (null_value= arg->null_value) + value= NULL; return value; } -- cgit v1.2.1 From 5cc149febaad181cac65903a62dfe507ae4b6f76 Mon Sep 17 00:00:00 2001 From: Oleksandr Byelkin Date: Thu, 24 Sep 2015 10:28:47 +0200 Subject: The compiler warnings fixed. --- sql/item_cmpfunc.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'sql/item_cmpfunc.cc') diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 4f07318d1fd..1d4771ae133 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -2692,7 +2692,7 @@ Item_func_if::str_op(String *str) String *res=arg->val_str(str); if (res) res->set_charset(collation.collation); - if (null_value=arg->null_value) + if ((null_value=arg->null_value)) res= NULL; return res; } @@ -2704,7 +2704,7 @@ Item_func_if::decimal_op(my_decimal *decimal_value) DBUG_ASSERT(fixed == 1); Item *arg= args[0]->val_bool() ? args[1] : args[2]; my_decimal *value= arg->val_decimal(decimal_value); - if (null_value= arg->null_value) + if ((null_value= arg->null_value)) value= NULL; return value; } -- cgit v1.2.1 From 2e3e8180483628e6744b6590acf5dd546c6a6076 Mon Sep 17 00:00:00 2001 From: Oleksandr Byelkin Date: Thu, 23 Apr 2015 19:11:06 +0200 Subject: MDEV-7445: Server crash with Signal 6 Problem was in rewriting left expression which had 2 references on it. Solved with making subselect reference main. Item_in_optimized can have not Item_in_subselect reference in left part so type casting with no check is dangerous. Item::cols() should be checked after Item::fix_fields(). --- sql/item_cmpfunc.cc | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) (limited to 'sql/item_cmpfunc.cc') diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 1d4771ae133..678288971ad 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -1442,9 +1442,25 @@ bool Item_in_optimizer::eval_not_null_tables(uchar *opt_arg) bool Item_in_optimizer::fix_left(THD *thd, Item **ref) { DBUG_ENTER("Item_in_optimizer::fix_left"); - if ((!args[0]->fixed && args[0]->fix_fields(thd, args)) || - (!cache && !(cache= Item_cache::get_cache(args[0])))) + Item **ref0= args; + if (args[1]->type() == Item::SUBSELECT_ITEM && + ((Item_subselect *)args[1])->is_in_predicate()) + { + /* + left_expr->fix_fields() may cause left_expr to be substituted for + another item. (e.g. an Item_field may be changed into Item_ref). This + transformation is undone at the end of statement execution (e.g. the + Item_ref is deleted). However, Item_in_optimizer::args[0] may keep + the pointer to the post-transformation item. Because of that, on the + next execution we need to copy args[1]->left_expr again. + */ + ref0= &(((Item_in_subselect *)args[1])->left_expr); + args[0]= ref0[0]; + } + if ((!args[0]->fixed && args[0]->fix_fields(thd, ref0)) || + (!cache && !(cache= Item_cache::get_cache(ref0[0])))) DBUG_RETURN(1); + args[0]= ref0[0]; DBUG_PRINT("info", ("actual fix fields")); cache->setup(args[0]); @@ -1500,6 +1516,16 @@ bool Item_in_optimizer::fix_left(THD *thd, Item **ref) bool Item_in_optimizer::fix_fields(THD *thd, Item **ref) { DBUG_ASSERT(fixed == 0); + Item_subselect *sub= 0; + uint col; + + /* + MAX/MIN optimization can convert the subquery into + expr + Item_singlerow_subselect + */ + if (args[1]->type() == Item::SUBSELECT_ITEM) + sub= (Item_subselect *)args[1]; + if (fix_left(thd, ref)) return TRUE; if (args[0]->maybe_null) @@ -1507,10 +1533,10 @@ bool Item_in_optimizer::fix_fields(THD *thd, Item **ref) if (!args[1]->fixed && args[1]->fix_fields(thd, args+1)) return TRUE; - Item_in_subselect * sub= (Item_in_subselect *)args[1]; - if (args[0]->cols() != sub->engine->cols()) + if ((sub && ((col= args[0]->cols()) != sub->engine->cols())) || + (!sub && (args[1]->cols() != (col= 1)))) { - my_error(ER_OPERAND_COLUMNS, MYF(0), args[0]->cols()); + my_error(ER_OPERAND_COLUMNS, MYF(0), col); return TRUE; } if (args[1]->maybe_null) -- cgit v1.2.1 From 54b998173b128bb8362b5dbafbd66c4199776937 Mon Sep 17 00:00:00 2001 From: Oleksandr Byelkin Date: Thu, 23 Apr 2015 20:08:57 +0200 Subject: MDEV-7846: Server crashes in Item_subselect::fix_fields or fails with Thread stack overrun Substitute into transformed subselects original left expression and than register its change in case it was substituted. --- sql/item_cmpfunc.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'sql/item_cmpfunc.cc') diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 678288971ad..c2adb58677d 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -1455,12 +1455,12 @@ bool Item_in_optimizer::fix_left(THD *thd, Item **ref) next execution we need to copy args[1]->left_expr again. */ ref0= &(((Item_in_subselect *)args[1])->left_expr); - args[0]= ref0[0]; } - if ((!args[0]->fixed && args[0]->fix_fields(thd, ref0)) || - (!cache && !(cache= Item_cache::get_cache(ref0[0])))) + if ((!(*ref0)->fixed && (*ref0)->fix_fields(thd, ref0)) || + (!cache && !(cache= Item_cache::get_cache(*ref0)))) DBUG_RETURN(1); - args[0]= ref0[0]; + if (args[0] != (*ref0)) + current_thd->change_item_tree(args, (*ref0)); DBUG_PRINT("info", ("actual fix fields")); cache->setup(args[0]); -- cgit v1.2.1 From 504802f333d3ba2a7385ca14f5b40a3facc72de8 Mon Sep 17 00:00:00 2001 From: Oleksandr Byelkin Date: Wed, 5 Aug 2015 11:57:35 +0200 Subject: MDEV-7846: postreview fix --- sql/item_cmpfunc.cc | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'sql/item_cmpfunc.cc') diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index c2adb58677d..0c48592eb9f 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -1442,6 +1442,12 @@ bool Item_in_optimizer::eval_not_null_tables(uchar *opt_arg) bool Item_in_optimizer::fix_left(THD *thd, Item **ref) { DBUG_ENTER("Item_in_optimizer::fix_left"); + /* + Here we will store pointer on place of main storage of left expression. + For usual IN (ALL/ANY) it is subquery left_expr. + For other cases (MAX/MIN optimization, non-transformed EXISTS (10.0)) + it is args[0]. + */ Item **ref0= args; if (args[1]->type() == Item::SUBSELECT_ITEM && ((Item_subselect *)args[1])->is_in_predicate()) @@ -1455,12 +1461,17 @@ bool Item_in_optimizer::fix_left(THD *thd, Item **ref) next execution we need to copy args[1]->left_expr again. */ ref0= &(((Item_in_subselect *)args[1])->left_expr); + args[0]= ((Item_in_subselect *)args[1])->left_expr; } if ((!(*ref0)->fixed && (*ref0)->fix_fields(thd, ref0)) || (!cache && !(cache= Item_cache::get_cache(*ref0)))) DBUG_RETURN(1); + /* + During fix_field() expression could be substituted. + So we copy changes before use + */ if (args[0] != (*ref0)) - current_thd->change_item_tree(args, (*ref0)); + args[0]= (*ref0); DBUG_PRINT("info", ("actual fix fields")); cache->setup(args[0]); -- cgit v1.2.1