Merge epotemkin@bk-internal.mysql.com:/home/bk/mysql-5.0-opt

into  moonbone.local:/work/23417-bug-5.0-opt-mysql


sql/sql_base.cc:
  Auto merged

mysql-test/r/group_by.result

@@ -933,3 +933,105 @@ b	sum(1)
 18	6
 19	6
 DROP TABLE t1;
+CREATE TABLE t1 (a INT PRIMARY KEY, b INT);
+INSERT INTO t1 VALUES (1,1),(2,1),(3,2),(4,2),(5,3),(6,3);
+SET SQL_MODE = 'ONLY_FULL_GROUP_BY';
+SELECT MAX(a)-MIN(a) FROM t1 GROUP BY b;
+MAX(a)-MIN(a)
+1
+1
+1
+SELECT CEILING(MIN(a)) FROM t1 GROUP BY b;
+CEILING(MIN(a))
+1
+3
+5
+SELECT CASE WHEN AVG(a)>=0 THEN 'Positive' ELSE 'Negative' END FROM t1 
+GROUP BY b;
+CASE WHEN AVG(a)>=0 THEN 'Positive' ELSE 'Negative' END
+Positive
+Positive
+Positive
+SELECT a + 1 FROM t1 GROUP BY a;
+a + 1
+2
+3
+4
+5
+6
+7
+SELECT a + b FROM t1 GROUP BY b;
+ERROR 42000: 'test.t1.a' isn't in GROUP BY
+SELECT (SELECT t1_outer.a FROM t1 AS t1_inner GROUP BY b LIMIT 1) 
+FROM t1 AS t1_outer;
+(SELECT t1_outer.a FROM t1 AS t1_inner GROUP BY b LIMIT 1)
+1
+2
+3
+4
+5
+6
+SELECT 1 FROM t1 as t1_outer GROUP BY a 
+HAVING (SELECT t1_outer.a FROM t1 AS t1_inner GROUP BY b LIMIT 1);
+1
+1
+1
+1
+1
+1
+1
+SELECT (SELECT t1_outer.a FROM t1 AS t1_inner LIMIT 1) 
+FROM t1 AS t1_outer GROUP BY t1_outer.b;
+ERROR 42000: 'test.t1_outer.a' isn't in GROUP BY
+SELECT 1 FROM t1 as t1_outer GROUP BY a 
+HAVING (SELECT t1_outer.b FROM t1 AS t1_inner LIMIT 1);
+ERROR 42S22: Unknown column 'test.t1_outer.b' in 'field list'
+SELECT (SELECT SUM(t1_inner.a) FROM t1 AS t1_inner LIMIT 1) 
+FROM t1 AS t1_outer GROUP BY t1_outer.b;
+(SELECT SUM(t1_inner.a) FROM t1 AS t1_inner LIMIT 1)
+21
+21
+21
+SELECT (SELECT SUM(t1_inner.a) FROM t1 AS t1_inner GROUP BY t1_inner.b LIMIT 1)
+FROM t1 AS t1_outer;
+(SELECT SUM(t1_inner.a) FROM t1 AS t1_inner GROUP BY t1_inner.b LIMIT 1)
+3
+3
+3
+3
+3
+3
+SELECT (SELECT SUM(t1_outer.a) FROM t1 AS t1_inner LIMIT 1) 
+FROM t1 AS t1_outer GROUP BY t1_outer.b;
+ERROR 42000: 'test.t1_outer.a' isn't in GROUP BY
+SELECT 1 FROM t1 as t1_outer 
+WHERE (SELECT t1_outer.b FROM t1 AS t1_inner GROUP BY t1_inner.b LIMIT 1);
+1
+1
+1
+1
+1
+1
+1
+SELECT b FROM t1 GROUP BY b HAVING CEILING(b) > 0;
+b
+1
+2
+3
+SELECT 1 FROM t1 GROUP BY b HAVING b = 2 OR b = 3 OR SUM(a) > 12;
+1
+1
+1
+SELECT 1 FROM t1 GROUP BY b HAVING ROW (b,b) = ROW (1,1);
+1
+1
+SELECT 1 FROM t1 GROUP BY b HAVING a = 2;
+ERROR 42S22: Unknown column 'a' in 'having clause'
+SELECT 1 FROM t1 GROUP BY SUM(b);
+ERROR HY000: Invalid use of group function
+SELECT b FROM t1 AS t1_outer GROUP BY a HAVING t1_outer.a IN 
+(SELECT SUM(t1_inner.b)+t1_outer.b FROM t1 AS t1_inner GROUP BY t1_inner.a
+HAVING SUM(t1_inner.b)+t1_outer.b > 5);
+ERROR 42000: 'test.t1_outer.b' isn't in GROUP BY
+DROP TABLE t1;
+SET SQL_MODE = '';

