#At file:///export/home/didrik/mysqldev-next-mr/next-mr-bf-br2/ based on revid:alik@stripped
2927 Tor Didriksen 2009-10-28
Bug#48060 Memory leak - Item::val_bool() (item.cc:184) from optimizer_subquery grammar
Item_sum::set_aggregator() may be called multiple times during query preparation.
On subsequent calls: verify that the aggregator type is the same,
and re-use the existing Aggregator.
@ sql/item_sum.cc
In Item_sum::set_aggregator(): re-use existing Aggregator if already set.
Remove some friend declarations, add some accessor functions.
Cleanup some DBUG_ENTER and DBUG_RETURN code.
@ sql/item_sum.h
Make some member fields private, add accessors instead.
Remove some un-necessary friend declarations.
Remove some default arguments from constructors.
@ sql/opt_sum.cc
Use accessor functions in Item_sum.
@ sql/sql_select.cc
Fix mis-spelled DBUG_ENTER text.
Use accessor functions in Item_sum.
@ sql/sql_yacc.yy
Use explicit true/false rather than default arguments when constructing
Item_sum_xxx objects.
modified:
sql/item_sum.cc
sql/item_sum.h
sql/opt_sum.cc
sql/sql_select.cc
sql/sql_yacc.yy
=== modified file 'sql/item_sum.cc'
--- a/sql/item_sum.cc 2009-09-28 07:21:25 +0000
+++ b/sql/item_sum.cc 2009-10-28 10:07:30 +0000
@@ -564,6 +564,11 @@ Item *Item_sum::set_arg(uint i, THD *thd
int Item_sum::set_aggregator(Aggregator::Aggregator_type aggregator)
{
+ if (aggr)
+ {
+ DBUG_ASSERT(aggregator == aggr->Aggrtype());
+ return FALSE;
+ }
switch (aggregator)
{
case Aggregator::DISTINCT_AGGREGATOR:
@@ -736,12 +741,12 @@ bool Aggregator_distinct::setup(THD *thd
if (list.push_back(item))
return TRUE; // End of memory
if (item->const_item() && item->is_null())
- always_null=1;
+ always_null= true;
}
if (always_null)
return FALSE;
- count_field_types(select_lex,tmp_table_param,list,0);
- tmp_table_param->force_copy_fields= item_sum->force_copy_fields;
+ count_field_types(select_lex, tmp_table_param, list, 0);
+ tmp_table_param->force_copy_fields= item_sum->has_force_copy_fields();
DBUG_ASSERT(table == 0);
/*
Make create_tmp_table() convert BIT columns to BIGINT.
@@ -844,10 +849,10 @@ bool Aggregator_distinct::setup(THD *thd
List<Create_field> field_list;
Create_field field_def; /* field definition */
Item *arg;
- DBUG_ENTER("Item_sum_distinct::setup");
+ DBUG_ENTER("Aggregator_distinct::setup");
/* It's legal to call setup() more than once when in a subquery */
if (tree)
- return FALSE;
+ DBUG_RETURN(FALSE);
/*
Virtual table and the tree are created anew on each re-execution of
@@ -855,23 +860,23 @@ bool Aggregator_distinct::setup(THD *thd
mem_root.
*/
if (field_list.push_back(&field_def))
- return TRUE;
+ DBUG_RETURN(TRUE);
item_sum->null_value= item_sum->maybe_null= 1;
item_sum->quick_group= 0;
DBUG_ASSERT(item_sum->get_arg(0)->fixed);
- arg = item_sum->get_arg(0);
+ arg= item_sum->get_arg(0);
if (arg->const_item())
{
(void) arg->val_int();
if (arg->null_value)
- always_null=1;
+ always_null= true;
}
if (always_null)
- return FALSE;
+ DBUG_RETURN(FALSE);
enum enum_field_types field_type;
@@ -884,7 +889,7 @@ bool Aggregator_distinct::setup(THD *thd
arg->unsigned_flag);
if (! (table= create_virtual_tmp_table(thd, field_list)))
- return TRUE;
+ DBUG_RETURN(TRUE);
/* XXX: check that the case of CHAR(0) works OK */
tree_key_length= table->s->reclength - table->s->null_bytes;
=== modified file 'sql/item_sum.h'
--- a/sql/item_sum.h 2009-10-22 22:30:28 +0000
+++ b/sql/item_sum.h 2009-10-28 10:07:30 +0000
@@ -302,13 +302,14 @@ class st_select_lex;
class Item_sum :public Item_result_field
{
-public:
+protected:
/**
Aggregator class instance. Not set initially. Allocated only after
it is determined if the incoming data are already distinct.
*/
Aggregator *aggr;
+private:
/**
Used in making ROLLUP. Set for the ROLLUP copies of the original
Item_sum and passed to create_tmp_field() to cause it to work
@@ -324,6 +325,11 @@ public:
*/
bool with_distinct;
+public:
+
+ bool has_force_copy_fields() const { return force_copy_fields; }
+ bool has_with_distinct() const { return with_distinct; }
+
enum Sumfunctype
{ COUNT_FUNC, COUNT_DISTINCT_FUNC, SUM_FUNC, SUM_DISTINCT_FUNC, AVG_FUNC,
AVG_DISTINCT_FUNC, MIN_FUNC, MAX_FUNC, STD_FUNC,
@@ -447,12 +453,12 @@ public:
may be initialized to 0 by clear() and to NULL by
no_rows_in_result().
*/
- void no_rows_in_result()
+ virtual void no_rows_in_result()
{
if (!aggr)
- set_aggregator(with_distinct ?
- Aggregator::DISTINCT_AGGREGATOR :
- Aggregator::SIMPLE_AGGREGATOR);
+ set_aggregator(with_distinct ?
+ Aggregator::DISTINCT_AGGREGATOR :
+ Aggregator::SIMPLE_AGGREGATOR);
reset();
}
virtual void make_unique() { force_copy_fields= TRUE; }
@@ -511,11 +517,12 @@ public:
*/
int set_aggregator(Aggregator::Aggregator_type aggregator);
+
virtual void clear()= 0;
virtual bool add()= 0;
- virtual bool setup(THD *thd) {return 0;}
+ virtual bool setup(THD *thd) { return false; }
- void cleanup ();
+ virtual void cleanup();
};
@@ -533,9 +540,6 @@ class Unique;
class Aggregator_distinct : public Aggregator
{
friend class Item_sum_sum;
- friend class Item_sum_count;
- friend class Item_sum_avg;
-protected:
/*
flag to prevent consecutive runs of endup(). Normally in endup there are
@@ -567,7 +571,7 @@ protected:
uint32 *field_lengths;
/*
- used in conjunction with 'table' to support the access to Field classes
+ Used in conjunction with 'table' to support the access to Field classes
for COUNT(DISTINCT). Needed by copy_fields()/copy_funcs().
*/
TMP_TABLE_PARAM *tmp_table_param;
@@ -637,7 +641,6 @@ public:
class Item_sum_num :public Item_sum
{
- friend class Aggregator_distinct;
protected:
/*
val_xxx() functions may be called several times during the execution of a
@@ -692,14 +695,14 @@ protected:
void fix_length_and_dec();
public:
- Item_sum_sum(Item *item_par, bool distinct= FALSE) :Item_sum_num(item_par)
+ Item_sum_sum(Item *item_par, bool distinct) :Item_sum_num(item_par)
{
set_distinct(distinct);
}
Item_sum_sum(THD *thd, Item_sum_sum *item);
enum Sumfunctype sum_func () const
{
- return with_distinct ? SUM_DISTINCT_FUNC : SUM_FUNC;
+ return has_with_distinct() ? SUM_DISTINCT_FUNC : SUM_FUNC;
}
void clear();
bool add();
@@ -713,7 +716,7 @@ public:
void no_rows_in_result() {}
const char *func_name() const
{
- return with_distinct ? "sum(distinct " : "sum(";
+ return has_with_distinct() ? "sum(distinct " : "sum(";
}
Item *copy_or_same(THD* thd);
};
@@ -752,7 +755,7 @@ class Item_sum_count :public Item_sum_in
{}
enum Sumfunctype sum_func () const
{
- return with_distinct ? COUNT_DISTINCT_FUNC : COUNT_FUNC;
+ return has_with_distinct() ? COUNT_DISTINCT_FUNC : COUNT_FUNC;
}
void no_rows_in_result() { count=0; }
void make_const(longlong count_arg)
@@ -765,7 +768,7 @@ class Item_sum_count :public Item_sum_in
void update_field();
const char *func_name() const
{
- return with_distinct ? "count(distinct " : "count(";
+ return has_with_distinct() ? "count(distinct " : "count(";
}
Item *copy_or_same(THD* thd);
};
@@ -806,7 +809,7 @@ public:
uint prec_increment;
uint f_precision, f_scale, dec_bin_size;
- Item_sum_avg(Item *item_par, bool distinct= FALSE)
+ Item_sum_avg(Item *item_par, bool distinct)
:Item_sum_sum(item_par, distinct), count(0)
{}
Item_sum_avg(THD *thd, Item_sum_avg *item)
@@ -816,7 +819,7 @@ public:
void fix_length_and_dec();
enum Sumfunctype sum_func () const
{
- return with_distinct ? AVG_DISTINCT_FUNC : AVG_FUNC;
+ return has_with_distinct() ? AVG_DISTINCT_FUNC : AVG_FUNC;
}
void clear();
bool add();
@@ -832,7 +835,7 @@ public:
void no_rows_in_result() {}
const char *func_name() const
{
- return with_distinct ? "avg(distinct " : "avg(";
+ return has_with_distinct() ? "avg(distinct " : "avg(";
}
Item *copy_or_same(THD* thd);
Field *create_tmp_field(bool group, TABLE *table, uint convert_blob_length);
=== modified file 'sql/opt_sum.cc'
--- a/sql/opt_sum.cc 2009-10-19 13:36:19 +0000
+++ b/sql/opt_sum.cc 2009-10-28 10:07:30 +0000
@@ -356,9 +356,9 @@ int opt_sum_query(TABLE_LIST *tables, Li
const_result= 0;
break;
}
- item_sum->set_aggregator (item_sum->with_distinct ?
- Aggregator::DISTINCT_AGGREGATOR :
- Aggregator::SIMPLE_AGGREGATOR);
+ item_sum->set_aggregator(item_sum->has_with_distinct() ?
+ Aggregator::DISTINCT_AGGREGATOR :
+ Aggregator::SIMPLE_AGGREGATOR);
if (!count)
{
/* If count == 0, then we know that is_exact_count == TRUE. */
@@ -447,9 +447,9 @@ int opt_sum_query(TABLE_LIST *tables, Li
const_result= 0;
break;
}
- item_sum->set_aggregator (item_sum->with_distinct ?
- Aggregator::DISTINCT_AGGREGATOR :
- Aggregator::SIMPLE_AGGREGATOR);
+ item_sum->set_aggregator(item_sum->has_with_distinct() ?
+ Aggregator::DISTINCT_AGGREGATOR :
+ Aggregator::SIMPLE_AGGREGATOR);
if (!count)
{
/* If count != 1, then we know that is_exact_count == TRUE. */
=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc 2009-10-23 09:19:54 +0000
+++ b/sql/sql_select.cc 2009-10-28 10:07:30 +0000
@@ -2094,8 +2094,10 @@ JOIN::exec()
if (curr_join->make_sum_func_list(*curr_all_fields, *curr_fields_list,
1, TRUE) ||
- prepare_sum_aggregators (curr_join->sum_funcs, !curr_join->join_tab ||
- !curr_join->join_tab->is_using_agg_loose_index_scan()) ||
+ prepare_sum_aggregators(curr_join->sum_funcs,
+ !curr_join->join_tab ||
+ !curr_join->join_tab->
+ is_using_agg_loose_index_scan()) ||
setup_sum_funcs(curr_join->thd, curr_join->sum_funcs) ||
thd->is_fatal_error)
DBUG_VOID_RETURN;
@@ -4008,7 +4010,7 @@ is_indexed_agg_distinct(JOIN *join, List
join->select_lex->olap == ROLLUP_TYPE) /* Check (B3) for ROLLUP */
return false;
- if (join->make_sum_func_list(join->all_fields, join->fields_list, 1))
+ if (join->make_sum_func_list(join->all_fields, join->fields_list, true))
return false;
for (sum_item_ptr= join->sum_funcs; *sum_item_ptr; sum_item_ptr++)
@@ -15477,10 +15479,10 @@ static bool setup_sum_funcs(THD *thd, It
static bool prepare_sum_aggregators(Item_sum **func_ptr, bool need_distinct)
{
Item_sum *func;
- DBUG_ENTER("setup_sum_funcs");
+ DBUG_ENTER("prepare_sum_aggregators");
while ((func= *(func_ptr++)))
{
- if (func->set_aggregator(need_distinct && func->with_distinct ?
+ if (func->set_aggregator(need_distinct && func->has_with_distinct() ?
Aggregator::DISTINCT_AGGREGATOR :
Aggregator::SIMPLE_AGGREGATOR))
DBUG_RETURN(TRUE);
=== modified file 'sql/sql_yacc.yy'
--- a/sql/sql_yacc.yy 2009-10-27 09:59:09 +0000
+++ b/sql/sql_yacc.yy 2009-10-28 10:07:30 +0000
@@ -8249,7 +8249,7 @@ udf_expr:
sum_expr:
AVG_SYM '(' in_sum_expr ')'
{
- $$= new (YYTHD->mem_root) Item_sum_avg($3);
+ $$= new (YYTHD->mem_root) Item_sum_avg($3, FALSE);
if ($$ == NULL)
MYSQL_YYABORT;
}
@@ -8357,7 +8357,7 @@ sum_expr:
}
| SUM_SYM '(' in_sum_expr ')'
{
- $$= new (YYTHD->mem_root) Item_sum_sum($3);
+ $$= new (YYTHD->mem_root) Item_sum_sum($3, FALSE);
if ($$ == NULL)
MYSQL_YYABORT;
}
Attachment: [text/bzr-bundle] bzr/tor.didriksen@sun.com-20091028100730-ml3e9cnrl3xjx6f8.bundle
| Thread |
|---|
| • bzr commit into mysql-5.5-next-mr-bugfixing branch (tor.didriksen:2927)Bug#48060 | Tor Didriksen | 28 Oct |
