diff options
| -rw-r--r-- | mysql-test/r/group_by.result | 102 | ||||
| -rw-r--r-- | mysql-test/t/group_by.test | 51 | ||||
| -rw-r--r-- | sql/item.cc | 23 | ||||
| -rw-r--r-- | sql/item.h | 3 | ||||
| -rw-r--r-- | sql/mysql_priv.h | 3 | ||||
| -rw-r--r-- | sql/sql_base.cc | 4 | ||||
| -rw-r--r-- | sql/sql_lex.cc | 2 | ||||
| -rw-r--r-- | sql/sql_lex.h | 4 | ||||
| -rw-r--r-- | sql/sql_select.cc | 78 | ||||
| -rw-r--r-- | sql/sql_union.cc | 1 |
10 files changed, 245 insertions, 26 deletions
diff --git a/mysql-test/r/group_by.result b/mysql-test/r/group_by.result index 7d1e8832069..97375898f41 100644 --- a/mysql-test/r/group_by.result +++ b/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 = ''; diff --git a/mysql-test/t/group_by.test b/mysql-test/t/group_by.test index 3e926fba0c6..92c92bf3957 100644 --- a/mysql-test/t/group_by.test +++ b/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 = ''; diff --git a/sql/item.cc b/sql/item.cc index cc653a7db14..f1423a87837 100644 --- a/sql/item.cc +++ b/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); + 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 @@ -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: diff --git a/sql/item.h b/sql/item.h index 13f0b95c1d1..6065e385a6c 100644 --- a/sql/item.h +++ b/sql/item.h @@ -452,7 +452,8 @@ public: 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; diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index 534d42e1c17..caf3e6479f9 100644 --- a/sql/mysql_priv.h +++ b/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 diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 5dcd596f6c2..8ed8526f143 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -4393,6 +4393,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()) || @@ -4408,7 +4409,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)); diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 45272645633..3ae95ef9036 100644 --- a/sql/sql_lex.cc +++ b/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(); } /* diff --git a/sql/sql_lex.h b/sql/sql_lex.h index db119d527d9..1bf37c92b85 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -581,6 +581,10 @@ public: 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 */ diff --git a/sql/sql_select.cc b/sql/sql_select.cc index abb949ae473..fb4e2c3ab30 100644 --- a/sql/sql_select.cc +++ b/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 && - !item->const_item()) + 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)) { - /* - 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; + 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), field->full_name()); + return 1; +next_field: + field= naf_it++; + } } + cur_pos_in_select_list++; } } if (org_fields != all_fields.elements) diff --git a/sql/sql_union.cc b/sql/sql_union.cc index 55e52389a83..8b7dde2f818 100644 --- a/sql/sql_union.cc +++ b/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); } |