Skip to content

Commit d6371d3

Browse files
spetruniaspetrunia
committed
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.
1 parent 21adad0 commit d6371d3

File tree

8 files changed

+279
-9
lines changed

8 files changed

+279
-9
lines changed

‎mysql-test/r/analyze_format_json.result‎

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -580,3 +580,186 @@ ANALYZE
580580
}
581581
}
582582
drop table t0, t1, t2;
583+
#
584+
# MDEV-7267: Server crashes in Item_field::print on ANALYZE FORMAT=JSON
585+
#
586+
CREATE TABLE t1 (a INT);
587+
INSERT INTO t1 VALUES (1),(2);
588+
CREATE TABLE t2 (b INT);
589+
INSERT INTO t2 VALUES (3),(4);
590+
ANALYZE FORMAT=JSON SELECT STRAIGHT_JOIN * FROM t1, t2 WHERE b IN ( SELECT a FROM t1 );
591+
ANALYZE
592+
{
593+
"query_block": {
594+
"select_id": 1,
595+
"r_loops": 1,
596+
"volatile parameter": "REPLACED",
597+
"table": {
598+
"table_name": "t1",
599+
"access_type": "ALL",
600+
"r_loops": 1,
601+
"rows": 2,
602+
"r_rows": 2,
603+
"volatile parameter": "REPLACED",
604+
"filtered": 100,
605+
"r_filtered": 100
606+
},
607+
"block-nl-join": {
608+
"table": {
609+
"table_name": "<subquery2>",
610+
"access_type": "ALL",
611+
"possible_keys": ["distinct_key"],
612+
"r_loops": 1,
613+
"rows": 2,
614+
"r_rows": 2,
615+
"volatile parameter": "REPLACED",
616+
"filtered": 100,
617+
"r_filtered": 100
618+
},
619+
"buffer_type": "flat",
620+
"buffer_size": "256Kb",
621+
"join_type": "BNL",
622+
"r_filtered": 100,
623+
"materialized": {
624+
"unique": 1,
625+
"query_block": {
626+
"select_id": 2,
627+
"r_loops": 1,
628+
"volatile parameter": "REPLACED",
629+
"table": {
630+
"table_name": "t1",
631+
"access_type": "ALL",
632+
"r_loops": 1,
633+
"rows": 2,
634+
"r_rows": 2,
635+
"volatile parameter": "REPLACED",
636+
"filtered": 100,
637+
"r_filtered": 100
638+
}
639+
}
640+
}
641+
},
642+
"block-nl-join": {
643+
"table": {
644+
"table_name": "t2",
645+
"access_type": "ALL",
646+
"r_loops": 1,
647+
"rows": 2,
648+
"r_rows": 2,
649+
"volatile parameter": "REPLACED",
650+
"filtered": 100,
651+
"r_filtered": 100
652+
},
653+
"buffer_type": "incremental",
654+
"buffer_size": "256Kb",
655+
"join_type": "BNL",
656+
"attached_condition": "(t2.b = `<subquery2>`.a)",
657+
"r_filtered": 0
658+
}
659+
}
660+
}
661+
drop table t1,t2;
662+
#
663+
# MDEV-8864: Server crash #2 in Item_field::print on ANALYZE FORMAT=JSON
664+
#
665+
CREATE TABLE t1 (f1 INT) ENGINE=MyISAM;
666+
INSERT INTO t1 VALUES (1),(2);
667+
CREATE TABLE t2 (f2 INT) ENGINE=MyISAM;
668+
INSERT INTO t2 VALUES (2),(3);
669+
CREATE TABLE t3 (f3 INT) ENGINE=MyISAM;
670+
INSERT INTO t3 VALUES (3),(4);
671+
ANALYZE FORMAT=JSON
672+
SELECT GROUP_CONCAT(f3) AS gc, ( SELECT MAX(f1) FROM t1, t2 WHERE f2 = f3 ) sq
673+
FROM t2, t3
674+
WHERE f3 IN ( 1, 2 )
675+
GROUP BY sq ORDER BY gc;
676+
ANALYZE
677+
{
678+
"query_block": {
679+
"select_id": 1,
680+
"r_loops": 1,
681+
"volatile parameter": "REPLACED",
682+
"filesort": {
683+
"r_loops": 1,
684+
"volatile parameter": "REPLACED",
685+
"r_used_priority_queue": false,
686+
"r_output_rows": 0,
687+
"volatile parameter": "REPLACED",
688+
"filesort": {
689+
"r_loops": 1,
690+
"volatile parameter": "REPLACED",
691+
"r_used_priority_queue": false,
692+
"r_output_rows": 0,
693+
"volatile parameter": "REPLACED",
694+
"temporary_table": {
695+
"temporary_table": {
696+
"table": {
697+
"table_name": "t2",
698+
"access_type": "ALL",
699+
"r_loops": 1,
700+
"rows": 2,
701+
"r_rows": 2,
702+
"volatile parameter": "REPLACED",
703+
"filtered": 100,
704+
"r_filtered": 100
705+
},
706+
"block-nl-join": {
707+
"table": {
708+
"table_name": "t3",
709+
"access_type": "ALL",
710+
"r_loops": 1,
711+
"rows": 2,
712+
"r_rows": 2,
713+
"volatile parameter": "REPLACED",
714+
"filtered": 100,
715+
"r_filtered": 0,
716+
"attached_condition": "(t3.f3 in (1,2))"
717+
},
718+
"buffer_type": "flat",
719+
"buffer_size": "256Kb",
720+
"join_type": "BNL",
721+
"r_filtered": null
722+
},
723+
"subqueries": [
724+
{
725+
"expression_cache": {
726+
"state": "uninitialized",
727+
"r_loops": 0,
728+
"query_block": {
729+
"select_id": 2,
730+
"table": {
731+
"table_name": "t1",
732+
"access_type": "ALL",
733+
"r_loops": 0,
734+
"rows": 2,
735+
"r_rows": null,
736+
"filtered": 100,
737+
"r_filtered": null
738+
},
739+
"block-nl-join": {
740+
"table": {
741+
"table_name": "t2",
742+
"access_type": "ALL",
743+
"r_loops": 0,
744+
"rows": 2,
745+
"r_rows": null,
746+
"filtered": 100,
747+
"r_filtered": null
748+
},
749+
"buffer_type": "flat",
750+
"buffer_size": "256Kb",
751+
"join_type": "BNL",
752+
"attached_condition": "(t2.f2 = t3.f3)",
753+
"r_filtered": null
754+
}
755+
}
756+
}
757+
}
758+
]
759+
}
760+
}
761+
}
762+
}
763+
}
764+
}
765+
drop table t1,t2,t3;

