From: Date: December 11 2006 10:46pm Subject: bk commit into 5.0 tree (evgen:1.2325) BUG#23417 List-Archive: http://lists.mysql.com/commits/16799 X-Bug: 23417 Message-Id: <20061211214619.4C4AA22DB86@moonbone.moonbone.local> Below is the list of changes that have just been committed into a local 5.0 repository of evgen. When evgen 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-12-12 00:46:14+03:00, evgen@stripped +8 -0 Bug#23417: Too strict checks against GROUP BY in the ONLY_FULL_GROUP_BY mode In the ONLY_FULL_GROUP_BY no hidden fields allowed in the select list. To ensure this all expression in the select list are checked to be an aggregate function, a constant or to be present in the GROUP BY list. The last requirement is too strict and doesn't allow expressions like CEILING(MAX(X)) which are valid. To solve this problem each select now keeps the list of fields that aren't used under any aggregate function. If an expression from the select list isn't found in the GROUP BY list the setup_group() function additionally checks whether non-aggregated fields from that expression is present. If there at least one field the error is thrown. The Item_field objects now can store the position in the select list of the expression to which they are belongs to. The position is saved during the field's Item_field::fix_fields() call. The non_agg_fields list for non-aggregated fields is added to the SELECT_LEX class. The SELECT_LEX::cur_pos_in_select_list now stores the position in the select list of the expression being currently fixed. mysql-test/r/group_by.result@stripped, 2006-12-12 00:39:36+03:00, evgen@stripped +88 -0 Added a test case for the bug#23417: Too strict checks against GROUP BY in the ONLY_FULL_GROUP_BY mode mysql-test/t/group_by.test@stripped, 2006-12-12 00:38:58+03:00, evgen@stripped +46 -0 Added a test case for the bug#23417: Too strict checks against GROUP BY in the ONLY_FULL_GROUP_BY mode sql/item.cc@stripped, 2006-12-12 00:45:37+03:00, evgen@stripped +20 -6 Bug#23417: Too strict checks against GROUP BY in the ONLY_FULL_GROUP_BY mode The Item_field objects now store the position in the select list of the expression to which they are belongs tod uring the field's Item_field::fix_fields() call. sql/item.h@stripped, 2006-12-12 00:43:12+03:00, evgen@stripped +5 -0 Bug#23417: Too strict checks against GROUP BY in the ONLY_FULL_GROUP_BY mode The Item_field objects now can store the position in the select list of the expression to which they are belongs to. sql/sql_base.cc@stripped, 2006-12-12 00:42:53+03:00, evgen@stripped +4 -0 Bug#23417: Too strict checks against GROUP BY in the ONLY_FULL_GROUP_BY mode implementation. sql/sql_lex.cc@stripped, 2006-12-12 00:42:12+03:00, evgen@stripped +2 -0 Bug#23417: Too strict checks against GROUP BY in the ONLY_FULL_GROUP_BY mode implementation. sql/sql_lex.h@stripped, 2006-12-12 00:41:16+03:00, evgen@stripped +4 -0 Bug#23417: Too strict checks against GROUP BY in the ONLY_FULL_GROUP_BY mode The non_agg_fields list for non-aggregated fields is added to the SELECT_LEX class. The SELECT_LEX::cur_pos_in_select_list now stores the position in the select list of the expression being currently fixed. sql/sql_select.cc@stripped, 2006-12-12 00:40:01+03:00, evgen@stripped +48 -12 Bug#23417: Too strict checks against GROUP BY in the ONLY_FULL_GROUP_BY mode Each select now keeps the list of fields that aren't used under any aggregate function. If an expression from the select list isn't found in the GROUP BY list the setup_group() function additionally checks whether non-aggregated fields from that expression is present. If there at least one field the error is thrown. # 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: evgen # Host: moonbone.local # Root: /work/23417-bug-5.0-opt-mysql --- 1.244/sql/item.cc 2006-12-01 15:02:50 +03:00 +++ 1.245/sql/item.cc 2006-12-12 00:45:37 +03:00 @@ -1607,7 +1607,7 @@ Item_field::Item_field(Field *f) :Item_ident(0, NullS, *f->table_name, f->field_name), item_equal(0), no_const_subst(0), - have_privileges(0), any_privileges(0) + have_privileges(0), any_privileges(0), pos_in_select_list(0) { set_field(f); /* @@ -1621,7 +1621,7 @@ Field *f) :Item_ident(context_arg, f->table->s->db, *f->table_name, f->field_name), item_equal(0), no_const_subst(0), - have_privileges(0), any_privileges(0) + have_privileges(0), any_privileges(0), pos_in_select_list(0) { /* We always need to provide Item_field with a fully qualified field @@ -1660,7 +1660,7 @@ const char *field_name_arg) :Item_ident(context_arg, db_arg,table_name_arg,field_name_arg), field(0), result_field(0), item_equal(0), no_const_subst(0), - have_privileges(0), any_privileges(0) + have_privileges(0), any_privileges(0), pos_in_select_list(0) { collation.set(DERIVATION_IMPLICIT); } @@ -1673,7 +1673,8 @@ item_equal(item->item_equal), no_const_subst(item->no_const_subst), have_privileges(item->have_privileges), - any_privileges(item->any_privileges) + any_privileges(item->any_privileges), + pos_in_select_list(0) { collation.set(DERIVATION_IMPLICIT); } @@ -3470,6 +3471,11 @@ { if (*from_field) { + if (select->cur_pos_in_select_list >= 0) + { + pos_in_select_list= 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; @@ -3672,10 +3678,11 @@ bool Item_field::fix_fields(THD *thd, Item **reference) { DBUG_ASSERT(fixed == 0); + Field *from_field= (Field *)not_found_field; + bool outer_fixed= false; + if (!field) // If field is not checked { - Field *from_field= (Field *)not_found_field; - bool outer_fixed= false; /* In case of view, find_field_in_tables() write pointer to view field expression to 'reference', i.e. it substitute that expression instead @@ -3765,6 +3772,7 @@ goto error; else if (!ret) return FALSE; + outer_fixed= 1; } set_field(from_field); @@ -3808,6 +3816,12 @@ } #endif fixed= 1; + if (!outer_fixed && !thd->lex->in_sum_func && + thd->lex->current_select->cur_pos_in_select_list >= 0) + { + thd->lex->current_select->non_agg_fields.push_back(this); + pos_in_select_list= thd->lex->current_select->cur_pos_in_select_list; + } return FALSE; error: --- 1.213/sql/item.h 2006-11-28 16:47:46 +03:00 +++ 1.214/sql/item.h 2006-12-12 00:43:12 +03:00 @@ -1219,6 +1219,11 @@ uint have_privileges; /* field need any privileges (for VIEW creation) */ bool any_privileges; + /* + Position in the select list of the expression to which + this field belongs to. + */ + int pos_in_select_list; Item_field(Name_resolution_context *context_arg, const char *db_arg,const char *table_name_arg, --- 1.358/sql/sql_base.cc 2006-12-01 15:02:50 +03:00 +++ 1.359/sql/sql_base.cc 2006-12-12 00:42:53 +03:00 @@ -4388,6 +4388,7 @@ 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()) || @@ -4403,7 +4404,10 @@ 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= -1; + thd->lex->allow_sum_func= save_allow_sum_func; thd->set_query_id= save_set_query_id; DBUG_RETURN(test(thd->net.report_error)); --- 1.206/sql/sql_lex.cc 2006-11-16 22:19:25 +03:00 +++ 1.207/sql/sql_lex.cc 2006-12-12 00:42:12 +03:00 @@ -1181,6 +1181,8 @@ offset_limit= 0; /* denotes the default offset = 0 */ with_sum_func= 0; is_correlated= 0; + cur_pos_in_select_list= -1; + non_agg_fields.empty(); } /* --- 1.233/sql/sql_lex.h 2006-11-30 19:24:58 +03:00 +++ 1.234/sql/sql_lex.h 2006-12-12 00:41:16 +03:00 @@ -582,6 +582,10 @@ 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 non_agg_fields; + /* index in the select list of the expression currently being fixed */ + int cur_pos_in_select_list; List udf_list; /* udf function calls stack */ --- 1.478/sql/sql_select.cc 2006-11-30 19:25:00 +03:00 +++ 1.479/sql/sql_select.cc 2006-12-12 00:40:01 +03:00 @@ -13165,6 +13165,8 @@ bool *hidden_group_fields) { *hidden_group_fields=0; + ORDER *ptr; + if (!order) return 0; /* Everything is ok */ @@ -13178,36 +13180,70 @@ uint org_fields=all_fields.elements; thd->where="group statement"; - for (; order; order=order->next) + for (ptr= order; ptr; ptr= ptr->next) { - if (find_order_in_list(thd, ref_pointer_array, tables, order, fields, + if (find_order_in_list(thd, ref_pointer_array, tables, ptr, fields, all_fields, TRUE)) return 1; - (*order->item)->marker=1; /* Mark found */ - if ((*order->item)->with_sum_func) + (*ptr->item)->marker=1; /* Mark found */ + if ((*ptr->item)->with_sum_func) { - my_error(ER_WRONG_GROUP_FIELD, MYF(0), (*order->item)->full_name()); + my_error(ER_WRONG_GROUP_FIELD, MYF(0), (*ptr->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 list of fields that aren't under an aggregate function + is created. Each field in that list is stores the position in the + select list of the expression to 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. Otherwise we scan the list + of non aggregated fields and if we'll find at least one field that + belongs to this expression we throw the error. When we can't find any + fields belonging to this expression this means either it's a constant + expression or all fields in it are used under an aggregate function(s). + */ Item *item; + Item_field *field; + int cur_pos_in_select_list= 0; + bool get_next_field= TRUE; List_iterator li(fields); + List_iterator naf_it(thd->lex->current_select->non_agg_fields); while ((item=li++)) { if (item->type() != Item::SUM_FUNC_ITEM && !item->marker && !item->const_item()) { - /* - 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()); - return 1; + if (item->real_item()->type() == Item::FIELD_ITEM && + item->used_tables() & OUTER_REF_TABLE_BIT) + continue; + + while (!get_next_field || (field= naf_it++)) + { + get_next_field= TRUE; + /* Skip fields from previous expressions. */ + if (field->pos_in_select_list < cur_pos_in_select_list) + continue; + if (field->pos_in_select_list > cur_pos_in_select_list) + { + /* Found a field from the next expression. Save it. */ + get_next_field= FALSE; + break; + } + /* + TODO: change ER_WRONG_FIELD_WITH_GROUP to more detailed + ER_NON_GROUPING_FIELD_USED + */ + my_error(ER_WRONG_FIELD_WITH_GROUP, MYF(0), field->full_name()); + return 1; + } } + cur_pos_in_select_list++; } } if (org_fields != all_fields.elements) --- 1.75/mysql-test/r/group_by.result 2006-10-16 17:27:01 +04:00 +++ 1.76/mysql-test/r/group_by.result 2006-12-12 00:39:36 +03:00 @@ -933,3 +933,91 @@ 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 (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 +DROP TABLE t1; +SET SQL_MODE = ''; --- 1.61/mysql-test/t/group_by.test 2006-10-16 14:24:48 +04:00 +++ 1.62/mysql-test/t/group_by.test 2006-12-12 00:38:58 +03:00 @@ -701,3 +701,49 @@ 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: (ONLY_FULL_GROUP_BY) function of aggregate is still an aggregate +# [S3/S2] +# +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 (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); + +DROP TABLE t1; +SET SQL_MODE = '';