bk commit into 5.0 tree (gkodinov:1.2284) BUG#23184

[kgeorge]

Below is the list of changes that have just been committed into a local
5.0 repository of kgeorge. When kgeorge does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2006-10-24 18:33:32+03:00, gkodinov@stripped +4 -0
  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.

  mysql-test/r/func_group.result@stripped, 2006-10-24 18:33:18+03:00, gkodinov@stripped +26
-0
    Bug #23184: SELECT causes server crash
     - test case

  mysql-test/t/func_group.test@stripped, 2006-10-24 18:33:19+03:00, gkodinov@stripped +25
-0
    Bug #23184: SELECT causes server crash
     - test case

  sql/item_sum.cc@stripped, 2006-10-24 18:33:20+03:00, gkodinov@stripped +36 -12
    Bug #23184: SELECT causes server crash
     - caching of the aggreagte function results so no need to recalculate at val_xxx()

  sql/item_sum.h@stripped, 2006-10-24 18:33:20+03:00, gkodinov@stripped +26 -8
    Bug #23184: SELECT causes server crash
     - caching of the aggreagte function results so no need to recalculate at val_xxx()

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	gkodinov
# Host:	macbook.gmz
# Root:	/Users/kgeorge/mysql/work/B23184-5.0-opt

--- 1.181/sql/item_sum.cc	2006-10-24 18:33:52 +03:00
+++ 1.182/sql/item_sum.cc	2006-10-24 18:33:52 +03:00
@@ -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);
 
+  aggregate_cache_valid= 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);
+  aggregate_cache_valid= 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();
+  aggregate_cache_valid= FALSE;
   DBUG_VOID_RETURN;
 }
 
@@ -938,6 +941,7 @@ void Item_sum_distinct::cleanup()
   delete tree;
   tree= 0;
   table= 0;
+  aggregate_cache_valid= 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 (!aggregate_cache_valid)
   {
-    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);
+    }
+    aggregate_cache_valid= TRUE;
   }
 }
 
@@ -1014,9 +1022,13 @@ Item_sum_avg_distinct::fix_length_and_de
 void
 Item_sum_avg_distinct::calculate_val_and_count()
 {
-  Item_sum_distinct::calculate_val_and_count();
-  if (count)
-    val.traits->div(&val, count);
+  if (!aggregate_cache_valid)
+  {
+    Item_sum_distinct::calculate_val_and_count();
+    if (count)
+      val.traits->div(&val, count);
+    aggregate_cache_valid= TRUE;
+  }
 }
 
 
@@ -2477,6 +2489,7 @@ void Item_sum_count_distinct::cleanup()
     */
     delete tree;
     tree= 0;
+    aggregate_cache_valid= FALSE;
     if (table)
     {
       free_tmp_table(table->in_use, table);
@@ -2498,6 +2511,7 @@ void Item_sum_count_distinct::make_uniqu
   original= 0;
   force_copy_fields= 1;
   tree= 0;
+  aggregate_cache_valid= FALSE;
   tmp_table_param= 0;
   always_null= FALSE;
 }
@@ -2617,6 +2631,7 @@ bool Item_sum_count_distinct::setup(THD 
       but this has to be handled - otherwise someone can crash
       the server with a DoS attack
     */
+    aggregate_cache_valid= FALSE;
     if (! tree)
       return TRUE;
   }
