MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:mhansson Date:January 31 2008 6:07pm
Subject:bk commit into 5.0 tree (mhansson:1.2590) BUG#27219
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of mhansson.  When mhansson 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, 2008-01-31 19:07:28+01:00, mhansson@stripped +8 -0
  Bug#27219: count(*) in order by
  
  The error check for using aggregate functions in ORDER BY was incomplete,
  especially with respect to subqueries.
  Fixed by extending the check to cater for nesting of grouped statements.

  mysql-test/r/order_by.result@stripped, 2008-01-31 19:07:22+01:00, mhansson@stripped +119 -0
    Bug#27219: Test result

  mysql-test/t/order_by.test@stripped, 2008-01-31 19:07:22+01:00, mhansson@stripped +125 -0
    Bug#27219: Test case

  sql/item.cc@stripped, 2008-01-31 19:07:22+01:00, mhansson@stripped +27 -1
    Bug#27219: 
    - Changed signature of find_field_in_group_list.
    - Implementation of overload of Item::const_during_execution.

  sql/item.h@stripped, 2008-01-31 19:07:22+01:00, mhansson@stripped +7 -0
    Bug#27219:
    - Added comment to Item::const_during_execution
    - overloaded Item::const_during_execution in Item_field.

  sql/item_sum.cc@stripped, 2008-01-31 19:07:22+01:00, mhansson@stripped +36 -0
    Bug#27219: New override of const_during_execution()

  sql/item_sum.h@stripped, 2008-01-31 19:07:22+01:00, mhansson@stripped +1 -0
    Bug#27219: New override of const_during_execution()

  sql/sql_select.cc@stripped, 2008-01-31 19:07:22+01:00, mhansson@stripped +104 -0
    Bug#27219: 
    - The fix
    - Implementation of new predicate method JOIN::aggregates_item

  sql/sql_select.h@stripped, 2008-01-31 19:07:22+01:00, mhansson@stripped +15 -0
    Bug#27219: Test case

diff -Nrup a/mysql-test/r/order_by.result b/mysql-test/r/order_by.result
--- a/mysql-test/r/order_by.result	2007-04-04 07:24:55 +02:00
+++ b/mysql-test/r/order_by.result	2008-01-31 19:07:22 +01:00
@@ -1064,3 +1064,122 @@ a	b
 10	00:00:10
 0	00:00:00
 DROP TABLE t1;
