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 <some charset>)-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

@@ -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

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

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++)
     {