From: Date: November 30 2006 3:59pm Subject: bk commit into 5.0 tree (gkodinov:1.2316) BUG#23417 List-Archive: http://lists.mysql.com/commits/16224 X-Bug: 23417 Message-Id: <20061130145947.D53299DA39C@macbook.gmz> 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, 2006-11-30 16:59:25+02:00, gkodinov@stripped +14 -0 Bug #23417: (ONLY_FULL_GROUP_BY) function of aggregate is still an aggregate [S3/S2] The SQL standard compliant validity check of SELECT list and HAVING clause against the GROUP BY expressions was not done consistently. This check is performed only in ONLY_FULL_GROUP_BY mode. Implemented a comprehensive check. The new check works correctly with subqueries and expressions in GROUP BY. The check for ONLY_FULL_GROUP_BY works as a recursive algorithm starting at each subquery level with a GROUP BY. It is done for the select list and the HAVING clause. At each query element : expression or otherwise it checks : - if the element is quoted in the current GROUP BY list. When it is it stops right there and declares this query element valid. See Item::ref_group_by_validity_analyzer - if the element is a subquery. When it is it marks the fact that it's inside a subquery (by pushing to the subqueries element of ref_group_by_validity_analyzer_arg), descends through the subquery elements and then pops up the mark. See Item_subselect::ref_group_by_validity_analyzer - if the element has sub-elements (function args, etc.), it descends through them. See Item_func::ref_group_by_validity_analyzer, Item_cond::ref_group_by_validity_analyzer, etc. - If the current query element is an identifier (Item_field or Item_ref) if it is valid (taking into account subquery nesting level and the GROUP BY list) and decides whether to follow the Item_ref or not. See Item_ident::ref_group_by_validity_analyzer. - If the current query element is a column reference (Item_field) and it's not valid reference (as checked above) an error is thrown. See Item_field::ref_group_by_validity_analyzer. There's error handling : implemented through ref_group_by_validity_analyzer_arg::err_discovered. Initially this is off (false). But if an error is discovered 'err_discovered' is set to on causing each ref_group_by_validity_analyzer to return FALSE immediately, which effects in getting out of the current compile() call immediately. In order to implement the above traversal Item::walk() will not suffice : we need to be able to decide wether to check the sub-items at a hook level. So the Item::compile() function is extended to cover the same scope as Item::walk(). Then Item::compile() is used to traverse the Item tree. mysql-test/r/group_by.result@stripped, 2006-11-30 16:59:03+02:00, gkodinov@stripped +95 -0 Bug #23417: (ONLY_FULL_GROUP_BY) function of aggregate is still an aggregate [S3/S2] - test case mysql-test/t/group_by.test@stripped, 2006-11-30 16:59:04+02:00, gkodinov@stripped +50 -0 Bug #23417: (ONLY_FULL_GROUP_BY) function of aggregate is still an aggregate [S3/S2] - test case sql/item.cc@stripped, 2006-11-30 16:59:05+02:00, gkodinov@stripped +138 -2 Bug #23417: (ONLY_FULL_GROUP_BY) function of aggregate is still an aggregate [S3/S2] - implementation - removed duplicate parenthesis sql/item.h@stripped, 2006-11-30 16:59:06+02:00, gkodinov@stripped +44 -0 Bug #23417: (ONLY_FULL_GROUP_BY) function of aggregate is still an aggregate [S3/S2] - implementation sql/item_cmpfunc.cc@stripped, 2006-11-30 16:59:06+02:00, gkodinov@stripped +51 -0 Bug #23417: (ONLY_FULL_GROUP_BY) function of aggregate is still an aggregate [S3/S2] - implementation sql/item_cmpfunc.h@stripped, 2006-11-30 16:59:07+02:00, gkodinov@stripped +2 -0 Bug #23417: (ONLY_FULL_GROUP_BY) function of aggregate is still an aggregate [S3/S2] - implementation sql/item_row.cc@stripped, 2006-11-30 16:59:08+02:00, gkodinov@stripped +41 -0 Bug #23417: (ONLY_FULL_GROUP_BY) function of aggregate is still an aggregate [S3/S2] - implementation sql/item_row.h@stripped, 2006-11-30 16:59:08+02:00, gkodinov@stripped +2 -0 Bug #23417: (ONLY_FULL_GROUP_BY) function of aggregate is still an aggregate [S3/S2] - implementation sql/item_strfunc.h@stripped, 2006-11-30 16:59:09+02:00, gkodinov@stripped +3 -0 Bug #23417: (ONLY_FULL_GROUP_BY) function of aggregate is still an aggregate [S3/S2] - implementation sql/item_subselect.cc@stripped, 2006-11-30 16:59:10+02:00, gkodinov@stripped +81 -0 Bug #23417: (ONLY_FULL_GROUP_BY) function of aggregate is still an aggregate [S3/S2] - implementation sql/item_subselect.h@stripped, 2006-11-30 16:59:10+02:00, gkodinov@stripped +21 -0 Bug #23417: (ONLY_FULL_GROUP_BY) function of aggregate is still an aggregate [S3/S2] - implementation sql/item_sum.cc@stripped, 2006-11-30 16:59:11+02:00, gkodinov@stripped +48 -0 Bug #23417: (ONLY_FULL_GROUP_BY) function of aggregate is still an aggregate [S3/S2] - implementation sql/item_sum.h@stripped, 2006-11-30 16:59:12+02:00, gkodinov@stripped +15 -0 Bug #23417: (ONLY_FULL_GROUP_BY) function of aggregate is still an aggregate [S3/S2] - implementation sql/sql_select.cc@stripped, 2006-11-30 16:59:13+02:00, gkodinov@stripped +23 -23 Bug #23417: (ONLY_FULL_GROUP_BY) function of aggregate is still an aggregate [S3/S2] - implementation # 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/B23417-5.0-opt --- 1.242/sql/item.cc 2006-11-16 21:19:24 +02:00 +++ 1.243/sql/item.cc 2006-11-30 16:59:05 +02:00 @@ -1787,6 +1787,75 @@ void Item_ident::print(String *str) } } +/* + Check validity and decide about descending into sub-elements + + SYNOPSIS + ref_group_by_validity_analyzer() + arg pointer to ref_group_by_validity_analyzer_arg. Arguments. + + DESCRIPTION + Used to check a SQL statement with GROUP BY for validity with regards to + using only expressions that are quoted in the GROUP BY and aggregate + functions (when ONLY_FULL_GROUP_BY is on). + The check for ONLY_FULL_GROUP_BY works as a recursive algorithm starting + at each subquery level with a GROUP BY. It is done for the select list + and the HAVING clause. + Note that each overloaded implementation must (directly or indirectly) + call Item::ref_group_by_validity_analyzer : this is to ensure that the + error handling works. + It descends down the nested query elements - subqueries, expressions etc. + At each query element : expression or otherwise it checks : + - if the element is quoted in the current GROUP BY list. When it is it + stops right there and declares this query element valid. + See Item::ref_group_by_validity_analyzer + - if the element is a subquery. When it is it marks the fact that it's + inside a subquery (by pushing to the subqueries element of + ref_group_by_validity_analyzer_arg), descends through the subquery + elements and then pops up the mark. + See Item_subselect::ref_group_by_validity_analyzer + - if the element has sub-elements (function args, etc.), it descends + through them. + See Item_func::ref_group_by_validity_analyzer, + Item_cond::ref_group_by_validity_analyzer, etc. + - If the current query element is an identifier (Item_field or Item_ref) + if it is valid (taking into account subquery nesting level and the + GROUP BY list) and decides whether to follow the Item_ref or not. + See Item_ident::ref_group_by_validity_analyzer. + - If the current query element is a column reference (Item_field) + and it's not valid reference (as checked above) an error is thrown. + See Item_field::ref_group_by_validity_analyzer. + There's error handling : implemented through + ref_group_by_validity_analyzer_arg::err_discovered. Initially this is off + (false). But if an error is discovered 'err_discovered' is set to on causing + each ref_group_by_validity_analyzer to return FALSE immediately, which + effects in getting out of the current compile() call immediately. + + RETURN VALUE + TRUE Must check the sub-elements (if any) + FALSE don't check sub-elements. +*/ +bool Item_ident::ref_group_by_validity_analyzer (byte **arg) +{ + struct ref_group_by_validity_analyzer_arg *args = + *(struct ref_group_by_validity_analyzer_arg **)arg; + + if (!Item::ref_group_by_validity_analyzer(arg)) + return FALSE; + + /* wrong if : */ + return (Item::ref_group_by_validity_analyzer(arg) && + /* + in a subquery below GROUP BY and refers to the select that has + the GROUP BY + */ + ((args->subqueries.elements && + depended_from && depended_from == args->thd->lex->current_select) || + /* at the same level as the GROUP BY and not an external reference */ + (!args->subqueries.elements && !depended_from))); +} + + /* ARGSUSED */ String *Item_field::val_str(String *str) { @@ -4764,6 +4833,73 @@ bool Item_field::send(Protocol *protocol } +/* + Check validity and decide about descending into sub-elements + + SYNOPSIS + ref_group_by_validity_analyzer() + arg pointer to ref_group_by_validity_analyzer_arg. Arguments. + + DESCRIPTION + Used to check a SQL statement with GROUP BY for validity with regards to + using only expressions that are quoted in the GROUP BY and aggregate + functions (when ONLY_FULL_GROUP_BY is on). + See Item_ident::ref_group_by_validity_analyzer for description of the + overall algorithm. + + RETURN VALUE + TRUE Must check the sub-elements (if any) + FALSE don't check sub-elements. +*/ +bool Item_field::ref_group_by_validity_analyzer (byte **arg) +{ + if (Item_ident::ref_group_by_validity_analyzer(arg)) + { + struct ref_group_by_validity_analyzer_arg *args = + *(struct ref_group_by_validity_analyzer_arg **)arg; + args->err_discovered= TRUE; + my_error(ER_WRONG_FIELD_WITH_GROUP, MYF(0), full_name()); + } + return FALSE; +} + + +/* + Check validity and decide about descending into sub-elements + + SYNOPSIS + ref_group_by_validity_analyzer() + arg pointer to ref_group_by_validity_analyzer_arg. Arguments. + + DESCRIPTION + Used to check a SQL statement with GROUP BY for validity with regards to + using only expressions that are quoted in the GROUP BY and aggregate + functions (when ONLY_FULL_GROUP_BY is on). + See Item_ident::ref_group_by_validity_analyzer for description of the + overall algorithm. + + RETURN VALUE + TRUE Must check the sub-elements (if any) + FALSE don't check sub-elements. +*/ +bool Item::ref_group_by_validity_analyzer (byte **arg) +{ + struct ref_group_by_validity_analyzer_arg *args = + *(struct ref_group_by_validity_analyzer_arg **)arg; + + if (args->err_discovered || const_item()) + return FALSE; + + for (ORDER *group_by= args->group_by; group_by; group_by= group_by->next) + { + if ((*group_by->item)->eq(this, 0)) + return FALSE; + } + + return TRUE; +} + + Item_ref::Item_ref(Name_resolution_context *context_arg, Item **item, const char *table_name_arg, const char *field_name_arg) @@ -4912,9 +5048,9 @@ bool Item_ref::fix_fields(THD *thd, Item ER_WRONG_FIELD_WITH_GROUP, instead of the less informative ER_BAD_FIELD_ERROR which we produce now. */ - if ((place != IN_HAVING || + if (place != IN_HAVING || (!select->with_sum_func && - select->group_list.elements == 0))) + select->group_list.elements == 0)) { /* In case of view, find_field_in_tables() write pointer to view --- 1.212/sql/item.h 2006-11-16 21:19:24 +02:00 +++ 1.213/sql/item.h 2006-11-30 16:59:06 +02:00 @@ -414,6 +414,23 @@ typedef bool (Item::*Item_analyzer) (byt typedef Item* (Item::*Item_transformer) (byte *arg); typedef void (*Cond_traverser) (const Item *item, void *arg); +/* + A collection of the arguments needed to check strict GROUP BY validity + (ONLY_FULL_GROUP_BY). + See Item_ident::ref_group_by_validity_analyzer for description of the + overall algorithm. +*/ +class ref_group_by_validity_analyzer_arg +{ +public: + THD *thd; /* current thread */ + ORDER *group_by; /* GROUP BY list being checked */ + bool err_discovered; /* error has been discovered */ + List subqueries; /* subquery stack */ + + ref_group_by_validity_analyzer_arg(THD *thd_arg, ORDER *group_by_arg) : + thd(thd_arg), group_by(group_by_arg), err_discovered(FALSE) {} +}; class Item { Item(const Item &); /* Prevent use of these */ @@ -765,6 +782,19 @@ public: return 0; } + /* helper function for compile() : compile on Item with a single child */ + Item *compile_single_arg(Item **arg, Item_analyzer analyzer, byte **arg_p, + Item_transformer transformer, byte *arg_t) + { + if (!(this->*analyzer)(arg_p)) + return 0; + byte *arg_v= *arg_p; + Item *new_arg= (*arg)->compile(analyzer, &arg_v, transformer, arg_t); + if (new_arg && new_arg != *arg) + *arg= new_arg; + return ((this->*transformer)(arg_t)); + } + virtual void traverse_cond(Cond_traverser traverser, void *arg, traverse_order order) { @@ -786,6 +816,8 @@ public: return TRUE; } + virtual bool ref_group_by_validity_analyzer (byte **arg); + virtual Item *ref_group_by_validity_transformer (byte *arg) { return NULL; } virtual Item *equal_fields_propagator(byte * arg) { return this; } virtual bool set_no_const_sub(byte *arg) { return FALSE; } virtual Item *replace_equal_field(byte * arg) { return this; } @@ -1172,6 +1204,7 @@ public: const char *db_name, const char *table_name, List_iterator *it, bool any_privileges); + bool ref_group_by_validity_analyzer (byte **arg); }; @@ -1294,6 +1327,8 @@ public: Item_field *filed_for_view_update() { return this; } Item *safe_charset_converter(CHARSET_INFO *tocs); int fix_outer_field(THD *thd, Field **field, Item **reference); + bool ref_group_by_validity_analyzer (byte **arg); + friend class Item_default_value; friend class Item_insert_value; friend class st_select_lex_unit; @@ -1893,6 +1928,9 @@ public: } bool walk(Item_processor processor, byte *arg) { return (*ref)->walk(processor, arg); } + Item* compile(Item_analyzer analyzer, byte **arg_p, + Item_transformer transformer, byte *arg_t) + { return compile_single_arg(ref, analyzer, arg_p, transformer, arg_t); } void print(String *str); bool result_as_longlong() { @@ -2158,6 +2196,9 @@ public: return arg->walk(processor, args) || (this->*processor)(args); } + Item* compile(Item_analyzer analyzer, byte **arg_p, + Item_transformer transformer, byte *arg_t) + { return compile_single_arg(&arg, analyzer, arg_p, transformer, arg_t); } Item *transform(Item_transformer transformer, byte *args); }; @@ -2198,6 +2239,9 @@ public: return arg->walk(processor, args) || (this->*processor)(args); } + Item* compile(Item_analyzer analyzer, byte **arg_p, + Item_transformer transformer, byte *arg_t) + { return compile_single_arg(&arg, analyzer, arg_p, transformer, arg_t); } }; --- 1.224/sql/item_cmpfunc.cc 2006-10-31 19:42:45 +02:00 +++ 1.225/sql/item_cmpfunc.cc 2006-11-30 16:59:06 +02:00 @@ -4013,6 +4013,57 @@ bool Item_equal::walk(Item_processor pro return Item_func::walk(processor, arg); } + +/* + Descend through the sub-elements of an item and call hooks + + SYNOPSIS + compile() + analyzer function to call at before processing the sub-elements + arg_p pointer to a pointer to pass to analyzer. + transformer function to call at the end of the processing. Same as + transform(). + arg_t pointer to pass transformer. + + DESCRIPTION + Descends through the sub-elements of an element. Similar to walk() and + transform(), but slightly better because it has two hooks that can be + called before and after the arguments processing. + Must be called after fix_fields(). + The analyzer hook returns a boolean. If it's FALSE the arguments are not + processed and the transformer hook is not called. + The transformer hook has the same semantics as in transform(). + + RETURN VALUE + NULL No transformation was performed + Item * The transformed item (may be the same as the this pointer +*/ +Item* Item_equal::compile(Item_analyzer analyzer, byte **arg_p, + Item_transformer transformer, byte *arg_t) +{ + if (!(this->*analyzer)(arg_p)) + return 0; + + List_iterator it(fields); + Item_field *item; + while ((item= it++)) + { + /* + The same parameter value of arg_p must be passed + to analyze any argument of the condition formula. + */ + byte *arg_v= *arg_p; + Item *new_item= item->compile(analyzer, &arg_v, transformer, arg_t); + if (new_item && new_item != item) + { + DBUG_ASSERT(new_item->type() == Item::FIELD_ITEM); + it.replace ((Item_field *)new_item); + } + } + return transformer ? Item_func::transform(transformer, arg_t) : this; +} + + Item *Item_equal::transform(Item_transformer transformer, byte *arg) { DBUG_ASSERT(!current_thd->is_stmt_prepare()); --- 1.134/sql/item_cmpfunc.h 2006-10-31 19:42:45 +02:00 +++ 1.135/sql/item_cmpfunc.h 2006-11-30 16:59:07 +02:00 @@ -1316,6 +1316,8 @@ public: bool fix_fields(THD *thd, Item **ref); void update_used_tables(); bool walk(Item_processor processor, byte *arg); + Item* compile(Item_analyzer analyzer, byte **arg_p, + Item_transformer transformer, byte *arg_t); Item *transform(Item_transformer transformer, byte *arg); void print(String *str); CHARSET_INFO *compare_collation() --- 1.117/sql/item_strfunc.h 2006-09-07 13:12:45 +03:00 +++ 1.118/sql/item_strfunc.h 2006-11-30 16:59:09 +02:00 @@ -493,6 +493,9 @@ public: return item->walk(processor, arg) || Item_str_func::walk(processor, arg); } + Item *compile(Item_analyzer analyzer, byte **arg_p, + Item_transformer transformer, byte *arg_t) + { return compile_single_arg(&item, analyzer, arg_p, transformer, arg_t); } Item *transform(Item_transformer transformer, byte *arg); void print(String *str); }; --- 1.189/sql/item_sum.cc 2006-11-16 21:19:24 +02:00 +++ 1.190/sql/item_sum.cc 2006-11-30 16:59:11 +02:00 @@ -390,6 +390,54 @@ bool Item_sum::walk (Item_processor proc } +/* + Descend through the sub-elements of an item and call hooks + + SYNOPSIS + compile() + analyzer function to call at before processing the sub-elements + arg_p pointer to a pointer to pass to analyzer. + transformer function to call at the end of the processing. Same as + transform(). + arg_t pointer to pass transformer. + + DESCRIPTION + Descends through the sub-elements of an element. Similar to walk() and + transform(), but slightly better because it has two hooks that can be + called before and after the arguments processing. + Must be called after fix_fields(). + The analyzer hook returns a boolean. If it's FALSE the arguments are not + processed and the transformer hook is not called. + The transformer hook has the same semantics as in transform(). + + RETURN VALUE + NULL No transformation was performed + Item * The transformed item (may be the same as the this pointer +*/ +Item *Item_sum::compile(Item_analyzer analyzer, byte **arg_p, + Item_transformer transformer, byte *arg_t) +{ + if (!(this->*analyzer)(arg_p)) + return 0; + if (arg_count) + { + Item **arg,**arg_end; + for (arg= args, arg_end= args+arg_count; arg != arg_end; arg++) + { + /* + The same parameter value of arg_p must be passed + to analyze any argument of the condition formula. + */ + byte *arg_v= *arg_p; + Item *new_item= (*arg)->compile(analyzer, &arg_v, transformer, arg_t); + if (new_item && *arg != new_item) + current_thd->change_item_tree(arg, new_item); + } + } + return transformer ? (this->*transformer)(arg_t) : this; +} + + Field *Item_sum::create_tmp_field(bool group, TABLE *table, uint convert_blob_length) { --- 1.106/sql/item_sum.h 2006-11-16 21:19:24 +02:00 +++ 1.107/sql/item_sum.h 2006-11-30 16:59:12 +02:00 @@ -344,6 +344,21 @@ public: virtual Field *create_tmp_field(bool group, TABLE *table, uint convert_blob_length); bool walk (Item_processor processor, byte *argument); + Item* compile(Item_analyzer analyzer, byte **arg_p, + Item_transformer transformer, byte *arg_t); + bool ref_group_by_validity_analyzer (byte **arg) + { + struct ref_group_by_validity_analyzer_arg *args = + *(struct ref_group_by_validity_analyzer_arg **)arg; + + if (!Item::ref_group_by_validity_analyzer(arg)) + return FALSE; + + if (args->subqueries.head()) + return TRUE; + else + return FALSE; + } bool init_sum_func_check(THD *thd); bool check_sum_func(THD *thd, Item **ref); bool register_sum_func(THD *thd, Item **ref); --- 1.475/sql/sql_select.cc 2006-11-21 11:44:56 +02:00 +++ 1.476/sql/sql_select.cc 2006-11-30 16:59:13 +02:00 @@ -372,6 +372,17 @@ JOIN::prepare(Item ***rref_pointer_array select_lex->having_fix_field= 0; if (having_fix_rc || thd->net.report_error) DBUG_RETURN(-1); /* purecov: inspected */ + if (thd->variables.sql_mode & MODE_ONLY_FULL_GROUP_BY) + { + /* strict GROUP BY checking for the HAVING expression */ + ref_group_by_validity_analyzer_arg args (thd, group_list), *pargs= &args; + having->compile(&Item::ref_group_by_validity_analyzer, + (byte **) &pargs, + &Item::ref_group_by_validity_transformer, + (byte *) &pargs); + if (args.err_discovered) + DBUG_RETURN(-1); + } thd->lex->allow_sum_func= save_allow_sum_func; } @@ -13170,46 +13181,35 @@ setup_group(THD *thd, Item **ref_pointer if (!order) return 0; /* Everything is ok */ - if (thd->variables.sql_mode & MODE_ONLY_FULL_GROUP_BY) - { - Item *item; - List_iterator 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 (ORDER *order_iter= order; order_iter; order_iter=order_iter->next) { - if (find_order_in_list(thd, ref_pointer_array, tables, order, fields, + if (find_order_in_list(thd, ref_pointer_array, tables, order_iter, fields, all_fields, TRUE)) return 1; - (*order->item)->marker=1; /* Mark found */ - if ((*order->item)->with_sum_func) + if ((*order_iter->item)->with_sum_func) { - my_error(ER_WRONG_GROUP_FIELD, MYF(0), (*order->item)->full_name()); + my_error(ER_WRONG_GROUP_FIELD, MYF(0), (*order_iter->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 */ + /* strict GROUP BY checking for the SELECT list */ Item *item; List_iterator li(fields); + ref_group_by_validity_analyzer_arg args (thd, order), *pargs= &args; 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; - } + item->compile(&Item::ref_group_by_validity_analyzer, + (byte **) &pargs, + &Item::ref_group_by_validity_transformer, + (byte *) &pargs); + if (args.err_discovered) + return 1; } } if (org_fields != all_fields.elements) --- 1.33/sql/item_row.cc 2006-08-24 14:49:08 +03:00 +++ 1.34/sql/item_row.cc 2006-11-30 16:59:08 +02:00 @@ -152,6 +152,47 @@ bool Item_row::walk(Item_processor proce return (this->*processor)(arg); } + +/* + Descend through the sub-elements of an item and call hooks + + SYNOPSIS + compile() + analyzer function to call at before processing the sub-elements + arg_p pointer to a pointer to pass to analyzer. + transformer function to call at the end of the processing. Same as + transform(). + arg_t pointer to pass transformer. + + DESCRIPTION + Descends through the sub-elements of an element. Similar to walk() and + transform(), but slightly better because it has two hooks that can be + called before and after the arguments processing. + Must be called after fix_fields(). + The analyzer hook returns a boolean. If it's FALSE the arguments are not + processed and the transformer hook is not called. + The transformer hook has the same semantics as in transform(). + + RETURN VALUE + NULL No transformation was performed + Item * The transformed item (may be the same as the this pointer +*/ +Item* Item_row::compile(Item_analyzer analyzer, byte **arg_p, + Item_transformer transformer, byte *arg_t) +{ + if (!(this->*analyzer)(arg_p)) + return 0; + for (uint i= 0; i < arg_count; i++) + { + byte *arg_v= *arg_p; + Item *new_item= items[i]->compile(analyzer, &arg_v, transformer, arg_t); + if (new_item && new_item != items[i]) + items[i]= new_item; + } + return transformer ? (this->*transformer)(arg_t) : this; +} + + Item *Item_row::transform(Item_transformer transformer, byte *arg) { DBUG_ASSERT(!current_thd->is_stmt_prepare()); --- 1.21/sql/item_row.h 2006-04-12 17:30:46 +03:00 +++ 1.22/sql/item_row.h 2006-11-30 16:59:08 +02:00 @@ -69,6 +69,8 @@ public: void print(String *str); bool walk(Item_processor processor, byte *arg); + Item *compile(Item_analyzer analyzer, byte **arg_p, + Item_transformer transformer, byte *arg_t); Item *transform(Item_transformer transformer, byte *arg); uint cols() { return arg_count; } --- 1.142/sql/item_subselect.cc 2006-11-17 09:15:37 +02:00 +++ 1.143/sql/item_subselect.cc 2006-11-30 16:59:10 +02:00 @@ -192,6 +192,87 @@ bool Item_subselect::fix_fields(THD *thd return res; } +/* + Descend through the sub-elements of an item and call hooks + + SYNOPSIS + compile() + analyzer function to call at before processing the sub-elements + arg_p pointer to a pointer to pass to analyzer. + transformer function to call at the end of the processing. Same as + transform(). + arg_t pointer to pass transformer. + + DESCRIPTION + Descends through the sub-elements of an element. Similar to walk() and + transform(), but slightly better because it has two hooks that can be + called before and after the arguments processing. + Must be called after fix_fields(). + The analyzer hook returns a boolean. If it's FALSE the arguments are not + processed and the transformer hook is not called. + The transformer hook has the same semantics as in transform(). + + RETURN VALUE + NULL No transformation was performed + Item * The transformed item (may be the same as the this pointer +*/ +Item* Item_subselect::compile(Item_analyzer analyzer, byte **arg_p, + Item_transformer transformer, byte *arg_t) +{ + if (!(this->*analyzer)(arg_p)) + return 0; + + for (SELECT_LEX *lex= unit->first_select(); lex; lex= lex->next_select()) + { + List_iterator li(lex->item_list); + Item *item; + ORDER *order; + + if (lex->where) + { + byte *arg_v= *arg_p; + Item *new_where= (lex->where)->compile(analyzer, &arg_v, transformer, + arg_t); + if (new_where && new_where != lex->where) + lex->where= new_where; + } + if (lex->having) + { + byte *arg_v= *arg_p; + Item *new_having= (lex->having)->compile(analyzer, &arg_v, transformer, + arg_t); + if (new_having && new_having != lex->having) + lex->having= new_having; + } + + while ((item=li++)) + { + byte *arg_v= *arg_p; + Item *new_item= item->compile(analyzer, &arg_v, transformer, arg_t); + if (new_item && new_item != item) + li.replace (new_item); + } + for (order= (ORDER*) lex->order_list.first ; order; order= order->next) + { + byte *arg_v= *arg_p; + Item *new_item= (*order->item)->compile(analyzer, &arg_v, transformer, + arg_t); + if (new_item && new_item != *order->item) + *order->item = new_item; + } + for (order= (ORDER*) lex->group_list.first ; order; order= order->next) + { + byte *arg_v= *arg_p; + Item *new_item= (*order->item)->compile(analyzer, &arg_v, transformer, + arg_t); + if (new_item && new_item != *order->item) + *order->item = new_item; + } + } + return transformer ? (this->*transformer)(arg_t) : this; +} + + bool Item_subselect::exec(bool full_scan) { int res; --- 1.82/sql/item_subselect.h 2006-11-16 21:19:24 +02:00 +++ 1.83/sql/item_subselect.h 2006-11-30 16:59:10 +02:00 @@ -125,6 +125,27 @@ public: */ virtual void reset_value_registration() {} enum_parsing_place place() { return parsing_place; } + Item* compile(Item_analyzer analyzer, byte **arg_p, + Item_transformer transformer, byte *arg_t); + bool ref_group_by_validity_analyzer (byte **arg) + { + struct ref_group_by_validity_analyzer_arg *args = + *(struct ref_group_by_validity_analyzer_arg **)arg; + + if (!Item::ref_group_by_validity_analyzer(arg)) + return FALSE; + + args->subqueries.push_front(this); + return TRUE; + } + + Item *ref_group_by_vailirity_transformer (byte *arg) + { + struct ref_group_by_validity_analyzer_arg *args = + *(struct ref_group_by_validity_analyzer_arg **)arg; + (void) args->subqueries.pop(); + return NULL; + } friend class select_subselect; friend class Item_in_optimizer; --- 1.75/mysql-test/r/group_by.result 2006-10-16 16:27:01 +03:00 +++ 1.76/mysql-test/r/group_by.result 2006-11-30 16:59:03 +02:00 @@ -933,3 +933,98 @@ 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 CEILING(a + 2) FROM t1 GROUP BY b + 2; +ERROR 42000: 'test.t1.a' isn't in GROUP BY +SELECT CEILING(b + 2) FROM t1 GROUP BY b + 2; +CEILING(b + 2) +3 +4 +5 +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 13:24:48 +03:00 +++ 1.62/mysql-test/t/group_by.test 2006-11-30 16:59:04 +02:00 @@ -701,3 +701,53 @@ EXPLAIN SELECT SQL_BIG_RESULT b, sum(1) 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; +--error ER_WRONG_FIELD_WITH_GROUP +SELECT CEILING(a + 2) FROM t1 GROUP BY b + 2; +SELECT CEILING(b + 2) FROM t1 GROUP BY b + 2; + +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 = '';