On Thu, Jun 12, 2008 at 04:23:31PM +0300, Georgi Kodinov wrote:
> #At file:///home/kgeorge/mysql/bzr/B34773-5.1-bugteam/
>
> 2662 Georgi Kodinov 2008-06-12
> Bug#34773: query with explain extended and derived table / other table
> crashes server
This doesn't work with prepared statements:
mysql> prepare s1 from 'EXPLAIN EXTENDED SELECT 1 FROM (SELECT COUNT(DISTINCT t1.a)
> FROM t1,t2 GROUP BY t1.a) AS s1';
Query OK, 0 rows affected (0.01 sec)
Statement prepared
mysql> execute s1;
Breakpoint 1, Item_sum::set_arg (this=0x8efa8a0, i=0, thd=0x8ee0280, new_val=0x8eff448)
at item_sum.cc:543
(gdb) p this
$8 = (Item_sum * const) 0x8efa5c0
(gdb) p this->orig_args
$9 = (class Item **) 0x0
(gdb) step
(gdb) step
sql_alloc (Size=4) at thr_malloc.cc:64
(gdb) next
(gdb) step
alloc_root (mem_root=0x8ee1bf0, length=4) at my_alloc.c:173
# Note the mem_root value
# Put a break in free_root, see it hit for some mem_roots but eventually we
# get:
Breakpoint 2, free_root (root=0x8ee1bf0, MyFlags=1) at my_alloc.c:334
#
# The mem_root where orig_args was allocated is destroyed.
#
+----+-------------+------------+------+---------------+------+---------+------+------+----------+---------------------------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows |
filtered | Extra |
+----+-------------+------------+------+---------------+------+---------+------+------+----------+---------------------------------+
| 1 | PRIMARY | <derived2> | ALL | NULL | NULL | NULL | NULL |
2 | 100.00 | |
| 2 | DERIVED | t1 | ALL | NULL | NULL | NULL | NULL | 2 |
100.00 | Using temporary; Using filesort |
| 2 | DERIVED | t2 | ALL | NULL | NULL | NULL | NULL | 2 |
100.00 | Using join buffer |
+----+-------------+------------+------+---------------+------+---------+------+------+----------+---------------------------------+
3 rows in set, 1 warning (1 min 18.97 sec)
#
# Run it again
#
mysql> execute s1;
#
# Looking at the garbage pointer.
#
Breakpoint 1, Item_sum::set_arg (this=0x8efa5c0, i=0, thd=0x8ee0280, new_val=0x8eff448)
at item_sum.cc:543
Current language: auto; currently c++
(gdb) p this
$10 = (Item_sum * const) 0x8efa5c0
(gdb) p this->orig_args
$11 = (class Item **) 0x8f27fb0
(gdb) c
Continuing.
Program received signal SIGSEGV, Segmentation fault.
0x0c2444c7 in ?? ()
(gdb) wher
#0 0x0c2444c7 in ?? ()
#1 0x08208c3f in Item_sum::print (this=0x8efa5c0, str=0xb49e9884,
query_type=QT_ORDINARY) at item_sum.cc:436
#2 0x081f644a in Item::print_item_w_name (this=0x8efa5c0, str=0xb49e9884,
query_type=QT_ORDINARY) at item.cc:451
#3 0x0832d240 in st_select_lex::print (this=0x8efa108, thd=0x8ee0280, str=0xb49e9884,
query_type=QT_ORDINARY) at sql_select.cc:16538
#4 0x081d8f1a in st_select_lex_unit::print (this=0x8efa290, str=0xb49e9884,
query_type=QT_ORDINARY) at sql_lex.cc:1950
#5 0x0832cb5d in TABLE_LIST::print (this=0x8efae10, thd=0x8ee0280, str=0xb49e9884,
query_type=QT_ORDINARY) at sql_select.cc:16430
>
> When creating temporary table that contains aggregate functions a
> non-reversible source transformation was performed to redirect aggregate
> function arguments towards temporary table columns.
> This caused EXPLAIN EXTENDED to fail because it was trying to resolve
> references to the (freed) temporary table.
> Fixed by preserving the original aggregate function arguments and
> using them (instead of the transformed ones) for EXPLAIN EXTENDED.
> modified:
> mysql-test/r/explain.result
> mysql-test/t/explain.test
> sql/item.cc
> sql/item_sum.cc
> sql/item_sum.h
> sql/opt_range.cc
> sql/opt_sum.cc
> sql/sql_select.cc
>
> per-file messages:
> mysql-test/r/explain.result
> Bug#34773: test case
> mysql-test/t/explain.test
> Bug#34773: test case
> sql/item.cc
> Bug#34773: use accessor functions instead of public members
> sql/item_sum.cc
> Bug#34773:
> - Encapsulate the arguments into Item_sum and
> provide accessor and mutator methods
> - print the orginal arguments (if present)
> in EXPLAIN EXTENDED
> sql/item_sum.h
> Bug#34773: Encapsulate the arguments into Item_sum and
> provide accessor and mutator methods.
> sql/opt_range.cc
> Bug#34773: use accessor functions instead of public members
> sql/opt_sum.cc
> Bug#34773: use accessor functions instead of public members
> sql/sql_select.cc
> Bug#34773: use accessor functions instead of public members
> === modified file 'mysql-test/r/explain.result'
> --- a/mysql-test/r/explain.result 2007-11-16 11:00:57 +0000
> +++ b/mysql-test/r/explain.result 2008-06-12 13:23:09 +0000
> @@ -107,3 +107,24 @@ X X X X X X X X X
> X X X X X X X X X Range checked for each record (index map: 0xFFFFFFFFFF)
> DROP TABLE t2;
> DROP TABLE t1;
> +CREATE TABLE t1(a INT);
> +CREATE TABLE t2(a INT);
> +INSERT INTO t1 VALUES (1),(2);
> +INSERT INTO t2 VALUES (1),(2);
> +EXPLAIN EXTENDED SELECT 1
> +FROM (SELECT COUNT(DISTINCT t1.a) FROM t1,t2 GROUP BY t1.a) AS s1;
> +id select_type table type possible_keys key key_len ref rows filtered Extra
> +1 PRIMARY <derived2> ALL NULL NULL NULL NULL 2 100.00
> +2 DERIVED t1 ALL NULL NULL NULL NULL 2 100.00 Using temporary; Using filesort
> +2 DERIVED t2 ALL NULL NULL NULL NULL 2 100.00 Using join buffer
> +Warnings:
> +Note 1003 select 1 AS `1` from (select count(distinct `test`.`t1`.`a`) AS
> `COUNT(DISTINCT t1.a)` from `test`.`t1` join `test`.`t2` group by `test`.`t1`.`a`) `s1`
> +EXPLAIN EXTENDED SELECT 1
> +FROM (SELECT COUNT(DISTINCT t1.a) FROM t1,t2 GROUP BY t1.a) AS s1;
> +id select_type table type possible_keys key key_len ref rows filtered Extra
> +1 PRIMARY <derived2> ALL NULL NULL NULL NULL 2 100.00
> +2 DERIVED t1 ALL NULL NULL NULL NULL 2 100.00 Using temporary; Using filesort
> +2 DERIVED t2 ALL NULL NULL NULL NULL 2 100.00 Using join buffer
> +Warnings:
> +Note 1003 select 1 AS `1` from (select count(distinct `test`.`t1`.`a`) AS
> `COUNT(DISTINCT t1.a)` from `test`.`t1` join `test`.`t2` group by `test`.`t1`.`a`) `s1`
> +DROP TABLE t1,t2;
>
> === modified file 'mysql-test/t/explain.test'
> --- a/mysql-test/t/explain.test 2007-11-16 11:00:57 +0000
> +++ b/mysql-test/t/explain.test 2008-06-12 13:23:09 +0000
> @@ -94,4 +94,22 @@ EXPLAIN SELECT 1 FROM
> DROP TABLE t2;
> DROP TABLE t1;
>
> +#
> +# Bug #34773: query with explain extended and derived table / other table
> +# crashes server
> +#
> +
> +CREATE TABLE t1(a INT);
> +CREATE TABLE t2(a INT);
> +INSERT INTO t1 VALUES (1),(2);
> +INSERT INTO t2 VALUES (1),(2);
> +
> +EXPLAIN EXTENDED SELECT 1
> + FROM (SELECT COUNT(DISTINCT t1.a) FROM t1,t2 GROUP BY t1.a) AS s1;
> +
> +EXPLAIN EXTENDED SELECT 1
> + FROM (SELECT COUNT(DISTINCT t1.a) FROM t1,t2 GROUP BY t1.a) AS s1;
> +
> +DROP TABLE t1,t2;
> +
> # End of 5.0 tests.
>
> === modified file 'sql/item.cc'
> --- a/sql/item.cc 2008-05-20 07:38:17 +0000
> +++ b/sql/item.cc 2008-06-12 13:23:09 +0000
> @@ -6919,7 +6919,7 @@ enum_field_types Item_type_holder::get_r
> */
> Item_sum *item_sum= (Item_sum *) item;
> if (item_sum->keep_field_type())
> - return get_real_type(item_sum->args[0]);
> + return get_real_type(item_sum->get_arg(0));
> break;
> }
> case FUNC_ITEM:
> @@ -7182,7 +7182,7 @@ void Item_type_holder::get_full_info(Ite
> if (item->type() == Item::SUM_FUNC_ITEM &&
> (((Item_sum*)item)->sum_func() == Item_sum::MAX_FUNC ||
> ((Item_sum*)item)->sum_func() == Item_sum::MIN_FUNC))
> - item = ((Item_sum*)item)->args[0];
> + item = ((Item_sum*)item)->get_arg(0);
> /*
> We can have enum/set type after merging only if we have one enum|set
> field (or MIN|MAX(enum|set field)) and number of NULL fields
>
> === modified file 'sql/item_sum.cc'
> --- a/sql/item_sum.cc 2008-05-01 11:54:59 +0000
> +++ b/sql/item_sum.cc 2008-06-12 13:23:09 +0000
> @@ -356,7 +356,7 @@ bool Item_sum::register_sum_func(THD *th
>
>
> Item_sum::Item_sum(List<Item> &list) :arg_count(list.elements),
> - forced_const(FALSE)
> + orig_args(NULL), forced_const(FALSE)
> {
> if ((args=(Item**) sql_alloc(sizeof(Item*)*arg_count)))
> {
> @@ -379,10 +379,12 @@ Item_sum::Item_sum(List<Item> &list) :ar
> */
>
> Item_sum::Item_sum(THD *thd, Item_sum *item):
> - Item_result_field(thd, item), arg_count(item->arg_count),
> + Item_result_field(thd, item),
> aggr_sel(item->aggr_sel),
> nest_level(item->nest_level), aggr_level(item->aggr_level),
> - quick_group(item->quick_group), used_tables_cache(item->used_tables_cache),
> + quick_group(item->quick_group),
> + arg_count(item->arg_count), orig_args(NULL),
> + used_tables_cache(item->used_tables_cache),
> forced_const(item->forced_const)
> {
> if (arg_count <= 2)
> @@ -425,12 +427,13 @@ void Item_sum::make_field(Send_field *tm
>
> void Item_sum::print(String *str, enum_query_type query_type)
> {
> + Item **pargs= orig_args ? orig_args : args;
> str->append(func_name());
> for (uint i=0 ; i < arg_count ; i++)
> {
> if (i)
> str->append(',');
> - args[i]->print(str, query_type);
> + pargs[i]->print(str, query_type);
> }
> str->append(')');
> }
> @@ -535,6 +538,18 @@ void Item_sum::update_used_tables ()
> }
>
>
> +Item *Item_sum::set_arg(int i, THD *thd, Item *new_val)
> +{
> + if (!orig_args)
> + {
> + orig_args= (Item **) sql_alloc (sizeof (Item *) * arg_count);
> + memcpy (orig_args, args, sizeof (Item *) * arg_count);
> + }
> + thd->change_item_tree(args + i, new_val);
> + return new_val;
> +}
> +
> +
> String *
> Item_sum_num::val_str(String *str)
> {
>
> === modified file 'sql/item_sum.h'
> --- a/sql/item_sum.h 2008-03-28 15:09:14 +0000
> +++ b/sql/item_sum.h 2008-06-12 13:23:09 +0000
> @@ -228,10 +228,8 @@ public:
> VARIANCE_FUNC, SUM_BIT_FUNC, UDF_SUM_FUNC, GROUP_CONCAT_FUNC
> };
>
> - Item **args, *tmp_args[2];
> Item **ref_by; /* pointer to a ref to the object used to register it */
> Item_sum *next; /* next in the circular chain of registered objects */
> - uint arg_count;
> Item_sum *in_sum_func; /* embedding set function if any */
> st_select_lex * aggr_sel; /* select where the function is aggregated */
> int8 nest_level; /* number of the nesting level of the set function */
> @@ -248,24 +246,32 @@ public:
> List<Item_field> outer_fields;
>
> protected:
> + uint arg_count;
> + Item **args, *tmp_args[2];
> + /*
> + Copy of args (for EXPLAIN EXTENDED) when source transformations on them
> + is needed.
* Please fix the grammar.
* Please rephrase "source transformations is needed" as this will leave
nearly everybody to wonder what are the "transformations" and when they "are
needed".
> + */
> + Item **orig_args;
> table_map used_tables_cache;
> bool forced_const;
>
> public:
>
> void mark_as_sum_func();
> - Item_sum() :arg_count(0), quick_group(1), forced_const(FALSE)
> + Item_sum() :quick_group(1), arg_count(0), orig_args(NULL),
> + forced_const(FALSE)
> {
> mark_as_sum_func();
> }
> - Item_sum(Item *a) :args(tmp_args), arg_count(1), quick_group(1),
> - forced_const(FALSE)
> + Item_sum(Item *a) :quick_group(1), arg_count(1), args(tmp_args),
> + orig_args(NULL), forced_const(FALSE)
> {
> args[0]=a;
> mark_as_sum_func();
> }
> - Item_sum( Item *a, Item *b ) :args(tmp_args), arg_count(2), quick_group(1),
> - forced_const(FALSE)
> + Item_sum( Item *a, Item *b ) :quick_group(1), arg_count(2), args(tmp_args),
> + orig_args(NULL), forced_const(FALSE)
> {
> args[0]=a; args[1]=b;
> mark_as_sum_func();
> @@ -374,6 +380,10 @@ public:
> bool register_sum_func(THD *thd, Item **ref);
> st_select_lex *depended_from()
> { return (nest_level == aggr_level ? 0 : aggr_sel); }
> +
> + Item *get_arg(int i) { return args[i]; }
> + Item *set_arg(int i, THD *thd, Item *new_val);
> + uint get_arg_count() { return arg_count; }
> };
>
>
>
> === modified file 'sql/opt_range.cc'
> --- a/sql/opt_range.cc 2008-04-14 10:58:53 +0000
> +++ b/sql/opt_range.cc 2008-06-12 13:23:09 +0000
> @@ -9157,7 +9157,7 @@ get_best_group_min_max(PARAM *param, SEL
> DBUG_RETURN(NULL);
>
> /* The argument of MIN/MAX. */
> - Item *expr= min_max_item->args[0]->real_item();
> + Item *expr= min_max_item->get_arg(0)->real_item();
> if (expr->type() == Item::FIELD_ITEM) /* Is it an attribute? */
> {
> if (! min_max_arg_item)
>
> === modified file 'sql/opt_sum.cc'
> --- a/sql/opt_sum.cc 2007-12-20 21:11:37 +0000
> +++ b/sql/opt_sum.cc 2008-06-12 13:23:09 +0000
> @@ -199,7 +199,7 @@ int opt_sum_query(TABLE_LIST *tables, Li
> to the number of rows in the tables if this number is exact and
> there are no outer joins.
> */
> - if (!conds && !((Item_sum_count*) item)->args[0]->maybe_null
> &&
> + if (!conds && !((Item_sum_count*)
> item)->get_arg(0)->maybe_null &&
> !outer_tables && maybe_exact_count)
> {
> if (!is_exact_count)
> @@ -225,7 +225,7 @@ int opt_sum_query(TABLE_LIST *tables, Li
> parts of the key is found in the COND, then we can use
> indexes to find the key.
> */
> - Item *expr=item_sum->args[0];
> + Item *expr=item_sum->get_arg(0);
> if (expr->real_item()->type() == Item::FIELD_ITEM)
> {
> uchar key_buff[MAX_KEY_LENGTH];
> @@ -373,7 +373,7 @@ int opt_sum_query(TABLE_LIST *tables, Li
> parts of the key is found in the COND, then we can use
> indexes to find the key.
> */
> - Item *expr=item_sum->args[0];
> + Item *expr=item_sum->get_arg(0);
> if (expr->real_item()->type() == Item::FIELD_ITEM)
> {
> uchar key_buff[MAX_KEY_LENGTH];
>
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc 2008-05-16 16:03:50 +0000
> +++ b/sql/sql_select.cc 2008-06-12 13:23:09 +0000
> @@ -9780,11 +9780,11 @@ create_tmp_table(THD *thd,TMP_TABLE_PARA
> }
> if (type == Item::SUM_FUNC_ITEM && !group && !save_sum_fields)
> { /* Can't calc group yet */
> - ((Item_sum*) item)->result_field=0;
> - for (i=0 ; i < ((Item_sum*) item)->arg_count ; i++)
> + Item_sum *sum_item= (Item_sum *) item;
> + sum_item->result_field=0;
> + for (i=0 ; i < sum_item->get_arg_count() ; i++)
> {
> - Item **argp= ((Item_sum*) item)->args + i;
> - Item *arg= *argp;
> + Item *arg= sum_item->get_arg(i);
> if (!arg->const_item())
> {
> Field *new_field=
> @@ -9812,7 +9812,7 @@ create_tmp_table(THD *thd,TMP_TABLE_PARA
> string_total_length+= new_field->pack_length();
> }
> thd->mem_root= mem_root_save;
> - thd->change_item_tree(argp, new Item_field(new_field));
> + arg= sum_item->set_arg(i, thd, new Item_field(new_field));
> thd->mem_root= &table->mem_root;
> if (!(new_field->flags & NOT_NULL_FLAG))
> {
> @@ -9821,7 +9821,7 @@ create_tmp_table(THD *thd,TMP_TABLE_PARA
> new_field->maybe_null() is still false, it will be
> changed below. But we have to setup Item_field correctly
> */
> - (*argp)->maybe_null=1;
> + arg->maybe_null=1;
> }
> new_field->field_index= fieldnr++;
> }
> @@ -14491,9 +14491,9 @@ count_field_types(SELECT_LEX *select_lex
> param->quick_group=0; // UDF SUM function
> param->sum_func_count++;
>
> - for (uint i=0 ; i < sum_item->arg_count ; i++)
> + for (uint i=0 ; i < sum_item->get_arg_count() ; i++)
> {
> - if (sum_item->args[0]->real_item()->type() ==
> Item::FIELD_ITEM)
> + if (sum_item->get_arg(i)->real_item()->type() ==
> Item::FIELD_ITEM)
> param->field_count++;
> else
> param->func_count++;
BR
Sergey
--
Sergey Petrunia, Lead Software Engineer
MySQL AB, www.mysql.com
Office: N/A
Blog: http://s.petrunia.net/blog
