Bug#48060 Memory leak - Item::val_bool() (item.cc:184) from optimizer_subquery grammar

Item_sum::set_aggregator() may be called multiple times during query preparation.
On subsequent calls: verify that the aggregator type is the same,
and re-use the existing Aggregator.


sql/item_sum.cc:
  In Item_sum::set_aggregator(): re-use existing Aggregator if already set.
  
  Remove some friend declarations, add some accessor functions.
  Cleanup some DBUG_ENTER and DBUG_RETURN code.
sql/item_sum.h:
  Make some member fields private, add accessors instead.
  Remove some un-necessary friend declarations.
  Remove some default arguments from constructors.
sql/opt_sum.cc:
  Use accessor functions in Item_sum.
sql/sql_select.cc:
  Fix mis-spelled DBUG_ENTER text.
  Use accessor functions in Item_sum.
sql/sql_yacc.yy:
  Use explicit true/false rather than default arguments when constructing
  Item_sum_xxx objects.

sql/item_sum.cc

@@ -564,6 +564,11 @@ Item *Item_sum::set_arg(uint i, THD *thd, Item *new_val)
 
 int Item_sum::set_aggregator(Aggregator::Aggregator_type aggregator)
 {
+  if (aggr)
+  {
+    DBUG_ASSERT(aggregator == aggr->Aggrtype());
+    return FALSE;
+  }
   switch (aggregator)
   {
   case Aggregator::DISTINCT_AGGREGATOR:
@@ -736,12 +741,12 @@ bool Aggregator_distinct::setup(THD *thd)
       if (list.push_back(item))
         return TRUE;                              // End of memory
       if (item->const_item() && item->is_null())
-        always_null=1;
+        always_null= true;
     }
     if (always_null)
       return FALSE;
     count_field_types(select_lex, tmp_table_param, list, 0);
-    tmp_table_param->force_copy_fields= item_sum->force_copy_fields;
+    tmp_table_param->force_copy_fields= item_sum->has_force_copy_fields();
     DBUG_ASSERT(table == 0);
     /*
       Make create_tmp_table() convert BIT columns to BIGINT.
@@ -844,10 +849,10 @@ bool Aggregator_distinct::setup(THD *thd)
     List<Create_field> field_list;
     Create_field field_def;                              /* field definition */
     Item *arg;
-    DBUG_ENTER("Item_sum_distinct::setup");
+    DBUG_ENTER("Aggregator_distinct::setup");
     /* It's legal to call setup() more than once when in a subquery */
     if (tree)
-      return FALSE;
+      DBUG_RETURN(FALSE);
 
     /*
       Virtual table and the tree are created anew on each re-execution of
@@ -855,7 +860,7 @@ bool Aggregator_distinct::setup(THD *thd)
       mem_root.
     */
     if (field_list.push_back(&field_def))
-      return TRUE;
+      DBUG_RETURN(TRUE);
 
     item_sum->null_value= item_sum->maybe_null= 1;
     item_sum->quick_group= 0;
@@ -867,11 +872,11 @@ bool Aggregator_distinct::setup(THD *thd)
     {
       (void) arg->val_int();
       if (arg->null_value)
-        always_null=1;
+        always_null= true;
     }
 
     if (always_null)
-      return FALSE;
+      DBUG_RETURN(FALSE);
 
     enum enum_field_types field_type;
 
@@ -884,7 +889,7 @@ bool Aggregator_distinct::setup(THD *thd)
                                  arg->unsigned_flag);
 
     if (! (table= create_virtual_tmp_table(thd, field_list)))
-      return TRUE;
+      DBUG_RETURN(TRUE);
 
     /* XXX: check that the case of CHAR(0) works OK */
     tree_key_length= table->s->reclength - table->s->null_bytes;

sql/item_sum.h