‎mysql-test/t/analyze_format_json.test‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,3 +176,39 @@ analyze format=json select a, max(b) as TOP from t2 group by a having 1=2;
176176
--replace_regex /"(r_total_time_ms|r_buffer_size)": .*?,/"volatile parameter": "REPLACED",/
177177
analyze format=json select a, max(b) as TOP from t2 group by a;
178178
drop table t0, t1, t2;
179+
180+
--echo #
181+
--echo # MDEV-7267: Server crashes in Item_field::print on ANALYZE FORMAT=JSON
182+
--echo #
183+
CREATE TABLE t1 (a INT);
184+
INSERT INTO t1 VALUES (1),(2);
185+
186+
CREATE TABLE t2 (b INT);
187+
INSERT INTO t2 VALUES (3),(4);
188+
189+
--replace_regex /"(r_total_time_ms|r_buffer_size)": .*?,/"volatile parameter": "REPLACED",/
190+
ANALYZE FORMAT=JSON SELECT STRAIGHT_JOIN * FROM t1, t2 WHERE b IN ( SELECT a FROM t1 );
191+
192+
drop table t1,t2;
193+
194+
--echo #
195+
--echo # MDEV-8864: Server crash #2 in Item_field::print on ANALYZE FORMAT=JSON
196+
--echo #
197+
CREATE TABLE t1 (f1 INT) ENGINE=MyISAM;
198+
INSERT INTO t1 VALUES (1),(2);
199+
200+
CREATE TABLE t2 (f2 INT) ENGINE=MyISAM;
201+
INSERT INTO t2 VALUES (2),(3);
202+
203+
CREATE TABLE t3 (f3 INT) ENGINE=MyISAM;
204+
INSERT INTO t3 VALUES (3),(4);
205+
206+
--replace_regex /"(r_total_time_ms|r_buffer_size)": .*?,/"volatile parameter": "REPLACED",/
207+
ANALYZE FORMAT=JSON
208+
SELECT GROUP_CONCAT(f3) AS gc, ( SELECT MAX(f1) FROM t1, t2 WHERE f2 = f3 ) sq
209+
FROM t2, t3
210+
WHERE f3 IN ( 1, 2 )
211+
GROUP BY sq ORDER BY gc;
212+
213+
drop table t1,t2,t3;
214+

