Bug #23184: SELECT causes server crash

 Item::val_xxx() may be called by the server several times at execute time 
 for a single query. Calls to val_xxx() may be very expensive and sometimes
 (count(distinct), sum(distinct), avg(distinct)) not possible.
 To avoid that problem the results of calculation for these aggregate 
 functions are cached so that val_xxx() methods just return the calculated 
 value for the second and subsequent calls.

1 files changed, 34 insertions, 13 deletions

diff --git a/sql/item_sum.cc b/sql/item_sum.cc
index 5ca1dbba94b..fccbdfa7925 100644
--- a/sql/item_sum.cc
+++ b/sql/item_sum.cc
@@ -893,6 +893,7 @@ bool Item_sum_distinct::setup(THD *thd)
   tree= new Unique(simple_raw_key_cmp, &tree_key_length, tree_key_length,
                    thd->variables.max_heap_table_size);
 
+  is_evaluated= FALSE;
   DBUG_RETURN(tree == 0);
 }
 
@@ -900,6 +901,7 @@ bool Item_sum_distinct::setup(THD *thd)
 bool Item_sum_distinct::add()
 {
   args[0]->save_in_field(table->field[0], FALSE);
+  is_evaluated= FALSE;
   if (!table->field[0]->is_null())
   {
     DBUG_ASSERT(tree);
@@ -929,6 +931,7 @@ void Item_sum_distinct::clear()
   DBUG_ASSERT(tree != 0);                        /* we always have a tree */
   null_value= 1;
   tree->reset();
+  is_evaluated= FALSE;
   DBUG_VOID_RETURN;
 }
 
@@ -938,6 +941,7 @@ void Item_sum_distinct::cleanup()
   delete tree;
   tree= 0;
   table= 0;
+  is_evaluated= FALSE;
 }
 
 Item_sum_distinct::~Item_sum_distinct()
@@ -949,16 +953,20 @@ Item_sum_distinct::~Item_sum_distinct()
 
 void Item_sum_distinct::calculate_val_and_count()
 {
-  count= 0;
-  val.traits->set_zero(&val);
-  /*
-    We don't have a tree only if 'setup()' hasn't been called;
-    this is the case of sql_select.cc:return_zero_rows.
-  */
-  if (tree)
+  if (!is_evaluated)
   {
-    table->field[0]->set_notnull();
-    tree->walk(item_sum_distinct_walk, (void*) this);
+    count= 0;
+    val.traits->set_zero(&val);
+    /*
+      We don't have a tree only if 'setup()' hasn't been called;
+      this is the case of sql_select.cc:return_zero_rows.
+     */
+    if (tree)
+    {
+      table->field[0]->set_notnull();
+      tree->walk(item_sum_distinct_walk, (void*) this);
+    }
+    is_evaluated= TRUE;
   }
 }
 
@@ -1014,9 +1022,13 @@ Item_sum_avg_distinct::fix_length_and_dec()
 void
 Item_sum_avg_distinct::calculate_val_and_count()
 {
-  Item_sum_distinct::calculate_val_and_count();
-  if (count)
-    val.traits->div(&val, count);
+  if (!is_evaluated)
+  {
+    Item_sum_distinct::calculate_val_and_count();
+    if (count)
+      val.traits->div(&val, count);
+    is_evaluated= TRUE;
+  }
 }
 
 
@@ -2477,6 +2489,7 @@ void Item_sum_count_distinct::cleanup()
     */
     delete tree;
     tree= 0;
+    is_evaluated= FALSE;
     if (table)
     {
       free_tmp_table(table->in_use, table);
@@ -2498,6 +2511,7 @@ void Item_sum_count_distinct::make_unique()
   original= 0;
   force_copy_fields= 1;
   tree= 0;
+  is_evaluated= FALSE;
   tmp_table_param= 0;
   always_null= FALSE;
 }
@@ -2617,6 +2631,7 @@ bool Item_sum_count_distinct::setup(THD *thd)
       but this has to be handled - otherwise someone can crash
       the server with a DoS attack
     */
+    is_evaluated= FALSE;
     if (! tree)
       return TRUE;
   }
@@ -2633,8 +2648,11 @@ Item *Item_sum_count_distinct::copy_or_same(THD* thd)
 void Item_sum_count_distinct::clear()
 {
   /* tree and table can be both null only if always_null */
+  is_evaluated= FALSE;
   if (tree)
+  {
     tree->reset();
+  }
   else if (table)
   {
     table->file->extra(HA_EXTRA_NO_CACHE);
@@ -2655,6 +2673,7 @@ bool Item_sum_count_distinct::add()
     if ((*field)->is_real_null(0))
       return 0;					// Don't count NULL
 
+  is_evaluated= FALSE;
   if (tree)
   {
     /*
@@ -2680,12 +2699,14 @@ longlong Item_sum_count_distinct::val_int()
     return LL(0);
   if (tree)
   {
-    ulonglong count;
+    if (is_evaluated)
+      return count;
 
     if (tree->elements == 0)
       return (longlong) tree->elements_in_tree(); // everything fits in memory
     count= 0;
     tree->walk(count_distinct_walk, (void*) &count);
+    is_evaluated= TRUE;
     return (longlong) count;
   }
   table->file->info(HA_STATUS_VARIABLE | HA_STATUS_NO_LOCK);