Skip to content

Commit

Permalink
Revert "MDEV-10713: signal 11 error on multi-table update - crash in …
Browse files Browse the repository at this point in the history
…handler::increment_statistics or in make_select or assertion failure pfs_thread == ((PFS_thread*) pthread_getspecific((THR_PFS)))"

This reverts commit 035a5ac.

Two minor problems and one regression:
1. caching the value in str_result. Other Item methods may use it,
   destroying the cache. See, for example, Item::save_in_field, where
   str_result is moved to use a local buffer (this failed main.grant)
2. Item_func_conv_charset::safe is now set too late, it's initialized
   only in val_str() but checked before that, this failed many tests
   in optimized builds.

to fix 1 - use tmp_result instead of str_result, to fix 2, use
the else branch in the Item_func_conv_charset constructor to set
safe purely from charset properties.

But this introduces a regression, constant strings can no longer be
converted, say, from utf8 to latin1 (because 'safe' will be false).
This fails few tests too. There is no way to fix it without reverting
the commit and converting constants, as before, in the constructor.
  • Loading branch information
vuvova committed Dec 8, 2016
1 parent f5e0522 commit ab65db6
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 25 deletions.
14 changes: 1 addition & 13 deletions sql/item_strfunc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2951,19 +2951,7 @@ String *Item_func_conv::val_str(String *str)
String *Item_func_conv_charset::val_str(String *str)
{
DBUG_ASSERT(fixed == 1);
if (cached_value == CONST_WILL_BE_CACHED)
{
uint errors= 0;
String tmp, *str= args[0]->val_str(&tmp);
if (!str || str_value.copy(str->ptr(), str->length(),
str->charset(), conv_charset, &errors))
null_value= 1;
cached_value= CACHED;
str_value.mark_as_const();
safe= (errors == 0);
is_expensive_cache= 0;
}
if (cached_value == CACHED)
if (use_cached_value)
return null_value ? 0 : &str_value;
String *arg= args[0]->val_str(str);
uint dummy_errors;
Expand Down
28 changes: 16 additions & 12 deletions sql/item_strfunc.h
Original file line number Diff line number Diff line change
Expand Up @@ -817,26 +817,31 @@ class Item_func_quote :public Item_str_func

class Item_func_conv_charset :public Item_str_func
{
bool use_cached_value;
String tmp_value;
enum state_of_cache { NOT_CONST, CONST_WILL_BE_CACHED, CACHED };
enum state_of_cache cached_value;
public:
bool safe;
CHARSET_INFO *conv_charset; // keep it public
Item_func_conv_charset(Item *a, CHARSET_INFO *cs) :Item_str_func(a),
cached_value(NOT_CONST), safe(0), conv_charset(cs)
{}
Item_func_conv_charset(Item *a, CHARSET_INFO *cs, bool cache_if_const)
:Item_str_func(a), conv_charset(cs)
Item_func_conv_charset(Item *a, CHARSET_INFO *cs) :Item_str_func(a)
{ conv_charset= cs; use_cached_value= 0; safe= 0; }
Item_func_conv_charset(Item *a, CHARSET_INFO *cs, bool cache_if_const)
:Item_str_func(a)
{
if (cache_if_const && args[0]->const_item())
conv_charset= cs;
if (cache_if_const && args[0]->const_item() && !args[0]->is_expensive())
{
is_expensive_cache= MY_TEST(args[0]->is_expensive());
cached_value= CONST_WILL_BE_CACHED;
uint errors= 0;
String tmp, *str= args[0]->val_str(&tmp);
if (!str || str_value.copy(str->ptr(), str->length(),
str->charset(), conv_charset, &errors))
null_value= 1;
use_cached_value= 1;
str_value.mark_as_const();
safe= (errors == 0);
}
else
{
cached_value= NOT_CONST;
use_cached_value= 0;
/*
Conversion from and to "binary" is safe.
Conversion to Unicode is safe.
Expand Down Expand Up @@ -887,7 +892,6 @@ class Item_func_conv_charset :public Item_str_func
void fix_length_and_dec();
const char *func_name() const { return "convert"; }
virtual void print(String *str, enum_query_type query_type);
virtual bool const_item() const { return cached_value != NOT_CONST; }
};

class Item_func_set_collation :public Item_str_func
Expand Down

0 comments on commit ab65db6

Please sign in to comment.