mysql-test/t/group_by.test

@@ -701,3 +701,54 @@ EXPLAIN SELECT SQL_BIG_RESULT b, sum(1) FROM t1 GROUP BY b;
 SELECT b, sum(1) FROM t1 GROUP BY b;
 SELECT SQL_BIG_RESULT b, sum(1) FROM t1 GROUP BY b;
 DROP TABLE t1;
+
+#
+# Bug #23417: Too strict checks against GROUP BY in the ONLY_FULL_GROUP_BY mode
+#
+CREATE TABLE t1 (a INT PRIMARY KEY, b INT);
+INSERT INTO t1 VALUES (1,1),(2,1),(3,2),(4,2),(5,3),(6,3);
+
+SET SQL_MODE = 'ONLY_FULL_GROUP_BY';
+SELECT MAX(a)-MIN(a) FROM t1 GROUP BY b;
+SELECT CEILING(MIN(a)) FROM t1 GROUP BY b;
+SELECT CASE WHEN AVG(a)>=0 THEN 'Positive' ELSE 'Negative' END FROM t1 
+ GROUP BY b;
+SELECT a + 1 FROM t1 GROUP BY a;
+--error ER_WRONG_FIELD_WITH_GROUP 
+SELECT a + b FROM t1 GROUP BY b;
+SELECT (SELECT t1_outer.a FROM t1 AS t1_inner GROUP BY b LIMIT 1) 
+  FROM t1 AS t1_outer;
+SELECT 1 FROM t1 as t1_outer GROUP BY a 
+  HAVING (SELECT t1_outer.a FROM t1 AS t1_inner GROUP BY b LIMIT 1);
+--error ER_WRONG_FIELD_WITH_GROUP 
+SELECT (SELECT t1_outer.a FROM t1 AS t1_inner LIMIT 1) 
+  FROM t1 AS t1_outer GROUP BY t1_outer.b;
+--error ER_BAD_FIELD_ERROR 
+SELECT 1 FROM t1 as t1_outer GROUP BY a 
+  HAVING (SELECT t1_outer.b FROM t1 AS t1_inner LIMIT 1);
+SELECT (SELECT SUM(t1_inner.a) FROM t1 AS t1_inner LIMIT 1) 
+  FROM t1 AS t1_outer GROUP BY t1_outer.b;
+SELECT (SELECT SUM(t1_inner.a) FROM t1 AS t1_inner GROUP BY t1_inner.b LIMIT 1)
+  FROM t1 AS t1_outer;
+--error ER_WRONG_FIELD_WITH_GROUP 
+SELECT (SELECT SUM(t1_outer.a) FROM t1 AS t1_inner LIMIT 1) 
+  FROM t1 AS t1_outer GROUP BY t1_outer.b;
+
+SELECT 1 FROM t1 as t1_outer 
+  WHERE (SELECT t1_outer.b FROM t1 AS t1_inner GROUP BY t1_inner.b LIMIT 1);
+
+SELECT b FROM t1 GROUP BY b HAVING CEILING(b) > 0;
+
+SELECT 1 FROM t1 GROUP BY b HAVING b = 2 OR b = 3 OR SUM(a) > 12;
+SELECT 1 FROM t1 GROUP BY b HAVING ROW (b,b) = ROW (1,1);
+
+--error ER_BAD_FIELD_ERROR
+SELECT 1 FROM t1 GROUP BY b HAVING a = 2;
+--error ER_INVALID_GROUP_FUNC_USE
+SELECT 1 FROM t1 GROUP BY SUM(b);
+--error ER_WRONG_FIELD_WITH_GROUP 
+SELECT b FROM t1 AS t1_outer GROUP BY a HAVING t1_outer.a IN 
+  (SELECT SUM(t1_inner.b)+t1_outer.b FROM t1 AS t1_inner GROUP BY t1_inner.a
+   HAVING SUM(t1_inner.b)+t1_outer.b > 5);
+DROP TABLE t1;
+SET SQL_MODE = '';

sql/item.cc

