Bug#21575790 ROLLUP WITH IN SUBQUERY CAUSES CRASH

There is an invariant in Item_in_optimizer which is violated by the
tree rewriting done for ROLLUP. Quote from Item_in_optimizer's class
Doxygen:

  args[0] is a copy of Item_in_subselect's left expression and should be
  kept equal also after resolving.

ROLLUP replaces arg[0] with an Item_ref in SELECT_LEX::change_group_ref:

  "Replace occurrences of group by fields in an expression by ref items.
   The function replaces occurrences of group by fields in expr
   by ref objects for these fields unless they are under aggregate
   functions."

This replacement fails to update Item_in_subselect's left
expression. This lack of synchronization leads to an error by going
down the wrong code path based on the incorrect state in left.

The fix is to make sure Item_in_subselect's arg[0] stays in synch with
the Item_in_subselect's left expression. It also makes sure the cache
of the old item now uses the new.

Regression test added.

Author: Dag Wanvik

sql/item_cmpfunc.cc

@@ -2481,6 +2481,22 @@ Item *Item_in_optimizer::transform(Item_transformer transformer, uchar *argument
 }
 
 
+void Item_in_optimizer::replace_argument(THD *thd, Item **oldpp, Item *newp)
+{
+  // Maintain the invariant described in this class's comment
+  Item_in_subselect *ss= down_cast<Item_in_subselect *>(args[1]);
+  thd->change_item_tree(&ss->left_expr, newp);
+  /*
+    fix_left() does cache setup. This setup() does (mainly) cache->example=arg[0];
+    we could wonder why change_item_tree isn't used instead of this simple
+    assignment. The reason is that cache->setup() is called at every
+    fix_fields(), so every execution, so it's not important if the previous
+    execution left a non-rolled-back now-pointing-to-garbage cache->example -
+    it will be overwritten.
+  */
+  fix_left(thd, NULL);
+}
+
 longlong Item_func_eq::val_int()
 {
   DBUG_ASSERT(fixed == 1);

sql/item_cmpfunc.h

@@ -319,6 +319,7 @@ class Item_in_optimizer: public Item_bool_func
   Item_cache **get_cache() { return &cache; }
   void keep_top_level_cache();
   Item *transform(Item_transformer transformer, uchar *arg);
+  void replace_argument(THD *thd, Item **oldpp, Item *newp);
 };
 
 /// Abstract factory interface for creating comparison predicates.

sql/item_func.cc

@@ -1101,6 +1101,12 @@ Item *Item_func::gc_subst_transformer(uchar *arg)
 }
 
 
+void Item_func::replace_argument(THD *thd, Item **oldpp, Item *newp)
+{
+  thd->change_item_tree(oldpp, newp);
+}
+
+
 double Item_int_func::val_real()
 {
   DBUG_ASSERT(fixed == 1);

sql/item_func.h

@@ -453,6 +453,12 @@ class Item_func :public Item_result_field
   }
   virtual Item *gc_subst_transformer(uchar *arg);
 
+  /**
+    Does essentially the same as THD::change_item_tree, plus
+    maintains any necessary any invariants.
+  */
+  virtual void replace_argument(THD *thd, Item **oldpp, Item *newp);
+
 protected:
   /**
     Whether or not an item should contribute to the filtering effect

sql/item_subselect.cc

@@ -1397,7 +1397,7 @@ bool Item_in_subselect::test_limit()
 Item_in_subselect::Item_in_subselect(Item * left_exp,
 				     st_select_lex *select):
   Item_exists_subselect(), left_expr(left_exp), left_expr_cache(NULL),
-  left_expr_cache_filled(false), need_expr_cache(TRUE), expr(NULL),
+  left_expr_cache_filled(false), need_expr_cache(TRUE), m_injected_left_expr(NULL),
   optimizer(NULL), was_null(FALSE), abort_on_null(FALSE),
   in2exists_info(NULL), pushed_cond_guards(NULL), upper_item(NULL)
 {
@@ -1415,7 +1415,7 @@ Item_in_subselect::Item_in_subselect(Item * left_exp,
 Item_in_subselect::Item_in_subselect(const POS &pos, Item * left_exp,
 				     PT_subselect *pt_subselect_arg)
 : super(pos), left_expr(left_exp), left_expr_cache(NULL),
-  left_expr_cache_filled(false), need_expr_cache(TRUE), expr(NULL),
+  left_expr_cache_filled(false), need_expr_cache(TRUE), m_injected_left_expr(NULL),
   optimizer(NULL), was_null(FALSE), abort_on_null(FALSE),
   in2exists_info(NULL), pushed_cond_guards(NULL), upper_item(NULL),
   pt_subselect(pt_subselect_arg)
@@ -1873,7 +1873,7 @@ Item_in_subselect::single_value_transformer(SELECT_LEX *select,
       As far as  Item_ref_in_optimizer do not substitute itself on fix_fields
       we can use same item for all selects.
     */