‎sql/item.cc‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2636,7 +2636,7 @@ void Item_field::fix_after_pullout(st_select_lex *new_parent, Item **ref)
26362636

26372637
Item *Item_field::get_tmp_table_item(THD *thd)
26382638
{
2639-
Item_field *new_item= new (thd->mem_root) Item_field(thd, this);
2639+
Item_field *new_item= new (thd->mem_root) Item_temptable_field(thd, this);
26402640
if (new_item)
26412641
new_item->field= new_item->result_field;
26422642
return new_item;
@@ -6611,6 +6611,16 @@ void Item_field::print(String *str, enum_query_type query_type)
66116611
}
66126612

66136613

6614+
void Item_temptable_field::print(String *str, enum_query_type query_type)
6615+
{
6616+
/*
6617+
Item_ident doesn't have references to the underlying Field/TABLE objects,
6618+
so it's ok to use the following:
6619+
*/
6620+
Item_ident::print(str, query_type);
6621+
}
6622+
6623+
66146624
Item_ref::Item_ref(THD *thd, Name_resolution_context *context_arg,
66156625
Item **item, const char *table_name_arg,
66166626
const char *field_name_arg,
@@ -7800,7 +7810,7 @@ int Item_cache_wrapper::save_in_field(Field *to, bool no_conversions)
78007810
Item* Item_cache_wrapper::get_tmp_table_item(THD *thd)
78017811
{
78027812
if (!orig_item->with_sum_func && !orig_item->const_item())
7803-
return new (thd->mem_root) Item_field(thd, result_field);
7813+
return new (thd->mem_root) Item_temptable_field(thd, result_field);
78047814
return copy_or_same(thd);
78057815
}
78067816

‎sql/item.h‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2480,6 +2480,47 @@ class Item_field :public Item_ident
24802480
friend class st_select_lex_unit;
24812481
};
24822482