+CREATE TABLE t1 (a INT, b INT);
+INSERT INTO t1 VALUES (1, 3), (1, 3), (2, 2), (3, 1), (3, 1), (3, 1);
+SET @old_sql_mode = @@sql_mode;
+SET @@sql_mode='ONLY_FULL_GROUP_BY';
+SELECT SUM(a) FROM t1;
+SUM(a)
+13
+SELECT a FROM t1 ORDER BY COUNT(*);
+ERROR HY000: Invalid use of group function
+SELECT a FROM t1 ORDER BY COUNT(a);
+ERROR HY000: Invalid use of group function
+SELECT a FROM t1 ORDER BY COUNT(b);
+ERROR HY000: Invalid use of group function
+SELECT a FROM t1 ORDER BY a, COUNT(b);
+ERROR HY000: Invalid use of group function
+SELECT a FROM t1 ORDER BY SUM(a);
+ERROR HY000: Invalid use of group function
+SELECT a FROM t1 ORDER BY MAX(a);
+ERROR HY000: Invalid use of group function
+SELECT a FROM t1 ORDER BY MIN(a);
+ERROR HY000: Invalid use of group function
+SELECT a FROM t1 ORDER BY AVG(a);
+ERROR HY000: Invalid use of group function
+SELECT a, COUNT(*) FROM t1 GROUP BY a ORDER BY COUNT(*);
+a	COUNT(*)
+2	1
+1	2
+3	3
+SELECT a, COUNT(*) FROM t1 GROUP BY a ORDER BY COUNT(*) DESC;
+a	COUNT(*)
+3	3
+1	2
+2	1
+SELECT a, SUM(b) FROM t1 GROUP BY a ORDER BY SUM(b);
+a	SUM(b)
+2	2
+3	3
+1	6
+SELECT a, SUM(b) FROM t1 GROUP BY a ORDER BY SUM(b) DESC;
+a	SUM(b)
+1	6
+3	3
+2	2
+CREATE TABLE t2 (c INT, d INT);
+SELECT a
+FROM t1
+WHERE a = ( SELECT c FROM t2 ORDER BY SUM(b) );
+ERROR HY000: Invalid use of group function
+SELECT a
+FROM t1
+WHERE a = ( SELECT c FROM t2 ORDER BY SUM(b) )
+GROUP BY a;
+a
+SELECT a
+FROM t1
+WHERE a IN ( SELECT c FROM t2 ORDER BY SUM(b) );
+a
+SELECT a
+FROM t1
+WHERE a IN ( SELECT c FROM t2 ORDER BY SUM(b) )
+GROUP BY a;
+a
+SELECT a 
+FROM t1 
+WHERE a = (SELECT c 
+FROM t2 
+GROUP BY c
+ORDER BY SUM(d), SUM(b) 
+LIMIT 1);
+ERROR HY000: Invalid use of group function
+SELECT 1 
+FROM t1 
+WHERE 1 = (SELECT SUM(b)  
+FROM t1
+WHERE a = (SELECT SUM(d)
+FROM t2
+GROUP BY c
+ORDER BY SUM(d), SUM(b) 
+LIMIT 1)
+GROUP BY a);
+1
+SELECT a, SUM(b) 
+FROM t1 
+WHERE a = (SELECT SUM(d)
+FROM t2
+GROUP BY c
+ORDER BY SUM(d), SUM(b)
+LIMIT 1)
+GROUP BY a;
+a	SUM(b)
+SELECT a, SUM(b) 
+FROM t1 
+WHERE a = (SELECT SUM(d)
+FROM t2
+GROUP BY c
+ORDER BY SUM(d) + SUM(b)
+LIMIT 1)
+GROUP BY a;
+a	SUM(b)
+SELECT a, SUM(b) 
+FROM t1 
+WHERE a = (SELECT SUM(d)
+FROM t2
+GROUP BY c
+ORDER BY SUM(d + a)
+LIMIT 1)
+GROUP BY a;
+a	SUM(b)
+SELECT a, SUM(b) 
+FROM t1 
+WHERE a = (SELECT SUM(d)
+FROM t2
+GROUP BY c
+ORDER BY SUM(d + a)
+LIMIT 1)
+GROUP BY a, b;
+ERROR HY000: Invalid use of group function
+SET @@sql_mode = @old_sql_mode;
+DROP TABLE t1, t2;
diff -Nrup a/mysql-test/t/order_by.test b/mysql-test/t/order_by.test
--- a/mysql-test/t/order_by.test	2007-06-01 15:49:00 +02:00
+++ b/mysql-test/t/order_by.test	2008-01-31 19:07:22 +01:00
@@ -726,3 +726,128 @@ SELECT a, b FROM t1 ORDER BY b DESC;
 SELECT a, b FROM t1 ORDER BY SEC_TO_TIME(a) DESC;
 
 DROP TABLE t1;