@@ -302,13 +302,14 @@ class st_select_lex;
 
 class Item_sum :public Item_result_field
 {
-public:
+protected:
   /**
     Aggregator class instance. Not set initially. Allocated only after
     it is determined if the incoming data are already distinct.
   */
   Aggregator *aggr;
 
+private:
   /**
     Used in making ROLLUP. Set for the ROLLUP copies of the original
     Item_sum and passed to create_tmp_field() to cause it to work
@@ -324,6 +325,11 @@ public:
   */
   bool with_distinct;
 
+public:
+
+  bool has_force_copy_fields() const { return force_copy_fields; }
+  bool has_with_distinct()     const { return with_distinct; }
+
   enum Sumfunctype
   { COUNT_FUNC, COUNT_DISTINCT_FUNC, SUM_FUNC, SUM_DISTINCT_FUNC, AVG_FUNC,
     AVG_DISTINCT_FUNC, MIN_FUNC, MAX_FUNC, STD_FUNC,
@@ -447,7 +453,7 @@ public:
     may be initialized to 0 by clear() and to NULL by
     no_rows_in_result().
   */
-  void no_rows_in_result()
+  virtual void no_rows_in_result()
   {
     if (!aggr)
       set_aggregator(with_distinct ?
@@ -511,11 +517,12 @@ public:
   */
 
   int set_aggregator(Aggregator::Aggregator_type aggregator);
+
   virtual void clear()= 0;
   virtual bool add()= 0;
-  virtual bool setup(THD *thd) {return 0;}
+  virtual bool setup(THD *thd) { return false; }
 
-  void cleanup ();
+  virtual void cleanup();
 };
 
 
@@ -533,9 +540,6 @@ class Unique;
 class Aggregator_distinct : public Aggregator
 {
   friend class Item_sum_sum;
-  friend class Item_sum_count;
-  friend class Item_sum_avg;
-protected:
 
   /* 
     flag to prevent consecutive runs of endup(). Normally in endup there are 
@@ -567,7 +571,7 @@ protected:
   uint32 *field_lengths;
 
   /*
-    used in conjunction with 'table' to support the access to Field classes 
+    Used in conjunction with 'table' to support the access to Field classes 
     for COUNT(DISTINCT). Needed by copy_fields()/copy_funcs().
   */
   TMP_TABLE_PARAM *tmp_table_param;
@@ -637,7 +641,6 @@ public:
 
 class Item_sum_num :public Item_sum
 {
-  friend class Aggregator_distinct;
 protected:
   /*
    val_xxx() functions may be called several times during the execution of a 
@@ -692,14 +695,14 @@ protected:
   void fix_length_and_dec();
 
 public:
-  Item_sum_sum(Item *item_par, bool distinct= FALSE) :Item_sum_num(item_par) 
+  Item_sum_sum(Item *item_par, bool distinct) :Item_sum_num(item_par) 
   {
     set_distinct(distinct);
   }
   Item_sum_sum(THD *thd, Item_sum_sum *item);
   enum Sumfunctype sum_func () const 
   { 
-    return with_distinct ? SUM_DISTINCT_FUNC : SUM_FUNC; 
+    return has_with_distinct() ? SUM_DISTINCT_FUNC : SUM_FUNC; 
   }
   void clear();
   bool add();
@@ -713,7 +716,7 @@ public:
   void no_rows_in_result() {}
   const char *func_name() const 
   { 
-    return with_distinct ? "sum(distinct " : "sum("; 
+    return has_with_distinct() ? "sum(distinct " : "sum("; 
   }
   Item *copy_or_same(THD* thd);
 };
@@ -752,7 +755,7 @@ class Item_sum_count :public Item_sum_int
   {}
   enum Sumfunctype sum_func () const 
   { 
-    return with_distinct ? COUNT_DISTINCT_FUNC : COUNT_FUNC; 
+    return has_with_distinct() ? COUNT_DISTINCT_FUNC : COUNT_FUNC; 
   }
   void no_rows_in_result() { count=0; }
   void make_const(longlong count_arg) 
@@ -765,7 +768,7 @@ class Item_sum_count :public Item_sum_int
   void update_field();
   const char *func_name() const 
   { 
-    return with_distinct ? "count(distinct " : "count(";
+    return has_with_distinct() ? "count(distinct " : "count(";
   }
   Item *copy_or_same(THD* thd);
 };
@@ -806,7 +809,7 @@ public:
   uint prec_increment;
   uint f_precision, f_scale, dec_bin_size;
 
-  Item_sum_avg(Item *item_par, bool distinct= FALSE) 
+  Item_sum_avg(Item *item_par, bool distinct) 
     :Item_sum_sum(item_par, distinct), count(0) 
   {}
   Item_sum_avg(THD *thd, Item_sum_avg *item)
@@ -816,7 +819,7 @@ public:
   void fix_length_and_dec();
   enum Sumfunctype sum_func () const 
   {
-    return with_distinct ? AVG_DISTINCT_FUNC : AVG_FUNC;
+    return has_with_distinct() ? AVG_DISTINCT_FUNC : AVG_FUNC;
   }
   void clear();
   bool add();
@@ -832,7 +835,7 @@ public:
   void no_rows_in_result() {}
   const char *func_name() const 
   { 
-    return with_distinct ? "avg(distinct " : "avg("; 
+    return has_with_distinct() ? "avg(distinct " : "avg("; 
   }
   Item *copy_or_same(THD* thd);
   Field *create_tmp_field(bool group, TABLE *table, uint convert_blob_length);

sql/opt_sum.cc

@@ -356,7 +356,7 @@ int opt_sum_query(TABLE_LIST *tables, List<Item> &all_fields,COND *conds)
           const_result= 0;
           break;
         }
-        item_sum->set_aggregator (item_sum->with_distinct ? 
+        item_sum->set_aggregator(item_sum->has_with_distinct() ? 
                                  Aggregator::DISTINCT_AGGREGATOR :
                                  Aggregator::SIMPLE_AGGREGATOR);
         if (!count)
@@ -447,7 +447,7 @@ int opt_sum_query(TABLE_LIST *tables, List<Item> &all_fields,COND *conds)
           const_result= 0;
           break;
         }
-        item_sum->set_aggregator (item_sum->with_distinct ? 
+        item_sum->set_aggregator(item_sum->has_with_distinct() ? 
                                  Aggregator::DISTINCT_AGGREGATOR :
                                  Aggregator::SIMPLE_AGGREGATOR);
         if (!count)

sql/sql_select.cc

@@ -2094,8 +2094,10 @@ JOIN::exec()
 
     if (curr_join->make_sum_func_list(*curr_all_fields, *curr_fields_list,
 				      1, TRUE) || 
-        prepare_sum_aggregators (curr_join->sum_funcs, !curr_join->join_tab ||
+        prepare_sum_aggregators(curr_join->sum_funcs,
-                                !curr_join->join_tab->is_using_agg_loose_index_scan()) ||
+                                !curr_join->join_tab ||
+                                !curr_join->join_tab->
+                                  is_using_agg_loose_index_scan()) ||
         setup_sum_funcs(curr_join->thd, curr_join->sum_funcs) ||
         thd->is_fatal_error)
       DBUG_VOID_RETURN;
@@ -4008,7 +4010,7 @@ is_indexed_agg_distinct(JOIN *join, List<Item_field> *out_args)
       join->select_lex->olap == ROLLUP_TYPE)  /* Check (B3) for ROLLUP */
     return false;
 
-  if (join->make_sum_func_list(join->all_fields, join->fields_list, 1))
+  if (join->make_sum_func_list(join->all_fields, join->fields_list, true))
     return false;
 
   for (sum_item_ptr= join->sum_funcs; *sum_item_ptr; sum_item_ptr++)
@@ -15477,10 +15479,10 @@ static bool setup_sum_funcs(THD *thd, Item_sum **func_ptr)
 static bool prepare_sum_aggregators(Item_sum **func_ptr, bool need_distinct)
 {
   Item_sum *func;
-  DBUG_ENTER("setup_sum_funcs");
+  DBUG_ENTER("prepare_sum_aggregators");
   while ((func= *(func_ptr++)))
   {
-    if (func->set_aggregator(need_distinct && func->with_distinct ?
+    if (func->set_aggregator(need_distinct && func->has_with_distinct() ?
                              Aggregator::DISTINCT_AGGREGATOR :
                              Aggregator::SIMPLE_AGGREGATOR))
       DBUG_RETURN(TRUE);

sql/sql_yacc.yy

@@ -8249,7 +8249,7 @@ udf_expr:
 sum_expr:
           AVG_SYM '(' in_sum_expr ')'
           {
-            $$= new (YYTHD->mem_root) Item_sum_avg($3);
+            $$= new (YYTHD->mem_root) Item_sum_avg($3, FALSE);
             if ($$ == NULL)
               MYSQL_YYABORT;
           }
@@ -8357,7 +8357,7 @@ sum_expr:
           }
         | SUM_SYM '(' in_sum_expr ')'
           {
-            $$= new (YYTHD->mem_root) Item_sum_sum($3);
+            $$= new (YYTHD->mem_root) Item_sum_sum($3, FALSE);
             if ($$ == NULL)
               MYSQL_YYABORT;
           }