On 08.06.15 18:49, Sergey Petrunia wrote:

On Thu, Apr 23, 2015 at 07:11:09PM +0200, sanja@mariadb.com wrote:

...

revision-id: 5ccbfa02d4da91c3f9080dc1b1627d19cf40eccd parent(s): c7efc0315b51f214b6423ae4dcb4ddeb1b14e91a committer: Oleksandr Byelkin branch nick: server timestamp: 2015-04-23 19:11:06 +0200 message:

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().

--- ... diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 998cb1c..51a38f0 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]; + I was looking for the case where the above if statement is not taken. Eventually I have applied this patch, the patch after it (e540d023e2ec6f37efc9ab695ccdfd4a6744ad64) to get the test cases,

then added this at line (*): DBUG_ASSERT(args[1]->type() == Item::SUBSELECT_ITEM);

and ran the entire main test suite, with and without --ps-protocol. It worked. Can you add test coverage for this case? Nowdays min/max transformation done later so code was there "just in case" to be in line with all other places where we could expect non-subselect (like execution). But non of the code should show a bad example so it was fixed.

...

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)

...

BR Sergei