From 8b1566aaaf93e6e885badd6500a07a0f70cc81f3 Mon Sep 17 00:00:00 2001 From: Alexander Nozdrin Date: Tue, 21 Jun 2011 19:24:44 +0400 Subject: Patch for Bug 12652769 - 61470: CASE OPERATOR IN STORED ROUTINE RETAINS OLD VALUE OF INPUT PARAMETER. The user-visible problem was that CASE-control-flow function (not CASE-statement) misbehaved in stored routines under some circumstances. The problem resulted in a crash or wrong data returned. The error happened when expressions in CASE-function were not of the same character set. A CASE-function should return values of the same character set for all branches. Internally, that means a new Item-instance for the CONVERT(... USING )-function is added to the item tree when needed. The problem was that such changes were not properly recorded using THD::change_item_tree(), thus dangling pointers remain in the item tree after THD::rollback_item_tree_changes(), which lead to undefined behavior (i.e. crash / wrong data) for subsequent executions of the stored routine. This bug was introduced by a patch for Bug 11753363 (44793 - CHARACTER SETS: CASE CLAUSE, UCS2 OR UTF32, FAILURE). The fixed function is Item_func_case::fix_length_and_dec(). New CONVERT-items are added in agg_item_set_converter(), which calls THD::change_item_tree(). The problem was that an intermediate array was passed to agg_item_set_converter(). Thus, THD::change_item_tree() there was called on intermediate objects. Note: those intermediate objects are allocated on THD's memory root, so it's Ok to put them into "changed item lists". The fix is to track changes on the correct objects. --- sql/item_cmpfunc.cc | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) (limited to 'sql') diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index e0057d1550b..e28221109d9 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -3007,11 +3007,35 @@ void Item_func_case::agg_num_lengths(Item *arg) } +/** + Check if (*place) and new_value points to different Items and call + THD::change_item_tree() if needed. + + This function is a workaround for implementation deficiency in + Item_func_case. The problem there is that the 'args' attribute contains + Items from different expressions. + + The function must not be used elsewhere and will be remove eventually. +*/ + +static void change_item_tree_if_needed(THD *thd, + Item **place, + Item *new_value) +{ + if (*place == new_value) + return; + + thd->change_item_tree(place, new_value); +} + + void Item_func_case::fix_length_and_dec() { Item **agg; uint nagg; uint found_types= 0; + THD *thd= current_thd; + if (!(agg= (Item**) sql_alloc(sizeof(Item*)*(ncases+1)))) return; @@ -3036,9 +3060,10 @@ void Item_func_case::fix_length_and_dec() Some of the items might have been changed to Item_func_conv_charset. */ for (nagg= 0 ; nagg < ncases / 2 ; nagg++) - args[nagg * 2 + 1]= agg[nagg]; + change_item_tree_if_needed(thd, &args[nagg * 2 + 1], agg[nagg]); + if (else_expr_num != -1) - args[else_expr_num]= agg[nagg++]; + change_item_tree_if_needed(thd, &args[else_expr_num], agg[nagg++]); } else collation.set_numeric(); @@ -3098,9 +3123,10 @@ void Item_func_case::fix_length_and_dec() arrray, because some of the items might have been changed to converters (e.g. Item_func_conv_charset, or Item_string for constants). */ - args[first_expr_num]= agg[0]; + change_item_tree_if_needed(thd, &args[first_expr_num], agg[0]); + for (nagg= 0; nagg < ncases / 2; nagg++) - args[nagg * 2]= agg[nagg + 1]; + change_item_tree_if_needed(thd, &args[nagg * 2], agg[nagg + 1]); } for (i= 0; i <= (uint)DECIMAL_RESULT; i++) { -- cgit v1.2.1