From 1c810152963f770fae011b29ae30fbafbc516b23 Mon Sep 17 00:00:00 2001 From: Alexander Nozdrin Date: Tue, 21 Jun 2011 19:24:44 +0400 Subject: [PATCH] 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. --- mysql-test/r/sp.result | 72 ++++++++++++++++++++++++++++++++++++++++++ mysql-test/t/sp.test | 56 ++++++++++++++++++++++++++++++++ sql/item_cmpfunc.cc | 34 +++++++++++++++++--- 3 files changed, 158 insertions(+), 4 deletions(-) diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result index 6b4215e6b09..8512407aaf3 100644 --- a/mysql-test/r/sp.result +++ b/mysql-test/r/sp.result @@ -7500,4 +7500,76 @@ CALL p1(); DROP TABLE t1, t2, t3; DROP PROCEDURE p1; + +# -- +# -- Bug#12652769 - 61470: case operator in stored routine retains old +# -- value of input parameter +# --- +DROP TABLE IF EXISTS t1; +DROP PROCEDURE IF EXISTS p1; +CREATE TABLE t1 (s1 CHAR(5) CHARACTER SET utf8); +INSERT INTO t1 VALUES ('a'); +CREATE PROCEDURE p1(dt DATETIME, i INT) +BEGIN +SELECT +CASE +WHEN i = 1 THEN 2 +ELSE dt +END AS x1; +SELECT +CASE _latin1'a' + WHEN _utf8'a' THEN 'A' + END AS x2; +SELECT +CASE _utf8'a' + WHEN _latin1'a' THEN _utf8'A' + END AS x3; +SELECT +CASE s1 +WHEN _latin1'a' THEN _latin1'b' + ELSE _latin1'c' + END AS x4 +FROM t1; +END| + +CALL p1('2011-04-03 05:14:10', 1); +x1 +2 +x2 +A +x3 +A +x4 +b +CALL p1('2011-04-03 05:14:11', 2); +x1 +2011-04-03 05:14:11 +x2 +A +x3 +A +x4 +b +CALL p1('2011-04-03 05:14:12', 2); +x1 +2011-04-03 05:14:12 +x2 +A +x3 +A +x4 +b +CALL p1('2011-04-03 05:14:13', 2); +x1 +2011-04-03 05:14:13 +x2 +A +x3 +A +x4 +b + +DROP TABLE t1; +DROP PROCEDURE p1; + # End of 5.5 test diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test index b052b181d70..a9220aa04e6 100644 --- a/mysql-test/t/sp.test +++ b/mysql-test/t/sp.test @@ -8778,4 +8778,60 @@ DROP TABLE t1, t2, t3; DROP PROCEDURE p1; --echo + +--echo +--echo # -- +--echo # -- Bug#12652769 - 61470: case operator in stored routine retains old +--echo # -- value of input parameter +--echo # --- + +--disable_warnings +DROP TABLE IF EXISTS t1; +DROP PROCEDURE IF EXISTS p1; +--enable_warnings + +CREATE TABLE t1 (s1 CHAR(5) CHARACTER SET utf8); +INSERT INTO t1 VALUES ('a'); + +delimiter |; + +CREATE PROCEDURE p1(dt DATETIME, i INT) +BEGIN + SELECT + CASE + WHEN i = 1 THEN 2 + ELSE dt + END AS x1; + + SELECT + CASE _latin1'a' + WHEN _utf8'a' THEN 'A' + END AS x2; + + SELECT + CASE _utf8'a' + WHEN _latin1'a' THEN _utf8'A' + END AS x3; + + SELECT + CASE s1 + WHEN _latin1'a' THEN _latin1'b' + ELSE _latin1'c' + END AS x4 + FROM t1; +END| + +delimiter ;| + +--echo +CALL p1('2011-04-03 05:14:10', 1); +CALL p1('2011-04-03 05:14:11', 2); +CALL p1('2011-04-03 05:14:12', 2); +CALL p1('2011-04-03 05:14:13', 2); + +--echo +DROP TABLE t1; +DROP PROCEDURE p1; +--echo + --echo # End of 5.5 test 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++) {