@@ -2633,8 +2648,11 @@ Item *Item_sum_count_distinct::copy_or_s
 void Item_sum_count_distinct::clear()
 {
   /* tree and table can be both null only if always_null */
+  aggregate_cache_valid= 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
 
+  aggregate_cache_valid= FALSE;
   if (tree)
   {
     /*
@@ -2682,10 +2701,15 @@ longlong Item_sum_count_distinct::val_in
   {
     ulonglong count;
 
+    if (aggregate_cache_valid)
+      return count_cache;
+
     if (tree->elements == 0)
       return (longlong) tree->elements_in_tree(); // everything fits in memory
     count= 0;
     tree->walk(count_distinct_walk, (void*) &count);
+    aggregate_cache_valid= TRUE;
+    count_cache= count;
     return (longlong) count;
   }
   table->file->info(HA_STATUS_VARIABLE | HA_STATUS_NO_LOCK);

--- 1.103/sql/item_sum.h	2006-10-24 18:33:53 +03:00
+++ 1.104/sql/item_sum.h	2006-10-24 18:33:53 +03:00
@@ -321,12 +321,23 @@ public:
 
 class Item_sum_num :public Item_sum
 {
+protected:
+  /*
+   val_xxx() functions may be called several times during the execution of a 
+   query. Derived classes that require extensive calculation in val_xxx()
+   maintain cache of aggregate value. This variable governs the validity of 
+   that cache.
+  */
+  bool aggregate_cache_valid;
 public:
-  Item_sum_num() :Item_sum() {}
-  Item_sum_num(Item *item_par) :Item_sum(item_par) {}
-  Item_sum_num(Item *a, Item* b) :Item_sum(a,b) {}
-  Item_sum_num(List<Item> &list) :Item_sum(list) {}
-  Item_sum_num(THD *thd, Item_sum_num *item) :Item_sum(thd, item) {}
+  Item_sum_num() :Item_sum(),aggregate_cache_valid(FALSE) {}
+  Item_sum_num(Item *item_par) 
+    :Item_sum(item_par), aggregate_cache_valid(FALSE) {}
+  Item_sum_num(Item *a, Item* b) :Item_sum(a,b),aggregate_cache_valid(FALSE) {}
+  Item_sum_num(List<Item> &list) 
+    :Item_sum(list), aggregate_cache_valid(FALSE) {}
+  Item_sum_num(THD *thd, Item_sum_num *item) 
+    :Item_sum(thd, item),aggregate_cache_valid(item->aggregate_cache_valid) {}
   bool fix_fields(THD *, Item **);
   longlong val_int()
   {
@@ -509,6 +520,12 @@ class Item_sum_count_distinct :public It
   */
   Unique *tree;
   /*
+   Cache for the value of count between calls to val_int() so val_int()
+   will not recalculate on each call. Validitiy of the value is stored in
+   aggregate_cache_valid.
+  */
+  longlong count_cache;
+  /*
     Following is 0 normal object and pointer to original one for copy 
     (to correctly free resources)
   */
@@ -525,14 +542,15 @@ class Item_sum_count_distinct :public It
 public:
   Item_sum_count_distinct(List<Item> &list)
     :Item_sum_int(list), table(0), field_lengths(0), tmp_table_param(0),
-     force_copy_fields(0), tree(0), original(0), always_null(FALSE)
+     force_copy_fields(0), tree(0), count_cache(0),
+     original(0), always_null(FALSE)
   { quick_group= 0; }
   Item_sum_count_distinct(THD *thd, Item_sum_count_distinct *item)
     :Item_sum_int(thd, item), table(item->table),
      field_lengths(item->field_lengths),
      tmp_table_param(item->tmp_table_param),
-     force_copy_fields(0), tree(item->tree), original(item),
-     tree_key_length(item->tree_key_length),
+     force_copy_fields(0), tree(item->tree), count_cache(item->count_cache), 
+     original(item), tree_key_length(item->tree_key_length),
      always_null(item->always_null)
   {}
   ~Item_sum_count_distinct();

--- 1.53/mysql-test/r/func_group.result	2006-10-24 18:33:53 +03:00
+++ 1.54/mysql-test/r/func_group.result	2006-10-24 18:33:53 +03:00
@@ -1029,3 +1029,29 @@ t1	CREATE TABLE `t1` (
   `stddev(0)` double(8,4) default NULL
 ) ENGINE=MyISAM DEFAULT CHARSET=latin1
 drop table t1;
+CREATE TABLE t1 (a INT, b INT);
+INSERT INTO t1 VALUES (1,1),(1,2),(1,3),(1,4),(1,5),(1,6),(1,7),(1,8);
+INSERT INTO t1 SELECT a, b+8       FROM t1;
+INSERT INTO t1 SELECT a, b+16      FROM t1;
+INSERT INTO t1 SELECT a, b+32      FROM t1;
+INSERT INTO t1 SELECT a, b+64      FROM t1;
+INSERT INTO t1 SELECT a, b+128     FROM t1;
+INSERT INTO t1 SELECT a, b+256     FROM t1;
+INSERT INTO t1 SELECT a, b+512     FROM t1;
+INSERT INTO t1 SELECT a, b+1024    FROM t1;
+INSERT INTO t1 SELECT a, b+2048    FROM t1;
+INSERT INTO t1 SELECT a, b+4096    FROM t1;
+INSERT INTO t1 SELECT a, b+8192    FROM t1;
+INSERT INTO t1 SELECT a, b+16384   FROM t1;
+INSERT INTO t1 SELECT a, b+32768   FROM t1;
+SELECT a,COUNT(DISTINCT b) AS cnt FROM t1 GROUP BY a HAVING cnt > 50;
+a	cnt
+1	65536
+SELECT a,SUM(DISTINCT b) AS cnt FROM t1 GROUP BY a HAVING cnt > 50;
+a	cnt
[phone]
+SELECT a,AVG(DISTINCT b) AS cnt FROM t1 GROUP BY a HAVING cnt > 50;
+a	cnt
+1	32768.5000
+DROP TABLE t1;
+End of 5.0 tests

--- 1.49/mysql-test/t/func_group.test	2006-10-24 18:33:53 +03:00
+++ 1.50/mysql-test/t/func_group.test	2006-10-24 18:33:53 +03:00
@@ -700,3 +700,28 @@ create table t1 select stddev(0);
 show create table t1;
 drop table t1;
  
+#
+# Bug #23184: SELECT causes server crash
+# 
+CREATE TABLE t1 (a INT, b INT);
+INSERT INTO t1 VALUES (1,1),(1,2),(1,3),(1,4),(1,5),(1,6),(1,7),(1,8);
+INSERT INTO t1 SELECT a, b+8       FROM t1;
+INSERT INTO t1 SELECT a, b+16      FROM t1;
+INSERT INTO t1 SELECT a, b+32      FROM t1;
+INSERT INTO t1 SELECT a, b+64      FROM t1;
+INSERT INTO t1 SELECT a, b+128     FROM t1;
+INSERT INTO t1 SELECT a, b+256     FROM t1;
+INSERT INTO t1 SELECT a, b+512     FROM t1;
+INSERT INTO t1 SELECT a, b+1024    FROM t1;
+INSERT INTO t1 SELECT a, b+2048    FROM t1;
+INSERT INTO t1 SELECT a, b+4096    FROM t1;
+INSERT INTO t1 SELECT a, b+8192    FROM t1;
+INSERT INTO t1 SELECT a, b+16384   FROM t1;
+INSERT INTO t1 SELECT a, b+32768   FROM t1;
+SELECT a,COUNT(DISTINCT b) AS cnt FROM t1 GROUP BY a HAVING cnt > 50;
+SELECT a,SUM(DISTINCT b) AS cnt FROM t1 GROUP BY a HAVING cnt > 50;
+SELECT a,AVG(DISTINCT b) AS cnt FROM t1 GROUP BY a HAVING cnt > 50;
+
+DROP TABLE t1;
+
+--echo End of 5.0 tests