Georgi Kodinov skrev:
> #At file:///home/kgeorge/mysql/work/B48458-5.0-bugteam/ based on
> revid:joro@stripped
>
> 2840 Georgi Kodinov 2009-11-05
> Bug #48458: simple query tries to allocate enormous amount of
> memory
>
> The server was doing a bad class typecast causing setting of
> wrong value for the maximum number of items in an internal
> structure used in equality propagation.
> Fixed by not doing the wrong typecast and asserting the type
> of the Item where it should be done.
>
Ok, I may have understood this completely backwards, but could you say
that the following is true: Previously the equality propagation
procedure assumed that an expression over ROW functions, e.g. ROW(...)
<op> ROW(...) wronfully assumed the operator to be '='?
If this is not the case, I need to shape up. :-)
If this is that case, I think that would be good to include this in the
cset comment since it's 1) on a higher level 2) It's much more likely
paul will not bug you to clarify it later :-)
> ...
>
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc 2009-11-03 16:58:54 +0000
> +++ b/sql/sql_select.cc 2009-11-05 11:18:58 +0000
> @@ -7515,13 +7515,15 @@ static COND *build_equal_items_for_cond(
> {
> if ((item_equal= cond_equal.current_level.pop()))
> {
> + DBUG_ASSERT (item_equal->type() == Item::FUNC_ITEM &&
> + item_equal->functype() == Item_func::MULT_EQUAL_FUNC);
>
Is there a non-obvious reason for the asserts? In my (limited)
understanding they only serve to ensure that item_equal is of the class
Item_equal (through our poor-man's RTTI.) But COND_EQUAL::current_level
is an instantiated template of List<Item_equal>, hence only Item_equal's
can go in it anyway. Am I missing something?
> item_equal->fix_length_and_dec();
> item_equal->update_used_tables();
> + set_if_bigger(thd->lex->current_select->max_equal_elems,
> + item_equal->members());
> }
> else
> - item_equal= (Item_equal *) eq_list.pop();
> - set_if_bigger(thd->lex->current_select->max_equal_elems,
> - item_equal->members());
> + return eq_list.pop();
> return item_equal;
> }
> else
>
I think it would be more true to the coding standard if you did not wait
with 'return item_equal' until after the 'else'. The end result would be:
if ((item_equal= cond_equal.current_level.pop()))
{
item_equal->fix_length_and_dec();
item_equal->update_used_tables();
set_if_bigger(thd->lex->current_select->max_equal_elems,
item_equal->members());
return item_equal;
}
return eq_list.pop();
Apart from that, it's a great fix!
Best Regards
Martin