+
+#
+# Bug #27219: count(*) in order by
+#
+CREATE TABLE t1 (a INT, b INT);
+INSERT INTO t1 VALUES (1, 3), (1, 3), (2, 2), (3, 1), (3, 1), (3, 1);
+
+SET @old_sql_mode = @@sql_mode;
+SET @@sql_mode='ONLY_FULL_GROUP_BY';
+
+SELECT SUM(a) FROM t1;
+
+--error ER_INVALID_GROUP_FUNC_USE
+SELECT a FROM t1 ORDER BY COUNT(*);
+
+--error ER_INVALID_GROUP_FUNC_USE
+SELECT a FROM t1 ORDER BY COUNT(a);
+--error ER_INVALID_GROUP_FUNC_USE
+SELECT a FROM t1 ORDER BY COUNT(b);
+--error ER_INVALID_GROUP_FUNC_USE
+SELECT a FROM t1 ORDER BY a, COUNT(b);
+
+--error ER_INVALID_GROUP_FUNC_USE
+SELECT a FROM t1 ORDER BY SUM(a);
+
+--error ER_INVALID_GROUP_FUNC_USE
+SELECT a FROM t1 ORDER BY MAX(a);
+
+--error ER_INVALID_GROUP_FUNC_USE
+SELECT a FROM t1 ORDER BY MIN(a);
+
+--error ER_INVALID_GROUP_FUNC_USE
+SELECT a FROM t1 ORDER BY AVG(a);
+
+SELECT a, COUNT(*) FROM t1 GROUP BY a ORDER BY COUNT(*);
+SELECT a, COUNT(*) FROM t1 GROUP BY a ORDER BY COUNT(*) DESC;
+
+SELECT a, SUM(b) FROM t1 GROUP BY a ORDER BY SUM(b);
+SELECT a, SUM(b) FROM t1 GROUP BY a ORDER BY SUM(b) DESC;
+
+CREATE TABLE t2 (c INT, d INT);
+ 
+--  error ER_INVALID_GROUP_FUNC_USE
+SELECT a
+FROM t1
+WHERE a = ( SELECT c FROM t2 ORDER BY SUM(b) );
+
+SELECT a
+FROM t1
+WHERE a = ( SELECT c FROM t2 ORDER BY SUM(b) )
+GROUP BY a;
+
+# Allowed, even though it''s not obvious. The ORDER BY is simply ignored.
+SELECT a
+FROM t1
+WHERE a IN ( SELECT c FROM t2 ORDER BY SUM(b) );
+
+SELECT a
+FROM t1
+WHERE a IN ( SELECT c FROM t2 ORDER BY SUM(b) )
+GROUP BY a;
+
+#-- Should not be allowed. SUM(b) is aggregated on level 0 which does not have 
+#-- GROUP BY.
+--error ER_INVALID_GROUP_FUNC_USE
+SELECT a 
+FROM t1 
+WHERE a = (SELECT c 
+           FROM t2 
+           GROUP BY c
+           ORDER BY SUM(d), SUM(b) 
+           LIMIT 1);
+
+SELECT 1 
+FROM t1 
+WHERE 1 = (SELECT SUM(b)  
+           FROM t1
+           WHERE a = (SELECT SUM(d)
+                      FROM t2
+                      GROUP BY c
+                      ORDER BY SUM(d), SUM(b) 
+                      LIMIT 1)
+           GROUP BY a);
+
+# Should be allowed
+SELECT a, SUM(b) 
+FROM t1 
+WHERE a = (SELECT SUM(d)
+           FROM t2
+           GROUP BY c
+           ORDER BY SUM(d), SUM(b)
+           LIMIT 1)
+GROUP BY a;
+
+SELECT a, SUM(b) 
+FROM t1 
+WHERE a = (SELECT SUM(d)
+           FROM t2
+           GROUP BY c
+           ORDER BY SUM(d) + SUM(b)
+           LIMIT 1)
+GROUP BY a;
+
+SELECT a, SUM(b) 
+FROM t1 
+WHERE a = (SELECT SUM(d)
+           FROM t2
+           GROUP BY c
+           ORDER BY SUM(d + a)
+           LIMIT 1)
+GROUP BY a;
+
+--error ER_INVALID_GROUP_FUNC_USE
+SELECT a, SUM(b) 
+FROM t1 
+WHERE a = (SELECT SUM(d)
+           FROM t2
+           GROUP BY c
+           ORDER BY SUM(d + a)
+           LIMIT 1)
+GROUP BY a, b;
+
+SET @@sql_mode = @old_sql_mode;
+
+DROP TABLE t1, t2;
diff -Nrup a/sql/item.cc b/sql/item.cc
--- a/sql/item.cc	2008-01-11 18:57:33 +01:00
+++ b/sql/item.cc	2008-01-31 19:07:22 +01:00
@@ -3268,7 +3268,7 @@ void mark_select_range_as_dependent(THD 
     - NULL if find_item is not in group_list
 */
 
-static Item** find_field_in_group_list(Item *find_item, ORDER *group_list)
+static Item** find_field_in_group_list(const Item *find_item, ORDER *group_list)
 {
   const char *db_name;
   const char *table_name;
@@ -5099,6 +5099,32 @@ void Item_field::print(String *str)
     return;
   }
   Item_ident::print(str);
+}
+
+
+/**
+   @brief True if this field originates from some outer query where
+   aggregation is performed.
+
+   A field reference is a constant in a subquery if it is resolved to some
+   outer query with a GROUP BY clause containing only the field.
+   
+   In order to infer if this aggregate is a constant in the local query, the
+   algorithm searches outward until it finds a query where the field is to be
+   aggregated. JOIN::aggregates_item is used to decide this.
+
+   @see JOIN::aggregates_item
+*/
+bool Item_field::const_during_execution() const 
+{
+  Name_resolution_context *nrc= context;
+  for (; nrc; nrc= nrc->outer_context) {
+    ORDER *group_list= nrc->select_lex->join->group_list;
+    if (group_list && !group_list->next &&
+        find_field_in_group_list(this, group_list))
+      return TRUE;
+  }
+  return FALSE;
 }
 
 
