bk commit into 5.0 tree (gkodinov:1.2400) BUG#24484

[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, 2007-02-23 12:31:35+02:00, gkodinov@stripped +6 -0
  Bug #24484:
  To correctly decide which predicates can be evaluated with a given table
  the optimizer must know the exact set of tables that a predicate depends 
  on. If that mask is too wide (refer to non-existing tables) the optimizer
  can erroneously skip a predicate.
  One such case of wrong table usage mask were the aggregate functions.
  The have a all-1 mask (meaning depend on all tables, including non-existent
  ones).
  Fixed by making a real used_tables mask for the aggregates. The mask is
  constructed in the following way :
  1. OR the table dependency masks of all the arguments of the aggregate.
  2. If all the arguments of the function are from the local name resolution 
    context; it's not a MAX/MIN and it is evaluated in the same name resolution
    context where it is referenced all the tables from that name resolution 
    context are OR-ed to the dependency mask. This is to denote that an
    aggregate function depends on the number of rows it processes.
  3. Handle correctly the case of an aggregate function optimization (such that
    the aggregate function can be pre-calculated and made a constant).
  
  Made sure that an aggregate function is never a constant (unless subject of a 
  specific optimization and pre-calculation).  
  
  One other flaw was revealed and fixed in the process : references were 
  not calling the recalculation method for used_tables of their targets.

  mysql-test/r/subselect3.result@stripped, 2007-02-23 12:31:25+02:00, gkodinov@stripped +23
-0
    Bug #24484: test case

  mysql-test/t/subselect3.test@stripped, 2007-02-23 12:31:25+02:00, gkodinov@stripped +22 -0
    Bug #24484: test case

  sql/item.h@stripped, 2007-02-23 12:31:26+02:00, gkodinov@stripped +5 -0
    Bug #24484: Item_ref must update the used tables.

  sql/item_sum.cc@stripped, 2007-02-23 12:31:27+02:00, gkodinov@stripped +49 -7
    Bug #24484: correct calculation of used_tables for aggregates.

  sql/item_sum.h@stripped, 2007-02-23 12:31:28+02:00, gkodinov@stripped +37 -23
    Bug #24484: correct calculation of used_tables for aggregates.

  sql/sql_select.cc@stripped, 2007-02-23 12:31:28+02:00, gkodinov@stripped +11 -8
    Bug #24484: correct check for aggregates

# 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/B24484-5.0

--- 1.219/sql/item.h	2007-01-26 19:33:16 +02:00
+++ 1.220/sql/item.h	2007-02-23 12:31:26 +02:00
@@ -1881,6 +1881,11 @@ public:
   { 
     return depended_from ? OUTER_REF_TABLE_BIT : (*ref)->used_tables(); 
   }
+  void update_used_tables() 
+  { 
+    if (!depended_from) 
+      (*ref)->update_used_tables(); 
+  }
   table_map not_null_tables() const { return (*ref)->not_null_tables(); }
   void set_result_field(Field *field)	{ result_field= field; }
   bool is_result_field() { return 1; }

--- 1.194/sql/item_sum.cc	2006-12-27 07:28:20 +02:00
+++ 1.195/sql/item_sum.cc	2007-02-23 12:31:27 +02:00
@@ -63,6 +63,7 @@ bool Item_sum::init_sum_func_check(THD *
   nest_level= thd->lex->current_select->nest_level;
   ref_by= 0;
   aggr_level= -1;
+  aggr_sl= NULL;
   max_arg_level= -1;
   max_sum_func_level= -1;
   return FALSE;
@@ -150,7 +151,10 @@ bool Item_sum::check_sum_func(THD *thd, 
     invalid= aggr_level < 0 && !(allow_sum_func & (1 <<
nest_level));
   }
   if (!invalid && aggr_level < 0)
+  {
     aggr_level= nest_level;
+    aggr_sl= thd->lex->current_select;
+  }
   /*
     By this moment we either found a subquery where the set function is
     to be aggregated  and assigned a value that is  >= 0 to aggr_level,
@@ -176,6 +180,7 @@ bool Item_sum::check_sum_func(THD *thd, 
     */
     set_if_bigger(in_sum_func->max_sum_func_level, aggr_level);
   }
+  update_used_tables();
   thd->lex->in_sum_func= in_sum_func;
   return FALSE;
 }
@@ -210,7 +215,7 @@ bool Item_sum::check_sum_func(THD *thd, 
 bool Item_sum::register_sum_func(THD *thd, Item **ref)
 {
   SELECT_LEX *sl;
-  SELECT_LEX *aggr_sl= NULL;
+  aggr_sl= NULL;
   nesting_map allow_sum_func= thd->lex->allow_sum_func;
   for (sl= thd->lex->current_select->master_unit()->outer_select() ;
        sl && sl->nest_level > max_arg_level;
@@ -271,8 +276,8 @@ bool Item_sum::register_sum_func(THD *th
 }
 
 
-Item_sum::Item_sum(List<Item> &list)
-  :arg_count(list.elements)
+Item_sum::Item_sum(List<Item> &list) :arg_count(list.elements), 
+  forced_const(FALSE)
 {
   if ((args=(Item**) sql_alloc(sizeof(Item*)*arg_count)))
   {
@@ -296,7 +301,8 @@ Item_sum::Item_sum(List<Item> &list)
 
 Item_sum::Item_sum(THD *thd, Item_sum *item):
   Item_result_field(thd, item), arg_count(item->arg_count),
-  quick_group(item->quick_group)
+  quick_group(item->quick_group), used_tables_cache(item->used_tables_cache),
+  forced_const(item->forced_const), aggr_sl(item->aggr_sl)
 {
   if (arg_count <= 2)
     args=tmp_args;
@@ -425,6 +431,26 @@ case DECIMAL_RESULT:
 }
 
 
+void Item_sum::update_used_tables ()
+{
+  DBUG_ASSERT (aggr_sl);
+  if (!forced_const)
+  {
+    used_tables_cache= 0;
+    for (uint i=0 ; i < arg_count ; i++)
+    {
+      args[i]->update_used_tables();
+      used_tables_cache|= args[i]->used_tables();
+    }
+
+    if (!(used_tables_cache & OUTER_REF_TABLE_BIT) ||
+        used_tables_cache & ~PSEUDO_TABLE_BITS)
+      /* the aggregate function is aggregated into its local context */
+      used_tables_cache|= (1 << aggr_sl->join->tables) - 1;
+  }
+}
+
+
 String *
 Item_sum_num::val_str(String *str)
 {
@@ -484,7 +510,7 @@ Item_sum_num::fix_fields(THD *thd, Item 
 Item_sum_hybrid::Item_sum_hybrid(THD *thd, Item_sum_hybrid *item)
   :Item_sum(thd, item), value(item->value), hybrid_type(item->hybrid_type),
   hybrid_field_type(item->hybrid_field_type), cmp_sign(item->cmp_sign),
-  used_table_cache(item->used_table_cache), was_values(item->was_values)
+  was_values(item->was_values)
 {
   /* copy results from old value */
   switch (hybrid_type) {
@@ -1072,7 +1098,6 @@ void Item_sum_count::cleanup()
   DBUG_ENTER("Item_sum_count::cleanup");
   count= 0;
   Item_sum_int::cleanup();
-  used_table_cache= ~(table_map) 0;
   DBUG_VOID_RETURN;
 }
 
@@ -1553,7 +1578,7 @@ void Item_sum_hybrid::cleanup()
 {
   DBUG_ENTER("Item_sum_hybrid::cleanup");
   Item_sum::cleanup();
-  used_table_cache= ~(table_map) 0;
+  forced_const= FALSE;
 
   /*
     by default it is TRUE to avoid TRUE reporting by
@@ -2157,6 +2182,23 @@ Item_sum_hybrid::min_max_update_decimal_
   else if (result_field->is_null(0))
     result_field->set_null();
   result_field->store_decimal(old_nr);
+}
+
+
+void Item_sum_hybrid::update_used_tables()
+{
+  if (!forced_const)
+  {
+    /*
+      MIN and MAX depend only on their arguments (as normal functions do).
+    */
+    used_tables_cache= 0;
+    for (uint i=0 ; i < arg_count ; i++)
+    {
+      args[i]->update_used_tables();
+      used_tables_cache|=args[i]->used_tables();
+    }
+  }
 }
 
 

--- 1.110/sql/item_sum.h	2007-01-11 16:40:02 +02:00
+++ 1.111/sql/item_sum.h	2007-02-23 12:31:28 +02:00
@@ -215,7 +215,9 @@
   TODO: to catch queries where the limit is exceeded to make the
   code clean here.  
     
-*/        
+*/ 
+
+class st_select_lex;
 
 class Item_sum :public Item_result_field
 {
@@ -237,19 +239,26 @@ public:
   int8 max_sum_func_level;/* max level of aggregation for embedded functions */
   bool quick_group;			/* If incremental update of fields */
 
+protected:  
+  table_map used_tables_cache;
+  bool forced_const;
+  st_select_lex *aggr_sl;
+
+public:  
+
   void mark_as_sum_func();
-  Item_sum() :arg_count(0), quick_group(1) 
+  Item_sum() :arg_count(0), quick_group(1), forced_const(FALSE)
   {
     mark_as_sum_func();
   }
-  Item_sum(Item *a)
-    :args(tmp_args), arg_count(1), quick_group(1)
+  Item_sum(Item *a) :args(tmp_args), arg_count(1), quick_group(1), 
+    forced_const(FALSE)
   {
     args[0]=a;
     mark_as_sum_func();
   }
-  Item_sum( Item *a, Item *b )
-    :args(tmp_args), arg_count(2), quick_group(1)
+  Item_sum( Item *a, Item *b ) :args(tmp_args), arg_count(2), quick_group(1),
+    forced_const(FALSE)
   {
     args[0]=a; args[1]=b;
     mark_as_sum_func();
@@ -319,10 +328,20 @@ public:
   virtual const char *func_name() const= 0;
   virtual Item *result_item(Field *field)
     { return new Item_field(field); }
-  table_map used_tables() const { return ~(table_map) 0; } /* Not used */
-  bool const_item() const { return 0; }
+  table_map used_tables() const { return used_tables_cache; }
+  void update_used_tables ();
+  void cleanup() 
+  { 
+    Item::cleanup();
+    forced_const= FALSE; 
+  }
   bool is_null() { return null_value; }
-  void update_used_tables() { }
+  void make_const () 
+  { 
+    used_tables_cache= 0; 
+    forced_const= TRUE; 
+  }
+  virtual bool const_item() const { return forced_const; }
   void make_field(Send_field *field);
   void print(String *str);
   void fix_num_length_and_dec();
@@ -509,23 +528,23 @@ public:
 class Item_sum_count :public Item_sum_int
 {
   longlong count;
-  table_map used_table_cache;
 
   public:
   Item_sum_count(Item *item_par)
-    :Item_sum_int(item_par),count(0),used_table_cache(~(table_map) 0)
+    :Item_sum_int(item_par),count(0)
   {}
   Item_sum_count(THD *thd, Item_sum_count *item)
-    :Item_sum_int(thd, item), count(item->count),
-     used_table_cache(item->used_table_cache)
+    :Item_sum_int(thd, item), count(item->count)
   {}
-  table_map used_tables() const { return used_table_cache; }
-  bool const_item() const { return !used_table_cache; }
   enum Sumfunctype sum_func () const { return COUNT_FUNC; }
   void clear();
   void no_rows_in_result() { count=0; }
   bool add();
-  void make_const(longlong count_arg) { count=count_arg; used_table_cache=0; }
+  void make_const(longlong count_arg) 
+  { 
+    count=count_arg;
+    Item_sum::make_const();
+  }
   longlong val_int();
   void reset_field();
   void cleanup();
@@ -805,28 +824,23 @@ protected:
   Item_result hybrid_type;
   enum_field_types hybrid_field_type;
   int cmp_sign;
-  table_map used_table_cache;
   bool was_values;  // Set if we have found at least one row (for max/min only)
 
   public:
   Item_sum_hybrid(Item *item_par,int sign)
     :Item_sum(item_par), sum(0.0), sum_int(0),
     hybrid_type(INT_RESULT), hybrid_field_type(FIELD_TYPE_LONGLONG),
-    cmp_sign(sign), used_table_cache(~(table_map) 0),
-    was_values(TRUE)
+    cmp_sign(sign), was_values(TRUE)
   { collation.set(&my_charset_bin); }
   Item_sum_hybrid(THD *thd, Item_sum_hybrid *item);
   bool fix_fields(THD *, Item **);
-  table_map used_tables() const { return used_table_cache; }
-  bool const_item() const { return !used_table_cache; }
-
+  void update_used_tables();
   void clear();
   double val_real();
   longlong val_int();
   my_decimal *val_decimal(my_decimal *);
   void reset_field();
   String *val_str(String *);
-  void make_const() { used_table_cache=0; }
   bool keep_field_type(void) const { return 1; }
   enum Item_result result_type () const { return hybrid_type; }
   enum enum_field_types field_type() const { return hybrid_field_type; }

--- 1.488/sql/sql_select.cc	2007-02-01 10:50:33 +02:00
+++ 1.489/sql/sql_select.cc	2007-02-23 12:31:28 +02:00
@@ -346,8 +346,15 @@ JOIN::prepare(Item ***rref_pointer_array
        setup_tables_and_check_access(thd, &select_lex->context, join_list,
                                      tables_list, &conds, 
                                      &select_lex->leaf_tables, FALSE, 
-                                     SELECT_ACL, SELECT_ACL)) ||
-      setup_wild(thd, tables_list, fields_list, &all_fields, wild_num) ||
+                                     SELECT_ACL, SELECT_ACL)))
+    DBUG_RETURN(-1);
+
+  TABLE_LIST *table_ptr;
+  for (table_ptr= select_lex->leaf_tables, tables=0; table_ptr;
+       table_ptr= table_ptr->next_leaf)
+    tables++;
+
+  if (setup_wild(thd, tables_list, fields_list, &all_fields, wild_num) ||
       select_lex->setup_ref_array(thd, og_num) ||
       setup_fields(thd, (*rref_pointer_array), fields_list, 1,
 		   &all_fields, 1) ||
@@ -437,11 +444,6 @@ JOIN::prepare(Item ***rref_pointer_array
 	DBUG_RETURN(-1);
       }
     }
-    TABLE_LIST *table_ptr;
-    for (table_ptr= select_lex->leaf_tables;
-	 table_ptr;
-	 table_ptr= table_ptr->next_leaf)
-      tables++;
   }
   {
     /* Caclulate the number of groups */
@@ -6376,7 +6378,8 @@ static void update_depend_map(JOIN *join
     order->item[0]->update_used_tables();
     order->depend_map=depend_map=order->item[0]->used_tables();
     // Not item_sum(), RAND() and no reference to table outside of sub select
-    if (!(order->depend_map & (OUTER_REF_TABLE_BIT | RAND_TABLE_BIT)))
+    if (!(order->depend_map & (OUTER_REF_TABLE_BIT | RAND_TABLE_BIT))
+        && !order->item[0]->with_sum_func)
     {
       for (JOIN_TAB **tab=join->map2table;
 	   depend_map ;

--- 1.4/mysql-test/r/subselect3.result	2007-01-27 03:10:42 +02:00
+++ 1.5/mysql-test/r/subselect3.result	2007-02-23 12:31:25 +02:00
@@ -645,3 +645,26 @@ a	b	Z
 2	2	0
 3	3	1
 drop table t1,t2;
+CREATE TABLE t1 (a int, b INT, c CHAR(10) NOT NULL, PRIMARY KEY (a, b));
+INSERT INTO t1 VALUES (1,1,'a'), (1,2,'b'), (1,3,'c'), (1,4,'d'), (1,5,'e'),
+(2,1,'f'), (2,2,'g'), (2,3,'h'), (3,4,'i'),(3,3,'j'), (3,2,'k'), (3,1,'l'),
+(1,9,'m');
+CREATE TABLE t2 (a int, b INT, c CHAR(10) NOT NULL, PRIMARY KEY (a, b));
+INSERT INTO t2 SELECT * FROM t1;
+SELECT a, MAX(b), (SELECT t.c FROM t1 AS t WHERE t1.a=t.a AND t.b=MAX(t1.b))
+as test FROM t1 GROUP BY a;
+a	MAX(b)	test
+1	9	m
+2	3	h
+3	4	i
+SELECT * FROM t1 GROUP by t1.a
+HAVING (MAX(t1.b) > (SELECT MAX(t2.b) FROM t2 WHERE t2.c < t1.c
+HAVING MAX(t2.b+t1.a) < 10));
+a	b	c
+SELECT a, AVG(b), (SELECT t.c FROM t1 AS t WHERE t1.a=t.a AND t.b=AVG(t1.b))
+AS test FROM t1 GROUP BY a;
+a	AVG(b)	test
+1	4.0000	NULL
+2	2.0000	k
+3	2.5000	NULL
+DROP TABLE t1, t2;

--- 1.5/mysql-test/t/subselect3.test	2007-01-27 03:10:42 +02:00
+++ 1.6/mysql-test/t/subselect3.test	2007-02-23 12:31:25 +02:00
@@ -489,3 +489,25 @@ select a, b, (a,b) in (select a, min(b) 
 
 drop table t1,t2;
 
+#
+# Bug #24484: Aggregate function used in column list subquery gives erroneous 
+# error
+#
+CREATE TABLE t1 (a int, b INT, c CHAR(10) NOT NULL, PRIMARY KEY (a, b));
+INSERT INTO t1 VALUES (1,1,'a'), (1,2,'b'), (1,3,'c'), (1,4,'d'), (1,5,'e'),
+  (2,1,'f'), (2,2,'g'), (2,3,'h'), (3,4,'i'),(3,3,'j'), (3,2,'k'), (3,1,'l'),
+  (1,9,'m');
+CREATE TABLE t2 (a int, b INT, c CHAR(10) NOT NULL, PRIMARY KEY (a, b));
+INSERT INTO t2 SELECT * FROM t1;
+
+# Gives error, but should work since it is (a, b) is the PK so only one 
+# given match possible
+SELECT a, MAX(b), (SELECT t.c FROM t1 AS t WHERE t1.a=t.a AND t.b=MAX(t1.b))
+  as test FROM t1 GROUP BY a;
+SELECT * FROM t1 GROUP by t1.a
+  HAVING (MAX(t1.b) > (SELECT MAX(t2.b) FROM t2 WHERE t2.c < t1.c
+                                                HAVING MAX(t2.b+t1.a) < 10));
+SELECT a, AVG(b), (SELECT t.c FROM t1 AS t WHERE t1.a=t.a AND t.b=AVG(t1.b))
+  AS test FROM t1 GROUP BY a;
+
+DROP TABLE t1, t2;