maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Sanja!

On Apr 23, sanja@xxxxxxxxxxx wrote:
> revision-id: 9f8a458fb2d07298810bb5d9824ce728017bfb32
> parent(s): e540d023e2ec6f37efc9ab695ccdfd4a6744ad64
> committer: Oleksandr Byelkin
> branch nick: server
> timestamp: 2015-04-23 20:08:57 +0200
> message:
> 
> MDEV-7846: Server crashes in Item_subselect::fix_fields or fails with
> Thread stack overrun
> 
> Substitute into transformed subselects original left expression and
> than register its change in case it was substituted.

Looks mostly ok to me. But please:

1. Add comments (the patch is attached)
2. Add a test case with row items
3. In your test case - if you remove GROUP BY you get a crash again.
   This might be a different bug, though.

Regards,
Sergei

diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc
index e04f5ff..5aa5b6f 100644
--- a/sql/item_subselect.cc
+++ b/sql/item_subselect.cc
@@ -2651,6 +2651,8 @@ bool Item_in_subselect::fix_fields(THD *thd_arg, Item **ref)
   List<Item> *inner_cols;
   char const *save_where= thd->where;
 
+  DBUG_ASSERT(left_expr == left_expr_orig);
+
   if (test_strategy(SUBS_SEMI_JOIN))
     return !( (*ref)= new Item_int(1));
 
diff --git a/sql/item_subselect.h b/sql/item_subselect.h
index 930bd66..9c1ff45 100644
--- a/sql/item_subselect.h
+++ b/sql/item_subselect.h
@@ -449,7 +449,14 @@ class Item_in_subselect :public Item_exists_subselect
                                     Item **having_item);
 public:
   Item *left_expr;
+
+  /*
+    important for PS: left_expr_orig is the item that left_expr originally
+    pointed at. That item is allocated on the statement arena, while
+    left_expr could later be changed to something on the execution arena
+  */
   Item *left_expr_orig;
+
   /* Priority of this predicate in the convert-to-semi-join-nest process. */
   int sj_convert_priority;
   /*
diff --git a/sql/opt_subselect.cc b/sql/opt_subselect.cc
index 1363be0..4594238 100644
--- a/sql/opt_subselect.cc
+++ b/sql/opt_subselect.cc
@@ -1592,6 +1592,15 @@ static bool convert_subq_to_sj(JOIN *parent_join, Item_in_subselect *subq_pred)
   if (subq_pred->left_expr->cols() == 1)
   {
     nested_join->sj_outer_expr_list.push_back(subq_pred->left_expr);
+    /*
+      Create Item_func_eq. Note that
+      1. this is done on the statement, not execution, arena
+      2. if it's a PS then this happens only once - on the first execution.
+         On following re-executions, the item will be fix_field-ed normally.
+      3. Thus it should be created as if it was fix_field'ed, in particular
+         all pointers to items in the execution arena should be protected
+         with thd->change_item_tree
+    */
     Item_func_eq *item_eq=
       new Item_func_eq(subq_pred->left_expr_orig, subq_lex->ref_pointer_array[0]);
     if (subq_pred->left_expr_orig != subq_pred->left_expr)