diff -Nrup a/sql/item.h b/sql/item.h
--- a/sql/item.h	2007-12-13 11:49:11 +01:00
+++ b/sql/item.h	2008-01-31 19:07:22 +01:00
@@ -688,6 +688,12 @@ public:
   /* 
     Returns true if this is constant but its value may be not known yet.
     (Can be used for parameters of prep. stmts or of stored procedures.)
+
+    Aggregate functions and grouped fields in nested queries are constants
+    during the execution of the nested query if the aggregation is computed at
+    a higher nesting level.
+    
+    @see Item_sum.
   */
   virtual bool const_during_execution() const 
   { return (used_tables() & ~PARAM_TABLE_BIT) == 0; }
@@ -1347,6 +1353,7 @@ public:
     DBUG_ASSERT(field_type() == MYSQL_TYPE_GEOMETRY);
     return field->get_geometry_type();
   }
+  bool const_during_execution() const;
   friend class Item_default_value;
   friend class Item_insert_value;
   friend class st_select_lex_unit;
diff -Nrup a/sql/item_sum.cc b/sql/item_sum.cc
--- a/sql/item_sum.cc	2007-12-21 11:44:21 +01:00
+++ b/sql/item_sum.cc	2008-01-31 19:07:22 +01:00
@@ -291,6 +291,42 @@ bool Item_sum::register_sum_func(THD *th
   return FALSE;
 }
 
+/**
+   @brief True if this aggregate function references only tables in an outer
+   query where aggregation is performed.
+
+   An aggregate is a constant in a subquery if all of its field references are
+   resolved to some outer query with a GROUP BY clause.
+   
+   In order to infer if this aggregate is a constant in the local query, the
+   algorithm searches outward until it finds a query where the function can be
+   aggregated. JOIN::aggregates_item is used to decide this.
+
+   @see JOIN::aggregates_item
+*/
+bool Item_sum::const_during_execution() const 
+{
+  if (args[0]->type() == Item::FIELD_ITEM)
+  {
+    Item_field *item_field= (Item_field*)args[0];
+    Name_resolution_context *nrc= item_field->context;
+    for (; nrc && nrc->select_lex->nest_level > max_arg_level - 1; 
+         nrc= nrc->outer_context)
+      if (nrc->select_lex->join->aggregates_item(this))
+        return TRUE;
+    /* 
+       We're at the highest level for this column reference, which is by
+       definition the level where its table is instantiated. Hence it is
+       sufficient that there is a GROUP BY clause at this level.
+    */
+    if (nrc && 
+        nrc->outer_context &&
+        nrc->outer_context->select_lex->nest_level == max_arg_level &&
+        nrc->outer_context->select_lex->group_list.elements > 0)
+      return TRUE;
+  }
+  return FALSE;
+}
 
 Item_sum::Item_sum(List<Item> &list) :arg_count(list.elements), 
   forced_const(FALSE)
diff -Nrup a/sql/item_sum.h b/sql/item_sum.h
--- a/sql/item_sum.h	2007-12-14 12:24:18 +01:00
+++ b/sql/item_sum.h	2008-01-31 19:07:22 +01:00
@@ -367,6 +367,7 @@ public:  
   bool register_sum_func(THD *thd, Item **ref);
   st_select_lex *depended_from() 
     { return (nest_level == aggr_level ? 0 : aggr_sel); }
+  bool const_during_execution() const;
 };
 
 
diff -Nrup a/sql/sql_select.cc b/sql/sql_select.cc
--- a/sql/sql_select.cc	2007-12-20 11:23:59 +01:00
+++ b/sql/sql_select.cc	2008-01-31 19:07:22 +01:00
@@ -218,6 +218,8 @@ static void select_describe(JOIN *join, 
 static Item *remove_additional_cond(Item* conds);
 static void add_group_and_distinct_keys(JOIN *join, JOIN_TAB *join_tab);
 static bool test_if_ref(Item_field *left_item,Item *right_item);
+static void verify_aggregation(const Item *item, void *arg);
+static void verify_field_origin(const Item *item, void *arg);
 
 
 /*
@@ -599,6 +601,17 @@ JOIN::prepare(Item ***rref_pointer_array
 	DBUG_RETURN(-1);
       }
     }
+    for (ORDER *tmp_order= order; tmp_order ; tmp_order=tmp_order->next)
+    {
+      Item *order_item= *tmp_order->item;
+      if (order_item->with_sum_func) {
+        Aggregate_verifier verifier(this);
+        order_item->traverse_cond(&verify_aggregation, &verifier, Item::PREFIX);
+        if (verifier.error)
+          my_message(ER_INVALID_GROUP_FUNC_USE, ER(ER_INVALID_GROUP_FUNC_USE),
+                     MYF(0));
+      }
+    }
   }
   {
     /* Caclulate the number of groups */
@@ -15862,3 +15875,94 @@ bool JOIN::change_result(select_result *
   }
   DBUG_RETURN(FALSE);
 }
