From 67e2028161d1f653a852f1a4679ff5e523296218 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Sat, 14 Jan 2017 14:56:01 +0100 Subject: MDEV-9690 concurrent queries with virtual columns crash in temporal code Item_func_le included Arg_comparator. Arg_comparator remembered the current_thd during fix_fields and used that value during execution to allocate Item_cache in get_datetime_value(). But for vcols fix_fields and val_int can happen in different threads. Same bug for Item_func_in using in_datetime or cmp_item_datetime, both also remembered current_thd at fix_fields() to use it later for get_datetime_value(). As a fix, these objects no longer remember the current_thd, and get_datetime_value() uses current_thd at run time. This should not increase the number of current_thd calls much, as Item_cache is created only once anyway. --- sql/item_cmpfunc.cc | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) (limited to 'sql/item_cmpfunc.cc') diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 3bd0b5b3fa2..46566206094 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -777,7 +777,7 @@ int Arg_comparator::set_cmp_func(Item_result_field *owner_arg, Item **a1, Item **a2, Item_result type) { - thd= current_thd; + THD *thd= current_thd; owner= owner_arg; set_null= set_null && owner_arg; a= a1; @@ -846,7 +846,6 @@ Item** Arg_comparator::cache_converted_constant(THD *thd_arg, Item **value, void Arg_comparator::set_datetime_cmp_func(Item_result_field *owner_arg, Item **a1, Item **b1) { - thd= current_thd; owner= owner_arg; a= a1; b= b1; @@ -919,6 +918,9 @@ get_datetime_value(THD *thd, Item ***item_arg, Item **cache_arg, if (cache_arg && item->const_item() && !(item->type() == Item::CACHE_ITEM && item->cmp_type() == TIME_RESULT)) { + if (!thd) + thd= current_thd; + Query_arena backup; Query_arena *save_arena= thd->switch_to_arena_for_cached_items(&backup); Item_cache_temporal *cache= new Item_cache_temporal(f_type); @@ -959,12 +961,12 @@ int Arg_comparator::compare_datetime() owner->null_value= 1; /* Get DATE/DATETIME/TIME value of the 'a' item. */ - a_value= get_datetime_value(thd, &a, &a_cache, *b, &a_is_null); + a_value= get_datetime_value(0, &a, &a_cache, *b, &a_is_null); if (a_is_null) return -1; /* Get DATE/DATETIME/TIME value of the 'b' item. */ - b_value= get_datetime_value(thd, &b, &b_cache, *a, &b_is_null); + b_value= get_datetime_value(0, &b, &b_cache, *a, &b_is_null); if (b_is_null) return -1; @@ -982,10 +984,10 @@ int Arg_comparator::compare_e_datetime() longlong a_value, b_value; /* Get DATE/DATETIME/TIME value of the 'a' item. */ - a_value= get_datetime_value(thd, &a, &a_cache, *b, &a_is_null); + a_value= get_datetime_value(0, &a, &a_cache, *b, &a_is_null); /* Get DATE/DATETIME/TIME value of the 'b' item. */ - b_value= get_datetime_value(thd, &b, &b_cache, *a, &b_is_null); + b_value= get_datetime_value(0, &b, &b_cache, *a, &b_is_null); return a_is_null || b_is_null ? a_is_null == b_is_null : a_value == b_value; } @@ -3600,7 +3602,7 @@ void in_datetime::set(uint pos,Item *item) bool is_null; struct packed_longlong *buff= &((packed_longlong*) base)[pos]; - buff->val= get_datetime_value(thd, &tmp_item, 0, warn_item, &is_null); + buff->val= get_datetime_value(0, &tmp_item, 0, warn_item, &is_null); buff->unsigned_flag= 1L; } @@ -3608,7 +3610,7 @@ uchar *in_datetime::get_value(Item *item) { bool is_null; Item **tmp_item= lval_cache ? &lval_cache : &item; - tmp.val= get_datetime_value(thd, &tmp_item, &lval_cache, warn_item, &is_null); + tmp.val= get_datetime_value(0, &tmp_item, &lval_cache, warn_item, &is_null); if (item->null_value) return 0; tmp.unsigned_flag= 1L; @@ -3852,7 +3854,7 @@ void cmp_item_datetime::store_value(Item *item) { bool is_null; Item **tmp_item= lval_cache ? &lval_cache : &item; - value= get_datetime_value(thd, &tmp_item, &lval_cache, warn_item, &is_null); + value= get_datetime_value(0, &tmp_item, &lval_cache, warn_item, &is_null); } @@ -3861,7 +3863,7 @@ int cmp_item_datetime::cmp(Item *arg) bool is_null; Item **tmp_item= &arg; return value != - get_datetime_value(thd, &tmp_item, 0, warn_item, &is_null); + get_datetime_value(0, &tmp_item, 0, warn_item, &is_null); } -- cgit v1.2.1 From 798fcb541698cbf51f1ee33f44b023c11dc2b784 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Sat, 14 Jan 2017 20:55:33 +0100 Subject: bugfix: cmp_item_row::alloc_comparators() allocated on the wrong arena it used current_thd->alloc() and allocated on the thd's execution arena, not on table->expr_arena. Remove THD::arena_for_cached_items that is temporarily set in update_virtual_fields(), and replaces THD arena in get_datetime_value(). Instead set THD arena to table->expr_arena for the whole duration of update_virtual_fields() --- sql/item_cmpfunc.cc | 5 ----- 1 file changed, 5 deletions(-) (limited to 'sql/item_cmpfunc.cc') diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 46566206094..ebe088e5092 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -921,12 +921,7 @@ get_datetime_value(THD *thd, Item ***item_arg, Item **cache_arg, if (!thd) thd= current_thd; - Query_arena backup; - Query_arena *save_arena= thd->switch_to_arena_for_cached_items(&backup); Item_cache_temporal *cache= new Item_cache_temporal(f_type); - if (save_arena) - thd->set_query_arena(save_arena); - cache->store_packed(value, item); *cache_arg= cache; *item_arg= cache_arg; -- cgit v1.2.1