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.

1 files changed, 30 insertions, 4 deletions

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