-    Item_ref *const left=
+    Item_direct_ref *const left=
       new Item_direct_ref(&select->context, (Item**)optimizer->get_cache(),
 			 (char *)"<no matter>", (char *)in_left_expr_name);
     if (left == NULL)
@@ -1883,7 +1883,7 @@ Item_in_subselect::single_value_transformer(SELECT_LEX *select,
     if (!left_expr->const_item())
       left->depended_from= select->outer_select();
 
-    expr= left;
+    m_injected_left_expr= left;
 
     DBUG_ASSERT(in2exists_info == NULL);
     in2exists_info= new In2exists_info;
@@ -1965,7 +1965,7 @@ Item_in_subselect::single_value_in_to_exists_transformer(SELECT_LEX *select,
       select->group_list.elements)
   {
     bool tmp;
-    Item_bool_func *item= func->create(expr,
+    Item_bool_func *item= func->create(m_injected_left_expr,
                              new Item_ref_null_helper(&select->context,
                                                       this,
                                                       &select->ref_ptrs[0],
@@ -2012,7 +2012,7 @@ Item_in_subselect::single_value_in_to_exists_transformer(SELECT_LEX *select,
     if (select->table_list.elements || select->where_cond())
     {
       bool tmp;
-      Item_bool_func *item= func->create(expr, orig_item);
+      Item_bool_func *item= func->create(m_injected_left_expr, orig_item);
       /*
         We may soon add a 'OR inner IS NULL' to 'item', but that may later be
         removed if 'inner' is not nullable, so the in2exists mark must be on
@@ -2104,7 +2104,7 @@ Item_in_subselect::single_value_in_to_exists_transformer(SELECT_LEX *select,
           argument (reference) to fix_fields()
 	*/
         Item_bool_func *new_having=
-          func->create(expr,
+          func->create(m_injected_left_expr,
                        new Item_ref_null_helper(&select->context, this,
                                             &select->ref_ptrs[0],
                                             (char *)"<no matter>",

sql/item_subselect.h

@@ -386,7 +386,7 @@ class Item_in_subselect :public Item_exists_subselect
 public:
   Item *left_expr;
 protected:
-  /*
+  /**
     Cache of the left operand of the subquery predicate. Allocated in the
     runtime memory root, for each execution, thus need not be freed.
   */
@@ -395,13 +395,35 @@ class Item_in_subselect :public Item_exists_subselect
   /** The need for expr cache may be optimized away, @sa init_left_expr_cache. */
   bool need_expr_cache;
 
-  /*
-    expr & optimizer used in subselect rewriting to store Item for
-    all JOIN in UNION
+private:
+  /**
+    In the case of
+
+       x COMP_OP (SELECT1 UNION SELECT2 ...)
+
+    - the subquery transformation is done on SELECT1; this requires wrapping
+      'x' with more Item layers, and injecting that in a condition in SELECT1.
+
+    - the same transformation is done on SELECT2; but the wrapped 'x' doesn't
+      need to be created again, the one created for SELECT1 could be reused
+
+    - to achieve this, the wrapped 'x' is stored in member 'm_injected_left_expr'
+      when it is created for SELECT1, and is later reused for SELECT2.
+  */
+
+  /**
+    This will refer to a cached value which is reevaluated once for each candidate
+    row, cf. setup in ::single_value_transformer.
+  */
+  Item_direct_ref *m_injected_left_expr;
+
+  /**
+    Pointer to the created Item_in_optimizer; it is stored for the same
+    reasons as 'm_injected_left_expr'.
   */
-  Item *expr;
   Item_in_optimizer *optimizer;
   bool was_null;
+protected:
   bool abort_on_null;
 private:
   /**
@@ -471,7 +493,7 @@ class Item_in_subselect :public Item_exists_subselect
 
   Item_in_subselect()
     :Item_exists_subselect(), left_expr(NULL), left_expr_cache(NULL),
-    left_expr_cache_filled(false), need_expr_cache(TRUE), expr(NULL),
+    left_expr_cache_filled(false), need_expr_cache(TRUE), m_injected_left_expr(NULL),
     optimizer(NULL), was_null(FALSE), abort_on_null(FALSE),
     in2exists_info(NULL), pushed_cond_guards(NULL), upper_item(NULL)
   {}

sql/sql_resolver.cc

@@ -3486,7 +3486,8 @@ bool SELECT_LEX::change_group_ref(THD *thd, Item_func *expr, bool *changed)
           if (!(new_item= new Item_ref(&context, group->item, 0,
                                        item->item_name.ptr())))
             return true;              /* purecov: inspected */
-          thd->change_item_tree(arg, new_item);
+
+          expr->replace_argument(thd, arg, new_item);
           arg_changed= true;
         }
       }