@@ -3469,6 +3469,16 @@ Item_field::fix_outer_field(THD *thd, Field **from_field, Item **reference)
     {
       if (*from_field)
       {
+        if (thd->variables.sql_mode & MODE_ONLY_FULL_GROUP_BY &&
+            select->cur_pos_in_select_list != UNDEF_POS)
+        {
+          /*
+            As this is an outer field it should be added to the list of
+            non aggregated fields of the outer select.
+          */
+          marker= select->cur_pos_in_select_list;
+          select->non_agg_fields.push_back(this);
+        }
         if (*from_field != view_ref_found)
         {
           prev_subselect_item->used_tables_cache|= (*from_field)->table->map;
@@ -3671,10 +3681,11 @@ Item_field::fix_outer_field(THD *thd, Field **from_field, Item **reference)
 bool Item_field::fix_fields(THD *thd, Item **reference)
 {
   DBUG_ASSERT(fixed == 0);
-  if (!field)					// If field is not checked
-  {
   Field *from_field= (Field *)not_found_field;
   bool outer_fixed= false;
+
+  if (!field)					// If field is not checked
+  {
     /*
       In case of view, find_field_in_tables() write pointer to view field
       expression to 'reference', i.e. it substitute that expression instead
@@ -3766,6 +3777,7 @@ bool Item_field::fix_fields(THD *thd, Item **reference)
         goto error;
       else if (!ret)
         return FALSE;
+      outer_fixed= 1;
     }
 
     set_field(from_field);
@@ -3809,6 +3821,13 @@ bool Item_field::fix_fields(THD *thd, Item **reference)
   }
 #endif
   fixed= 1;
+  if (thd->variables.sql_mode & MODE_ONLY_FULL_GROUP_BY &&
+      !outer_fixed && !thd->lex->in_sum_func &&
+      thd->lex->current_select->cur_pos_in_select_list != UNDEF_POS)
+  {
+    thd->lex->current_select->non_agg_fields.push_back(this);
+    marker= thd->lex->current_select->cur_pos_in_select_list;
+  }
   return FALSE;
 
 error:

sql/item.h

@@ -452,7 +452,8 @@ class Item {
   Item *next;
   uint32 max_length;
   uint name_length;                     /* Length of name */
-  uint8 marker, decimals;
+  int8 marker;
+  uint8 decimals;
   my_bool maybe_null;			/* If item may be null */
   my_bool null_value;			/* if item is null */
   my_bool unsigned_flag;

sql/mysql_priv.h

@@ -410,7 +410,8 @@ MY_LOCALE *my_locale_by_name(const char *name);
 #define UNCACHEABLE_EXPLAIN     8
 /* Don't evaluate subqueries in prepare even if they're not correlated */
 #define UNCACHEABLE_PREPARE    16
-
+/* Used to chack GROUP BY list in the MODE_ONLY_FULL_GROUP_BY mode */
+#define UNDEF_POS (-1)
 #ifdef EXTRA_DEBUG
 /*
   Sync points allow us to force the server to reach a certain line of code

sql/sql_base.cc

@@ -4406,6 +4406,7 @@ bool setup_fields(THD *thd, Item **ref_pointer_array,
     bzero(ref_pointer_array, sizeof(Item *) * fields.elements);
 
   Item **ref= ref_pointer_array;
+  thd->lex->current_select->cur_pos_in_select_list= 0;
   while ((item= it++))
   {
     if (!item->fixed && item->fix_fields(thd, it.ref()) ||
@@ -4421,7 +4422,10 @@ bool setup_fields(THD *thd, Item **ref_pointer_array,
 	sum_func_list)
       item->split_sum_func(thd, ref_pointer_array, *sum_func_list);
     thd->used_tables|= item->used_tables();
+    thd->lex->current_select->cur_pos_in_select_list++;
   }
+  thd->lex->current_select->cur_pos_in_select_list= UNDEF_POS;
+
   thd->lex->allow_sum_func= save_allow_sum_func;
   thd->set_query_id= save_set_query_id;
   DBUG_RETURN(test(thd->net.report_error));

sql/sql_lex.cc

@@ -1180,6 +1180,8 @@ void st_select_lex::init_select()
   offset_limit= 0;      /* denotes the default offset = 0 */
   with_sum_func= 0;
   is_correlated= 0;
+  cur_pos_in_select_list= UNDEF_POS;
+  non_agg_fields.empty();
 }
 
 /*

sql/sql_lex.h

@@ -581,6 +581,10 @@ class st_select_lex: public st_select_lex_node
   bool no_wrap_view_item;
   /* exclude this select from check of unique_table() */
   bool exclude_from_table_unique_test;
+  /* List of fields that aren't under an aggregate function */
+  List<Item_field> non_agg_fields;
+  /* index in the select list of the expression currently being fixed */
+  int cur_pos_in_select_list;
 
   List<udf_func>     udf_list;                  /* udf function calls stack */
 

sql/sql_select.cc

@@ -13214,49 +13214,83 @@ setup_group(THD *thd, Item **ref_pointer_array, TABLE_LIST *tables,
 	    bool *hidden_group_fields)
 {
   *hidden_group_fields=0;
+  ORDER *ord;
+
   if (!order)
     return 0;				/* Everything is ok */
 
-  if (thd->variables.sql_mode & MODE_ONLY_FULL_GROUP_BY)
-  {
-    Item *item;
-    List_iterator<Item> li(fields);
-    while ((item=li++))
-      item->marker=0;			/* Marker that field is not used */
-  }
   uint org_fields=all_fields.elements;
 
   thd->where="group statement";
-  for (; order; order=order->next)
+  for (ord= order; ord; ord= ord->next)
   {
-    if (find_order_in_list(thd, ref_pointer_array, tables, order, fields,
+    if (find_order_in_list(thd, ref_pointer_array, tables, ord, fields,
 			   all_fields, TRUE))
       return 1;
-    (*order->item)->marker=1;		/* Mark found */
-    if ((*order->item)->with_sum_func)
+    (*ord->item)->marker= UNDEF_POS;		/* Mark found */
+    if ((*ord->item)->with_sum_func)
     {
-      my_error(ER_WRONG_GROUP_FIELD, MYF(0), (*order->item)->full_name());
+      my_error(ER_WRONG_GROUP_FIELD, MYF(0), (*ord->item)->full_name());
       return 1;
     }
   }
   if (thd->variables.sql_mode & MODE_ONLY_FULL_GROUP_BY)
   {
-    /* Don't allow one to use fields that is not used in GROUP BY */
+    /*
+      Don't allow one to use fields that is not used in GROUP BY
+      For each select a list of field references that aren't under an
+      aggregate function is created. Each field in this list keeps the
+      position of the select list expression which it belongs to.
+
+      First we check an expression from the select list against the GROUP BY
+      list. If it's found there then it's ok. It's also ok if this expression
+      is a constant or an aggregate function. Otherwise we scan the list
+      of non-aggregated fields and if we'll find at least one field reference
+      that belongs to this expression and doesn't occur in the GROUP BY list
+      we throw an error. If there are no fields in the created list for a
+      select list expression this means that all fields in it are used under
+      aggregate functions.
+    */
     Item *item;
+    Item_field *field;
+    int cur_pos_in_select_list= 0;
     List_iterator<Item> li(fields);
+    List_iterator<Item_field> naf_it(thd->lex->current_select->non_agg_fields);
 
-    while ((item=li++))
+    field= naf_it++;
+    while (field && (item=li++))
+    {
+      if (item->type() != Item::SUM_FUNC_ITEM && item->marker >= 0 &&
+          !item->const_item() &&
+          !(item->real_item()->type() == Item::FIELD_ITEM &&
+            item->used_tables() & OUTER_REF_TABLE_BIT))
       {
-      if (item->type() != Item::SUM_FUNC_ITEM && !item->marker &&
-	  !item->const_item())
+        while (field)
         {
+          /* Skip fields from previous expressions. */
+          if (field->marker < cur_pos_in_select_list)
+            goto next_field;
+          /* Found a field from the next expression. */
+          if (field->marker > cur_pos_in_select_list)
+            break;
+          /*
+            Check whether the field occur in the GROUP BY list.
+            Throw the error later if the field isn't found.
+          */
+          for (ord= order; ord; ord= ord->next)
+            if ((*ord->item)->eq((Item*)field, 0))
+              goto next_field;
           /*
             TODO: change ER_WRONG_FIELD_WITH_GROUP to more detailed
             ER_NON_GROUPING_FIELD_USED
           */
-	my_error(ER_WRONG_FIELD_WITH_GROUP, MYF(0), item->full_name());
+          my_error(ER_WRONG_FIELD_WITH_GROUP, MYF(0), field->full_name());
           return 1;
+next_field:
+          field= naf_it++;
+        }
       }
+      cur_pos_in_select_list++;
     }
   }
   if (org_fields != all_fields.elements)

sql/sql_union.cc

@@ -732,6 +732,7 @@ bool st_select_lex::cleanup()
   {
     error= (bool) ((uint) error | (uint) lex_unit->cleanup());
   }
+  non_agg_fields.empty();
   DBUG_RETURN(error);
 }