+
+/**
+   @brief True if an aggregate function can be computed by this JOIN. 
+
+   For a (possibly nested) query and an aggregate function appearing at a
+   subquery nesting depth greater than or equal to the nest level of this
+   JOIN.
+
+   The aggregate can trivially be computed if it has constant arguments, such
+   as COUNT(*), SUM(1), etc. In this case it is sufficient that this JOIN has
+   a GROUP BY clause.
+
+   If the aggregate is over an expression E(c_1, c_2, ..., c_n) over one or
+   more column references, the following has to hold.
+
+     - This JOIN has a GROUP BY clause
+
+     - For all c_1, ..., c_n the reference comes from a table instantiated at
+       this JOIN.
+
+   @return true If the result of the aggregate function can be computed at the
+   nesting level where this JOIN resides. Otherwise false.
+
+   @see Item_sum
+ */
+bool JOIN::aggregates_item(const Item_sum *item_sum) const
+{
+  /* For e.g. COUNT(*) it suffices that we have a GROUP BY clause. */
+  if (item_sum->args[0]->const_item())
+    return test(group_list);
+  
+  if (group_list)
+  {
+    Aggregate_verifier verifier(this);
+    Item_sum *my_item_sum= my_const_cast(Item_sum*)(item_sum);
+    my_item_sum->args[0]->traverse_cond(&verify_field_origin, &verifier, Item::PREFIX);
+    
+    return !verifier.error;
+  }
+  return FALSE;
+}
+
+
+/**
+   @brief Visitor function that checks all aggregates functions in an
+   expression tree and raises an error when an invalid one is encountered.
+
+   Valid aggregate functions are those that are aggregated by the query
+   associated with this Aggregate_verifier or by some outer query.
+ */
+static void verify_aggregation(const Item *item, void *arg) {
+  Aggregate_verifier *verifier= (Aggregate_verifier*)arg;
+  if (!item || verifier->error)
+    return;
+
+  const JOIN *join= verifier->join;
+  if (item->type() == Item::SUM_FUNC_ITEM &&
+      !join->aggregates_item((Item_sum*)item) &&
+      !item->const_during_execution())
+    verifier->error= TRUE;
+}
+
+
+/**
+   @brief Visitor function that checks subtrees in an expression tree and
+   raises an error when an invalid one is encountered.
+
+   It is assumed that the query associated with this Aggregate_verifier has a
+   GROUP BY clause.
+   
+   Valid subtrees are those that
+   - Are constants during subquery execution or.
+   - Are fields belonging to a table that participates in the query
+     associated with this Aggregate_verifier.
+ */
+static void verify_field_origin(const Item *item, void *arg) {
+  Aggregate_verifier *verifier= (Aggregate_verifier*)arg;
+  if (!item || verifier->error || item->const_during_execution())
+    return;
+
+  const JOIN *join= verifier->join;
+  if (item->type() == Item::FIELD_ITEM)
+  {
+    TABLE *t= ((const Item_field*)item)->field->table;
+    for (uint i= 0; i < join->tables; i++)
+      if (join->tables_list[i].table == t)
+        return;
+    verifier->error= TRUE;  
+  }
+}
+
diff -Nrup a/sql/sql_select.h b/sql/sql_select.h
--- a/sql/sql_select.h	2007-11-07 17:02:12 +01:00
+++ b/sql/sql_select.h	2008-01-31 19:07:22 +01:00
@@ -472,6 +472,7 @@ public:
     return (unit == &thd->lex->unit && (unit->fake_select_lex == 0 ||
                                         select_lex == unit->fake_select_lex));
   }
+  bool aggregates_item(const Item_sum *item_sum) const;
 };
 
 
@@ -653,6 +654,20 @@ protected:  
     null_key= to_field->is_null() || item->null_value;
     return (err > 2 ?  STORE_KEY_FATAL : (store_key_result) err);
   }
+};
+
+/**
+   Visitor class that is used to verify aggregate functions and their
+   arguments against a particular query. The query is usually nested.
+   
+   @see verify_aggregation(const Item*, void*)
+   @see verify_field_origin(const Item*, void*)
+ */
+class Aggregate_verifier {
+public:
+  const JOIN *join;
+  bool error;
+  Aggregate_verifier(const JOIN *join_arg) : join(join_arg), error(FALSE) {}
 };
 
 bool cp_buffer_from_ref(THD *thd, TABLE_REF *ref);
Thread
bk commit into 5.0 tree (mhansson:1.2590) BUG#27219mhansson31 Jan