Skip to content

Commit

Permalink
MDEV-9690 concurrent queries with virtual columns crash in temporal code
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@vuvova
vuvova committed Jan 14, 2017
1 parent 20ca1bc commit 67e2028
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 18 deletions.
39 changes: 39 additions & 0 deletions mysql-test/suite/vcol/r/wrong_arena.result
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
create table t1 (a datetime,
# get_datetime_value
b int as (a > 1), # Arg_comparator
c int as (a in (1,2,3)), # in_datetime
d int as ((a,a) in ((1,1),(2,1),(NULL,1))) # cmp_item_datetime
);
Warnings:
Warning 1292 Incorrect datetime value: '1'
Warning 1292 Incorrect datetime value: '2'
Warning 1292 Incorrect datetime value: '3'
show create table t1;
Table Create Table
t1 CREATE TABLE `t1` (
`a` datetime DEFAULT NULL,
`b` int(11) AS (a > 1) VIRTUAL,
`c` int(11) AS (a in (1,2,3)) VIRTUAL,
`d` int(11) AS ((a,a) in ((1,1),(2,1),(NULL,1))) VIRTUAL
) ENGINE=MyISAM DEFAULT CHARSET=latin1
Warnings:
Warning 1292 Incorrect datetime value: '1'
Warning 1292 Incorrect datetime value: '2'
Warning 1292 Incorrect datetime value: '3'
insert t1 (a) values ('2010-10-10 10:10:10');
select * from t1;
a b c d
2010-10-10 10:10:10 1 0 0
Warnings:
Warning 1292 Incorrect datetime value: '1'
Warning 1292 Incorrect datetime value: '1'
Warning 1292 Incorrect datetime value: '2'
Warning 1292 Incorrect datetime value: '1'
select * from t1;
a b c d
2010-10-10 10:10:10 1 0 0
Warnings:
Warning 1292 Incorrect datetime value: '1'
Warning 1292 Incorrect datetime value: '2'
Warning 1292 Incorrect datetime value: '1'
drop table t1;
22 changes: 22 additions & 0 deletions mysql-test/suite/vcol/t/wrong_arena.test
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#
# This tests various issues when vcol items allocate memory (e.g. more items)
# not in the TABLE::expr_arena.
#

#
# MDEV-9690 concurrent queries with virtual columns crash in temporal code
#
create table t1 (a datetime,
# get_datetime_value
b int as (a > 1), # Arg_comparator
c int as (a in (1,2,3)), # in_datetime
d int as ((a,a) in ((1,1),(2,1),(NULL,1))) # cmp_item_datetime
);
show create table t1;
connect con1, localhost, root;
insert t1 (a) values ('2010-10-10 10:10:10');
select * from t1;
disconnect con1;
connection default;
select * from t1;
drop table t1;
22 changes: 12 additions & 10 deletions sql/item_cmpfunc.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;

Expand All @@ -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;
}
Expand Down Expand Up @@ -3600,15 +3602,15 @@ 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;
}

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;
Expand Down Expand Up @@ -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);
}


Expand All @@ -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);
}


Expand Down
12 changes: 4 additions & 8 deletions sql/item_cmpfunc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,17 @@ class Arg_comparator: public Sql_alloc
Arg_comparator *comparators; // used only for compare_row()
double precision;
/* Fields used in DATE/DATETIME comparison. */
THD *thd;
Item *a_cache, *b_cache; // Cached values of a and b items
// when one of arguments is NULL.
public:
DTCollation cmp_collation;
/* Allow owner function to use string buffers. */
String value1, value2;

Arg_comparator(): set_null(TRUE), comparators(0), thd(0),
Arg_comparator(): set_null(TRUE), comparators(0),
a_cache(0), b_cache(0) {};
Arg_comparator(Item **a1, Item **a2): a(a1), b(a2), set_null(TRUE),
comparators(0), thd(0), a_cache(0), b_cache(0) {};
comparators(0), a_cache(0), b_cache(0) {};

int set_compare_func(Item_result_field *owner, Item_result type);
inline int set_compare_func(Item_result_field *owner_arg)
Expand Down Expand Up @@ -944,15 +943,13 @@ class in_longlong :public in_vector
class in_datetime :public in_longlong
{
public:
THD *thd;
/* An item used to issue warnings. */
Item *warn_item;
/* Cache for the left item. */
Item *lval_cache;

in_datetime(Item *warn_item_arg, uint elements)
:in_longlong(elements), thd(current_thd), warn_item(warn_item_arg),
lval_cache(0) {};
:in_longlong(elements), warn_item(warn_item_arg), lval_cache(0) {};
void set(uint pos,Item *item);
uchar *get_value(Item *item);
Item* create_item()
Expand Down Expand Up @@ -1112,14 +1109,13 @@ class cmp_item_datetime :public cmp_item
{
longlong value;
public:
THD *thd;
/* Item used for issuing warnings. */
Item *warn_item;
/* Cache for the left item. */
Item *lval_cache;

cmp_item_datetime(Item *warn_item_arg)
:thd(current_thd), warn_item(warn_item_arg), lval_cache(0) {}
: warn_item(warn_item_arg), lval_cache(0) {}
void store_value(Item *item);
int cmp(Item *arg);
int compare(cmp_item *ci);
Expand Down

0 comments on commit 67e2028

Please sign in to comment.