2483+
2484+
/*
2485+
@brief
2486+
Item_temptable_field is the same as Item_field, except that print()
2487+
continues to work even if the table has been dropped.
2488+
2489+
@detail
2490+
2491+
We need this item for "ANALYZE statement" feature. Query execution has
2492+
these steps:
2493+
2494+
1. Run the query.
2495+
2. Cleanup starts. Temporary tables are destroyed
2496+
3. print "ANALYZE statement" output, if needed
2497+
4. Call close_thread_table() for regular tables.
2498+
2499+
Step #4 is done after step #3, so "ANALYZE stmt" has no problem printing
2500+
Item_field objects that refer to regular tables.
2501+
2502+
However, Step #3 is done after Step #2. Attempt to print Item_field objects
2503+
that refer to temporary tables will cause access to freed memory.
2504+
2505+
To resolve this, we use Item_temptable_field to refer to items in temporary
2506+
(work) tables.
2507+
*/
2508+
2509+
class Item_temptable_field :public Item_field
2510+
{
2511+
public:
2512+
Item_temptable_field(THD *thd, Name_resolution_context *context_arg, Field *field)
2513+
: Item_field(thd, context_arg, field) {}
2514+
2515+
Item_temptable_field(THD *thd, Field *field)
2516+
: Item_field(thd, field) {}
2517+
2518+
Item_temptable_field(THD *thd, Item_field *item) : Item_field(thd, item) {};
2519+
2520+
virtual void print(String *str, enum_query_type query_type);
2521+
};
2522+
2523+
24832524
class Item_null :public Item_basic_constant
24842525
{
24852526
public:

‎sql/item_func.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -742,7 +742,7 @@ void Item_func::signal_divide_by_null()
742742
Item *Item_func::get_tmp_table_item(THD *thd)
743743
{
744744
if (!with_sum_func && !const_item())
745-
return new (thd->mem_root) Item_field(thd, result_field);
745+
return new (thd->mem_root) Item_temptable_field(thd, result_field);
746746
return copy_or_same(thd);
747747
}
748748

‎sql/item_subselect.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -878,7 +878,7 @@ bool Item_subselect::const_item() const
878878
Item *Item_subselect::get_tmp_table_item(THD *thd_arg)
879879
{
880880
if (!with_sum_func && !const_item())
881-
return new (thd->mem_root) Item_field(thd_arg, result_field);
881+
return new (thd->mem_root) Item_temptable_field(thd_arg, result_field);
882882
return copy_or_same(thd_arg);
883883
}
884884

@@ -4942,7 +4942,7 @@ bool subselect_hash_sj_engine::make_semi_join_conds()
49424942
Item_field *right_col_item;
49434943

49444944
if (!(right_col_item= new (thd->mem_root)
4945-
Item_field(thd, context, tmp_table->field[i])) ||
4945+
Item_temptable_field(thd, context, tmp_table->field[i])) ||
49464946
!(eq_cond= new (thd->mem_root)
49474947
Item_func_eq(thd, item_in->left_expr->element_index(i),
49484948
right_col_item)) ||

‎sql/item_sum.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,7 @@ Item *Item_sum::get_tmp_table_item(THD *thd)
490490
if (arg->type() == Item::FIELD_ITEM)
491491
((Item_field*) arg)->field= result_field_tmp++;
492492
else
493-
sum_item->args[i]= new (thd->mem_root) Item_field(thd, result_field_tmp++);
493+
sum_item->args[i]= new (thd->mem_root) Item_temptable_field(thd, result_field_tmp++);
494494
}
495495
}
496496
}

‎sql/sql_select.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16439,7 +16439,7 @@ create_tmp_table(THD *thd, TMP_TABLE_PARAM *param, List<Item> &fields,
1643916439
string_total_length+= new_field->pack_length();
1644016440
}
1644116441
thd->mem_root= mem_root_save;
16442-
arg= sum_item->set_arg(i, thd, new (thd->mem_root) Item_field(thd, new_field));
16442+
arg= sum_item->set_arg(i, thd, new (thd->mem_root) Item_temptable_field(thd, new_field));
1644316443
thd->mem_root= &table->mem_root;
1644416444
if (param->force_not_null_cols)
1644516445
{
@@ -22917,7 +22917,7 @@ change_to_use_tmp_fields(THD *thd, Item **ref_pointer_array,
2291722917
*/
2291822918
Item_func_set_user_var* suv=
2291922919
new (thd->mem_root) Item_func_set_user_var(thd, (Item_func_set_user_var*) item);
22920-
Item_field *new_field= new (thd->mem_root) Item_field(thd, field);
22920+
Item_field *new_field= new (thd->mem_root) Item_temptable_field(thd, field);
2292122921
if (!suv || !new_field)
2292222922
DBUG_RETURN(true); // Fatal error
2292322923
/*
@@ -22941,7 +22941,7 @@ change_to_use_tmp_fields(THD *thd, Item **ref_pointer_array,
2294122941
if (item->type() == Item::SUM_FUNC_ITEM && field->table->group)
2294222942
item_field= ((Item_sum*) item)->result_item(thd, field);
2294322943
else
22944-
item_field= (Item *) new (thd->mem_root) Item_field(thd, field);
22944+
item_field= (Item *) new (thd->mem_root) Item_temptable_field(thd, field);
2294522945
if (!item_field)
2294622946
DBUG_RETURN(true); // Fatal error
2294722947

0 commit comments

Comments
 (0)