diff --git a/sql/item.cc b/sql/item.cc index b4de973..3b1f8b7 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -1705,69 +1705,98 @@ class Item_aggregate_ref : public Item_ref

@param thd Thread handler @param ref_pointer_array Pointer to array of reference fields - @param fields All fields in select + @param fields All fields in select @param ref Pointer to item - @param skip_registered <=> function be must skipped for registered - SUM items + @param split_flags Zero or more of the following flags + SPLIT_FUNC_SKIP_REGISTERED: + Function be must skipped for registered SUM + SUM items + SPLIT_FUNC_SELECT + We are called on the select level and have to + register items operated on sum function Typo, it's not SPLIT_FUNC_SELECT, it is SPLIT_SUM_SELECT

@note - This is from split_sum_func2() for items that should be split - All found SUM items are added FIRST in the fields list and we replace the item with a reference.

+ If this is an item in the SELECT list then we also have to split out + all arguments to functions used together with the sum function. + For example in case of SELECT A*sum(B) we have to split out both + A and sum(B). + This is not needed for ORDER BY, GROUP BY or HAVING as all references + to items in the select list are already of type REF + thd->fatal_error() may be called if we are out of memory */

void Item::split_sum_func2(THD *thd, Item **ref_pointer_array, List<Item> &fields, Item **ref, - bool skip_registered) + uint split_flags) { - /* An item of type Item_sum is registered <=> ref_by != 0 */ - if (type() == SUM_FUNC_ITEM && skip_registered && - ((Item_sum *) this)->ref_by) - return; - if ((type() != SUM_FUNC_ITEM && with_sum_func) || - (type() == FUNC_ITEM && - (((Item_func *) this)->functype() == Item_func::ISNOTNULLTEST_FUNC || - ((Item_func *) this)->functype() == Item_func::TRIG_COND_FUNC))) + if (unlikely(type() == SUM_FUNC_ITEM)) { - /* Will split complicated items and ignore simple ones */ - split_sum_func(thd, ref_pointer_array, fields); + /* An item of type Item_sum is registered if ref_by != 0 */ + if ((split_flags & SPLIT_SUM_SKIP_REGISTERED) && + ((Item_sum *) this)->ref_by) + return; } - else if ((type() == SUM_FUNC_ITEM || (used_tables() & ~PARAM_TABLE_BIT)) && - type() != SUBSELECT_ITEM && - (type() != REF_ITEM || - ((Item_ref*)this)->ref_type() == Item_ref::VIEW_REF)) + else { - /* - Replace item with a reference so that we can easily calculate - it (in case of sum functions) or copy it (in case of fields) - - The test above is to ensure we don't do a reference for things - that are constants (PARAM_TABLE_BIT is in effect a constant) - or already referenced (for example an item in HAVING) - Exception is Item_direct_view_ref which we need to convert to - Item_ref to allow fields from view being stored in tmp table. - */ - Item_aggregate_ref *item_ref; - uint el= fields.elements; - /* - If this is an item_ref, get the original item - This is a safety measure if this is called for things that is - already a reference. - */ - Item *real_itm= real_item(); + /* Not a SUM() function */ + if (unlikely((!with_sum_func && !(split_flags & SPLIT_SUM_SELECT)))) + { + /* + This is not a SUM function and there are no SUM functions inside. + Nothing more to do. + */ + return; + } + if (likely(with_sum_func || + (type() == FUNC_ITEM && + (((Item_func *) this)->functype() == + Item_func::ISNOTNULLTEST_FUNC || + ((Item_func *) this)->functype() == + Item_func::TRIG_COND_FUNC)))) + { + /* Will call split_sum_func2() for all items */ + split_sum_func(thd, ref_pointer_array, fields, split_flags); + return; + }

- ref_pointer_array[el]= real_itm; - if (!(item_ref= new Item_aggregate_ref(&thd->lex->current_select->context, - ref_pointer_array + el, 0, name))) - return; // fatal_error is set - if (type() == SUM_FUNC_ITEM) - item_ref->depended_from= ((Item_sum *) this)->depended_from(); - fields.push_front(real_itm); - thd->change_item_tree(ref, item_ref); + if (unlikely((!(used_tables() & ~PARAM_TABLE_BIT) || + type() == SUBSELECT_ITEM || + (type() == REF_ITEM && + ((Item_ref*)this)->ref_type() != Item_ref::VIEW_REF)))) + return; } + + /* + Replace item with a reference so that we can easily calculate + it (in case of sum functions) or copy it (in case of fields) + + The test above is to ensure we don't do a reference for things + that are constants (PARAM_TABLE_BIT is in effect a constant) + or already referenced (for example an item in HAVING) + Exception is Item_direct_view_ref which we need to convert to + Item_ref to allow fields from view being stored in tmp table. + */ + Item_aggregate_ref *item_ref; + uint el= fields.elements; + /* + If this is an item_ref, get the original item + This is a safety measure if this is called for things that is + already a reference. + */ + Item *real_itm= real_item(); + + ref_pointer_array[el]= real_itm; + if (!(item_ref= new Item_aggregate_ref(&thd->lex->current_select->context, + ref_pointer_array + el, 0, name))) + return; // fatal_error is set + if (type() == SUM_FUNC_ITEM) + item_ref->depended_from= ((Item_sum *) this)->depended_from(); + fields.push_front(real_itm); + thd->change_item_tree(ref, item_ref); }

diff --git a/sql/item.h b/sql/item.h index 8254359..44649bf 100644 --- a/sql/item.h +++ b/sql/item.h @@ -74,6 +74,10 @@ char_to_byte_length_safe(uint32 char_length_arg, uint32 mbmaxlen_arg) }

+/* Bits for the split_sum_func() function */ +#define SPLIT_SUM_SKIP_REGISTERED 1 /* Skip registered funcs */ +#define SPLIT_SUM_SELECT 2 /* SELECT item; Split all parts */ + /* "Declared Type Collation" A combination of collation and its derivation. @@ -1169,10 +1173,10 @@ class Item: public Type_std_attributes return false; } virtual void split_sum_func(THD *thd, Item **ref_pointer_array, - List<Item> &fields) {} + List<Item> &fields, uint flags) {} /* Called for items that really have to be split */ void split_sum_func2(THD *thd, Item **ref_pointer_array, List<Item> &fields, - Item **ref, bool skip_registered); + Item **ref, uint flags); virtual bool get_date(MYSQL_TIME *ltime, ulonglong fuzzydate); bool get_time(MYSQL_TIME *ltime) { return get_date(ltime, TIME_TIME_ONLY | TIME_INVALID_DATES); } diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 7b34e64..caa645d 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -4575,12 +4575,13 @@ void Item_cond::traverse_cond(Cond_traverser traverser, */

void Item_cond::split_sum_func(THD *thd, Item **ref_pointer_array, - List<Item> &fields) + List<Item> &fields, uint flags) { List_iterator<Item> li(list); Item *item; while ((item= li++)) - item->split_sum_func2(thd, ref_pointer_array, fields, li.ref(), TRUE); + item->split_sum_func2(thd, ref_pointer_array, fields, li.ref(), + flags | SPLIT_SUM_SKIP_REGISTERED); Why not clear SPLIT_SUM_SELECT flag here? The Item_cond may be at the select list, but its parts are definitely not in

Hi Monty, Please find my comments on the patch for MDEV-4119 below. Sorry for the delay. the select list? The same applies to Item_row::split_sum_func and Item_func::split_sum_func.

}

diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 7d89cf5..ceab597 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -1759,7 +1759,8 @@ class Item_cond :public Item_bool_func SARGABLE_PARAM **sargables); SEL_TREE *get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr); virtual void print(String *str, enum_query_type query_type); - void split_sum_func(THD *thd, Item **ref_pointer_array, List<Item> &fields); + void split_sum_func(THD *thd, Item **ref_pointer_array, List<Item> &fields, + uint flags); friend int setup_conds(THD *thd, TABLE_LIST *tables, TABLE_LIST *leaves, COND **conds); void top_level_item() { abort_on_null=1; } diff --git a/sql/item_func.cc b/sql/item_func.cc index 3e78be8..684724b 100644 --- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -419,11 +419,12 @@ Item *Item_func::compile(Item_analyzer analyzer, uchar **arg_p, */

void Item_func::split_sum_func(THD *thd, Item **ref_pointer_array, - List<Item> &fields) + List<Item> &fields, uint flags) { Item **arg, **arg_end; for (arg= args, arg_end= args+arg_count; arg != arg_end ; arg++) - (*arg)->split_sum_func2(thd, ref_pointer_array, fields, arg, TRUE); + (*arg)->split_sum_func2(thd, ref_pointer_array, fields, arg, + flags | SPLIT_SUM_SKIP_REGISTERED); Why not clear SPLIT_SUM_SELECT flag here? (same question as in Item_cond::split_sum_func) }

........

diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 4a50003..ce02484 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -1944,8 +1954,9 @@ int JOIN::init_execution() /* Enable LIMIT ROWS EXAMINED during query execution if: (1) This JOIN is the outermost query (not a subquery or derived table) - This ensures that the limit is enabled when actual execution begins, and - not if a subquery is evaluated during optimization of the outer query. + This ensures that the limit is enabled when actual execution begins, + and not if a subquery is evaluated during optimization of the outer + query. (2) This JOIN is not the result of a UNION. In this case do not apply the limit in order to produce the partial query result stored in the UNION temp table. @@ -2562,8 +2573,12 @@ void JOIN::exec_inner() skip_sort_order= 0; } bool made_call= false; + SQL_SELECT *select= join_tab[const_tables].select; This line causes a crash in a few tests:

main.select_pkeycache main.select main.select_jcl6 main.subselect3 main.subselect3_jcl6 The reason is that execution gets here when the join is degenerate and join_tab==NULL.

if (order && - (order != group_list || !(select_options & SELECT_BIG_RESULT)) && + (order != group_list || !(select_options & SELECT_BIG_RESULT) || + (select && select->quick && + select->quick->get_type() == QUICK_SELECT_I::QS_TYPE_GROUP_MIN_MAX)) + && (const_tables == table_count || ((simple_order || skip_sort_order) && (made_call=true) &&

BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog