From d6371d3a8eccedb056708b9d8abae9cc4db3ed4e Mon Sep 17 00:00:00 2001 From: Sergei Petrunia Date: Tue, 6 Oct 2015 18:03:10 +0300 Subject: Combined fix for MDEV-7267 and MDEV-8864 The problem was that GROUP BY code created Item_field objects that referred to fields in the temp. tables used for GROUP BY. Item_ref and set_items_ref_array() call caused pointers to temp. table fields to occur in many places. This patch introduces Item_temptable_field, which can handle item->print() calls made after the underlying table is freed. --- mysql-test/r/analyze_format_json.result | 183 ++++++++++++++++++++++++++++++++ mysql-test/t/analyze_format_json.test | 36 +++++++ sql/item.cc | 14 ++- sql/item.h | 41 +++++++ sql/item_func.cc | 2 +- sql/item_subselect.cc | 4 +- sql/item_sum.cc | 2 +- sql/sql_select.cc | 6 +- 8 files changed, 279 insertions(+), 9 deletions(-) diff --git a/mysql-test/r/analyze_format_json.result b/mysql-test/r/analyze_format_json.result index 14ab125b9ac..69930bc0215 100644 --- a/mysql-test/r/analyze_format_json.result +++ b/mysql-test/r/analyze_format_json.result @@ -580,3 +580,186 @@ ANALYZE } } drop table t0, t1, t2; +# +# MDEV-7267: Server crashes in Item_field::print on ANALYZE FORMAT=JSON +# +CREATE TABLE t1 (a INT); +INSERT INTO t1 VALUES (1),(2); +CREATE TABLE t2 (b INT); +INSERT INTO t2 VALUES (3),(4); +ANALYZE FORMAT=JSON SELECT STRAIGHT_JOIN * FROM t1, t2 WHERE b IN ( SELECT a FROM t1 ); +ANALYZE +{ + "query_block": { + "select_id": 1, + "r_loops": 1, + "volatile parameter": "REPLACED", + "table": { + "table_name": "t1", + "access_type": "ALL", + "r_loops": 1, + "rows": 2, + "r_rows": 2, + "volatile parameter": "REPLACED", + "filtered": 100, + "r_filtered": 100 + }, + "block-nl-join": { + "table": { + "table_name": "", + "access_type": "ALL", + "possible_keys": ["distinct_key"], + "r_loops": 1, + "rows": 2, + "r_rows": 2, + "volatile parameter": "REPLACED", + "filtered": 100, + "r_filtered": 100 + }, + "buffer_type": "flat", + "buffer_size": "256Kb", + "join_type": "BNL", + "r_filtered": 100, + "materialized": { + "unique": 1, + "query_block": { + "select_id": 2, + "r_loops": 1, + "volatile parameter": "REPLACED", + "table": { + "table_name": "t1", + "access_type": "ALL", + "r_loops": 1, + "rows": 2, + "r_rows": 2, + "volatile parameter": "REPLACED", + "filtered": 100, + "r_filtered": 100 + } + } + } + }, + "block-nl-join": { + "table": { + "table_name": "t2", + "access_type": "ALL", + "r_loops": 1, + "rows": 2, + "r_rows": 2, + "volatile parameter": "REPLACED", + "filtered": 100, + "r_filtered": 100 + }, + "buffer_type": "incremental", + "buffer_size": "256Kb", + "join_type": "BNL", + "attached_condition": "(t2.b = ``.a)", + "r_filtered": 0 + } + } +} +drop table t1,t2; +# +# MDEV-8864: Server crash #2 in Item_field::print on ANALYZE FORMAT=JSON +# +CREATE TABLE t1 (f1 INT) ENGINE=MyISAM; +INSERT INTO t1 VALUES (1),(2); +CREATE TABLE t2 (f2 INT) ENGINE=MyISAM; +INSERT INTO t2 VALUES (2),(3); +CREATE TABLE t3 (f3 INT) ENGINE=MyISAM; +INSERT INTO t3 VALUES (3),(4); +ANALYZE FORMAT=JSON +SELECT GROUP_CONCAT(f3) AS gc, ( SELECT MAX(f1) FROM t1, t2 WHERE f2 = f3 ) sq +FROM t2, t3 +WHERE f3 IN ( 1, 2 ) +GROUP BY sq ORDER BY gc; +ANALYZE +{ + "query_block": { + "select_id": 1, + "r_loops": 1, + "volatile parameter": "REPLACED", + "filesort": { + "r_loops": 1, + "volatile parameter": "REPLACED", + "r_used_priority_queue": false, + "r_output_rows": 0, + "volatile parameter": "REPLACED", + "filesort": { + "r_loops": 1, + "volatile parameter": "REPLACED", + "r_used_priority_queue": false, + "r_output_rows": 0, + "volatile parameter": "REPLACED", + "temporary_table": { + "temporary_table": { + "table": { + "table_name": "t2", + "access_type": "ALL", + "r_loops": 1, + "rows": 2, + "r_rows": 2, + "volatile parameter": "REPLACED", + "filtered": 100, + "r_filtered": 100 + }, + "block-nl-join": { + "table": { + "table_name": "t3", + "access_type": "ALL", + "r_loops": 1, + "rows": 2, + "r_rows": 2, + "volatile parameter": "REPLACED", + "filtered": 100, + "r_filtered": 0, + "attached_condition": "(t3.f3 in (1,2))" + }, + "buffer_type": "flat", + "buffer_size": "256Kb", + "join_type": "BNL", + "r_filtered": null + }, + "subqueries": [ + { + "expression_cache": { + "state": "uninitialized", + "r_loops": 0, + "query_block": { + "select_id": 2, + "table": { + "table_name": "t1", + "access_type": "ALL", + "r_loops": 0, + "rows": 2, + "r_rows": null, + "filtered": 100, + "r_filtered": null + }, + "block-nl-join": { + "table": { + "table_name": "t2", + "access_type": "ALL", + "r_loops": 0, + "rows": 2, + "r_rows": null, + "filtered": 100, + "r_filtered": null + }, + "buffer_type": "flat", + "buffer_size": "256Kb", + "join_type": "BNL", + "attached_condition": "(t2.f2 = t3.f3)", + "r_filtered": null + } + } + } + } + ] + } + } + } + } + } +} +drop table t1,t2,t3; diff --git a/mysql-test/t/analyze_format_json.test b/mysql-test/t/analyze_format_json.test index db626dc387e..807e02d2334 100644 --- a/mysql-test/t/analyze_format_json.test +++ b/mysql-test/t/analyze_format_json.test @@ -176,3 +176,39 @@ analyze format=json select a, max(b) as TOP from t2 group by a having 1=2; --replace_regex /"(r_total_time_ms|r_buffer_size)": .*?,/"volatile parameter": "REPLACED",/ analyze format=json select a, max(b) as TOP from t2 group by a; drop table t0, t1, t2; + +--echo # +--echo # MDEV-7267: Server crashes in Item_field::print on ANALYZE FORMAT=JSON +--echo # +CREATE TABLE t1 (a INT); +INSERT INTO t1 VALUES (1),(2); + +CREATE TABLE t2 (b INT); +INSERT INTO t2 VALUES (3),(4); + +--replace_regex /"(r_total_time_ms|r_buffer_size)": .*?,/"volatile parameter": "REPLACED",/ +ANALYZE FORMAT=JSON SELECT STRAIGHT_JOIN * FROM t1, t2 WHERE b IN ( SELECT a FROM t1 ); + +drop table t1,t2; + +--echo # +--echo # MDEV-8864: Server crash #2 in Item_field::print on ANALYZE FORMAT=JSON +--echo # +CREATE TABLE t1 (f1 INT) ENGINE=MyISAM; +INSERT INTO t1 VALUES (1),(2); + +CREATE TABLE t2 (f2 INT) ENGINE=MyISAM; +INSERT INTO t2 VALUES (2),(3); + +CREATE TABLE t3 (f3 INT) ENGINE=MyISAM; +INSERT INTO t3 VALUES (3),(4); + +--replace_regex /"(r_total_time_ms|r_buffer_size)": .*?,/"volatile parameter": "REPLACED",/ +ANALYZE FORMAT=JSON +SELECT GROUP_CONCAT(f3) AS gc, ( SELECT MAX(f1) FROM t1, t2 WHERE f2 = f3 ) sq +FROM t2, t3 +WHERE f3 IN ( 1, 2 ) +GROUP BY sq ORDER BY gc; + +drop table t1,t2,t3; + diff --git a/sql/item.cc b/sql/item.cc index 29bc4c1b4f2..80ed2bf8109 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -2636,7 +2636,7 @@ void Item_field::fix_after_pullout(st_select_lex *new_parent, Item **ref) Item *Item_field::get_tmp_table_item(THD *thd) { - Item_field *new_item= new (thd->mem_root) Item_field(thd, this); + Item_field *new_item= new (thd->mem_root) Item_temptable_field(thd, this); if (new_item) new_item->field= new_item->result_field; return new_item; @@ -6611,6 +6611,16 @@ void Item_field::print(String *str, enum_query_type query_type) } +void Item_temptable_field::print(String *str, enum_query_type query_type) +{ + /* + Item_ident doesn't have references to the underlying Field/TABLE objects, + so it's ok to use the following: + */ + Item_ident::print(str, query_type); +} + + Item_ref::Item_ref(THD *thd, Name_resolution_context *context_arg, Item **item, const char *table_name_arg, const char *field_name_arg, @@ -7800,7 +7810,7 @@ int Item_cache_wrapper::save_in_field(Field *to, bool no_conversions) Item* Item_cache_wrapper::get_tmp_table_item(THD *thd) { if (!orig_item->with_sum_func && !orig_item->const_item()) - return new (thd->mem_root) Item_field(thd, result_field); + return new (thd->mem_root) Item_temptable_field(thd, result_field); return copy_or_same(thd); } diff --git a/sql/item.h b/sql/item.h index 440caef6819..5b07885eb14 100644 --- a/sql/item.h +++ b/sql/item.h @@ -2480,6 +2480,47 @@ public: friend class st_select_lex_unit; }; + +/* + @brief + Item_temptable_field is the same as Item_field, except that print() + continues to work even if the table has been dropped. + + @detail + + We need this item for "ANALYZE statement" feature. Query execution has + these steps: + + 1. Run the query. + 2. Cleanup starts. Temporary tables are destroyed + 3. print "ANALYZE statement" output, if needed + 4. Call close_thread_table() for regular tables. + + Step #4 is done after step #3, so "ANALYZE stmt" has no problem printing + Item_field objects that refer to regular tables. + + However, Step #3 is done after Step #2. Attempt to print Item_field objects + that refer to temporary tables will cause access to freed memory. + + To resolve this, we use Item_temptable_field to refer to items in temporary + (work) tables. +*/ + +class Item_temptable_field :public Item_field +{ +public: + Item_temptable_field(THD *thd, Name_resolution_context *context_arg, Field *field) + : Item_field(thd, context_arg, field) {} + + Item_temptable_field(THD *thd, Field *field) + : Item_field(thd, field) {} + + Item_temptable_field(THD *thd, Item_field *item) : Item_field(thd, item) {}; + + virtual void print(String *str, enum_query_type query_type); +}; + + class Item_null :public Item_basic_constant { public: diff --git a/sql/item_func.cc b/sql/item_func.cc index de718b738e4..725f712546e 100644 --- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -742,7 +742,7 @@ void Item_func::signal_divide_by_null() Item *Item_func::get_tmp_table_item(THD *thd) { if (!with_sum_func && !const_item()) - return new (thd->mem_root) Item_field(thd, result_field); + return new (thd->mem_root) Item_temptable_field(thd, result_field); return copy_or_same(thd); } diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc index 4e27eaf24cf..ddfe90fed59 100644 --- a/sql/item_subselect.cc +++ b/sql/item_subselect.cc @@ -878,7 +878,7 @@ bool Item_subselect::const_item() const Item *Item_subselect::get_tmp_table_item(THD *thd_arg) { if (!with_sum_func && !const_item()) - return new (thd->mem_root) Item_field(thd_arg, result_field); + return new (thd->mem_root) Item_temptable_field(thd_arg, result_field); return copy_or_same(thd_arg); } @@ -4942,7 +4942,7 @@ bool subselect_hash_sj_engine::make_semi_join_conds() Item_field *right_col_item; if (!(right_col_item= new (thd->mem_root) - Item_field(thd, context, tmp_table->field[i])) || + Item_temptable_field(thd, context, tmp_table->field[i])) || !(eq_cond= new (thd->mem_root) Item_func_eq(thd, item_in->left_expr->element_index(i), right_col_item)) || diff --git a/sql/item_sum.cc b/sql/item_sum.cc index 4cbbc274526..58be20e0322 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -490,7 +490,7 @@ Item *Item_sum::get_tmp_table_item(THD *thd) if (arg->type() == Item::FIELD_ITEM) ((Item_field*) arg)->field= result_field_tmp++; else - sum_item->args[i]= new (thd->mem_root) Item_field(thd, result_field_tmp++); + sum_item->args[i]= new (thd->mem_root) Item_temptable_field(thd, result_field_tmp++); } } } diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 53ca3c32f14..00309878955 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -16439,7 +16439,7 @@ create_tmp_table(THD *thd, TMP_TABLE_PARAM *param, List &fields, string_total_length+= new_field->pack_length(); } thd->mem_root= mem_root_save; - arg= sum_item->set_arg(i, thd, new (thd->mem_root) Item_field(thd, new_field)); + arg= sum_item->set_arg(i, thd, new (thd->mem_root) Item_temptable_field(thd, new_field)); thd->mem_root= &table->mem_root; if (param->force_not_null_cols) { @@ -22917,7 +22917,7 @@ change_to_use_tmp_fields(THD *thd, Item **ref_pointer_array, */ Item_func_set_user_var* suv= new (thd->mem_root) Item_func_set_user_var(thd, (Item_func_set_user_var*) item); - Item_field *new_field= new (thd->mem_root) Item_field(thd, field); + Item_field *new_field= new (thd->mem_root) Item_temptable_field(thd, field); if (!suv || !new_field) DBUG_RETURN(true); // Fatal error /* @@ -22941,7 +22941,7 @@ change_to_use_tmp_fields(THD *thd, Item **ref_pointer_array, if (item->type() == Item::SUM_FUNC_ITEM && field->table->group) item_field= ((Item_sum*) item)->result_item(thd, field); else - item_field= (Item *) new (thd->mem_root) Item_field(thd, field); + item_field= (Item *) new (thd->mem_root) Item_temptable_field(thd, field); if (!item_field) DBUG_RETURN(true); // Fatal error -- cgit v1.2.1