From 819eaead10f656356dbc8f41770679b5ed45a8c9 Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 12 Nov 2007 14:44:17 +0300 Subject: [PATCH] Fix for a BUG#31898: 16M memory allocations for user variables in stored procedure. The problem was that MySQL used unnecessarily large amounts of memory if user variables were used as an argument to CONCAT or CONCAT_WS -- 16M per each user variable used. Technically, it happened because MySQL used the following allocation strategy for string functions to avoid multiple realloc() calls: in the virtual operation fix_length_and_dec() the attribute max_length was calculated as a sum of max_length values for each argument. Although this approach worked well for small (or fixed) data types, there could be a problem if there as a user variable among the arguments of a string function -- max_length of the function would be 16M (as the max_length of a user variable is 16M). Both CONCAT() and CONCAT_WS() functions suffer from this problem. The fix is to do not use meta-data for allocating memory. The following strategy is proposed instead: allocate the exact length of the result string at the first record, double the amount of memory allocated when it is required. No test case for this bug because there is no way to test memory consumption in a robust way with our test suite. sql/item_strfunc.cc: Implement memory-wise allocation strategy. --- sql/item_strfunc.cc | 60 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 5 deletions(-) diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc index 03c65c9d654..a7624c5bbcd 100644 --- a/sql/item_strfunc.cc +++ b/sql/item_strfunc.cc @@ -356,10 +356,35 @@ String *Item_func_concat::val_str(String *str) } else { // Two big const strings - if (tmp_value.alloc(max_length) || - tmp_value.copy(*res) || - tmp_value.append(*res2)) + /* + NOTE: We should be prudent in the initial allocation unit -- the + size of the arguments is a function of data distribution, which + can be any. Instead of overcommitting at the first row, we grow + the allocated amount by the factor of 2. This ensures that no + more than 25% of memory will be overcommitted on average. + */ + + uint concat_len= res->length() + res2->length(); + + if (tmp_value.alloced_length() < concat_len) + { + if (tmp_value.alloced_length() == 0) + { + if (tmp_value.alloc(concat_len)) + goto null; + } + else + { + uint new_len = max(tmp_value.alloced_length() * 2, concat_len); + + if (tmp_value.realloc(new_len)) + goto null; + } + } + + if (tmp_value.copy(*res) || tmp_value.append(*res2)) goto null; + res= &tmp_value; use_as_buff=str; } @@ -679,8 +704,33 @@ String *Item_func_concat_ws::val_str(String *str) } else { // Two big const strings - if (tmp_value.alloc(max_length) || - tmp_value.copy(*res) || + /* + NOTE: We should be prudent in the initial allocation unit -- the + size of the arguments is a function of data distribution, which can + be any. Instead of overcommitting at the first row, we grow the + allocated amount by the factor of 2. This ensures that no more than + 25% of memory will be overcommitted on average. + */ + + uint concat_len= res->length() + sep_str->length() + res2->length(); + + if (tmp_value.alloced_length() < concat_len) + { + if (tmp_value.alloced_length() == 0) + { + if (tmp_value.alloc(concat_len)) + goto null; + } + else + { + uint new_len = max(tmp_value.alloced_length() * 2, concat_len); + + if (tmp_value.realloc(new_len)) + goto null; + } + } + + if (tmp_value.copy(*res) || tmp_value.append(*sep_str) || tmp_value.append(*res2)) goto null;