Commit 1d48aec2 authored by unknown's avatar unknown
Browse files

A fix and test case for Bug#5987 "subselect in bool function 

crashes server (prepared statements)": the bug was that all boolean
items always recovered its original arguments at statement cleanup 
stage.
This collided with Item_subselect::select_transformer, which tries to 
permanently change the item tree to use a transformed subselect instead of
original one.
So we had this call sequence for prepare:
mysql_stmt_prepare -> JOIN::prepare -> 
Item_subselect::fix_fields -> the item tree gets transformed ->
Item_bool_rowready_func2::cleanup, item tree is recovered to original
state, while it shouldn't have been;
mysql_stmt_execute -> attempts to execute a broken tree -> crash.
Now instead of bluntly recovering all arguments of bool functions in 
Item_bool_rowready_func2::cleanup, we recover only those
which were changed, and do it in one place.
There still would exist a possibility for a collision with subselect
tranformation, if permanent and temporary changes were performed at the 
same stage.
But fortunately subselect transformation is always done first, so it 
doesn't conflict with the optimization done by propogate_cond_constants.
Now we have: 
mysql_stmt_prepare -> JOIN::prepare -> subselect transformation 
permanently changes the tree -> cleanup doesn't recover anything, 
because nothing was registered for recovery.
mysql_stmt_execute -> JOIN::prepare (the tree is already transformed, 
so it doesn't change), JOIN::optimize -> 
propogate_cond_constants -> temporary changes the item tree 
with constants -> JOIN::execute -> cleanup -> 
the changes done by propogate_cond_constants are recovered, as
they were registered for recovery.


mysql-test/r/ps.result:
  Bug#5987: test results fixed.
mysql-test/t/ps.test:
  A test for bug#5987 "subselect in bool function crashes server 
  (prepared statements)"
sql/item.cc:
  resolve_const_item is now responsible to register all changes of the 
  item tree for recovery
sql/item.h:
  resolve_const_item signagture changed
sql/item_cmpfunc.h:
  Arguments of boolean functions are now recovered using the 
  centralized registry of THD.
sql/sql_class.cc:
  It's crucial to add new items to the beginning of the recovery list,
  so that the recovery is performed in LIFO mode: otherwise if we 
  change one node of a tree twice, it will be recovered to some intermediate
  state.
sql/sql_select.cc:
  change_cond_ref_to_const and propogate_cond_constants are now responsible
  to register all changes of the item tree for recovery.
  The recovery is done using the centralized THD registry of
  changed tree items.
parent 166d19e9

mysql-test/r/ps.result

+10 −0
Original line number Diff line number Diff line
@@ -308,3 +308,13 @@ execute stmt using @a, @a; 
a

drop table t1;

deallocate prepare stmt;

create table t1 (a int);

prepare stmt from "select * from t1 where 1 > (1 in (SELECT * FROM t1))";

execute stmt;

a

execute stmt;

a

execute stmt;

a

drop table t1;

deallocate prepare stmt;

mysql-test/t/ps.test

+14 −0
Original line number Diff line number Diff line
@@ -329,3 +329,17 @@ execute stmt using @a, @a; 
drop table t1;

deallocate prepare stmt;



#

# Bug #5987 subselect in bool function crashes server (prepared statements):

# don't overwrite transformed subselects with old arguments of a bool

# function.

#

create table t1 (a int);

prepare stmt from "select * from t1 where 1 > (1 in (SELECT * FROM t1))";

execute stmt;

execute stmt;

execute stmt;

drop table t1;

deallocate prepare stmt;

sql/item.cc

+21 −11
Original line number Diff line number Diff line
@@ -2209,10 +2209,12 @@ Item_result item_cmp_type(Item_result a,Item_result b) 
}





Item *resolve_const_item(Item *item,Item *comp_item)

void resolve_const_item(THD *thd, Item **ref, Item *comp_item)

{

  Item *item= *ref;

  Item *new_item;

  if (item->basic_const_item())

    return item;				// Can't be better

    return;                                     // Can't be better

  Item_result res_type=item_cmp_type(comp_item->result_type(),

				     item->result_type());

  char *name=item->name;			// Alloced by sql_alloc
@@ -2223,17 +2225,20 @@ Item *resolve_const_item(Item *item,Item *comp_item) 
    String tmp(buff,sizeof(buff),&my_charset_bin),*result;

    result=item->val_str(&tmp);

    if (item->null_value)

      return new Item_null(name);

      new_item= new Item_null(name);

    else

    {

      uint length= result->length();

      char *tmp_str= sql_strmake(result->ptr(), length);

    return new Item_string(name,tmp_str,length,result->charset());

      new_item= new Item_string(name, tmp_str, length, result->charset());

    }

  if (res_type == INT_RESULT)

  }

  else if (res_type == INT_RESULT)

  {

    longlong result=item->val_int();

    uint length=item->max_length;

    bool null_value=item->null_value;

    return (null_value ? (Item*) new Item_null(name) :

    new_item= (null_value ? (Item*) new Item_null(name) :

               (Item*) new Item_int(name, result, length));

  }

  else
@@ -2241,8 +2246,13 @@ Item *resolve_const_item(Item *item,Item *comp_item) 
    double result=item->val();

    uint length=item->max_length,decimals=item->decimals;

    bool null_value=item->null_value;

    return (null_value ? (Item*) new Item_null(name) :

	    (Item*) new Item_real(name,result,decimals,length));

    new_item= (null_value ? (Item*) new Item_null(name) : (Item*)

               new Item_real(name, result, decimals, length));

  }

  if (new_item)

  {

    thd->register_item_tree_change(ref, item, &thd->mem_root);

    *ref= new_item;

  }

}

sql/item.h

+1 −1
Original line number Diff line number Diff line
@@ -1223,5 +1223,5 @@ class Item_type_holder: public Item 


extern Item_buff *new_Item_buff(Item *item);

extern Item_result item_cmp_type(Item_result a,Item_result b);

extern Item *resolve_const_item(Item *item,Item *cmp_item);

extern void resolve_const_item(THD *thd, Item **ref, Item *cmp_item);

extern bool field_is_equal_to_item(Field *field,Item *item);

sql/item_cmpfunc.h

+1 −11
Original line number Diff line number Diff line
@@ -210,21 +210,11 @@ class Item_bool_func2 :public Item_int_func 


class Item_bool_rowready_func2 :public Item_bool_func2

{

  Item *orig_a, *orig_b; /* propagate_const can change parameters */

public:

  Item_bool_rowready_func2(Item *a,Item *b) :Item_bool_func2(a,b),

    orig_a(a), orig_b(b)

  Item_bool_rowready_func2(Item *a, Item *b) :Item_bool_func2(a, b)

  {

    allowed_arg_cols= a->cols();

  }

  void cleanup()

  {

    DBUG_ENTER("Item_bool_rowready_func2::cleanup");

    Item_bool_func2::cleanup();

    tmp_arg[0]= orig_a;

    tmp_arg[1]= orig_b;

    DBUG_VOID_RETURN;

  }

  Item *neg_transformer(THD *thd);

  virtual Item *negated_item();

};