Skip to content

Commit 0dea7c9

Browse files
author
Guilhem Bichot
committed
Fix for
Bug#16739050 PREPARED STATEMENT: CRASH IN ITEM_REF::REAL_ITEM OR PURE VIRTUAL FUNCTION CALL and 2 duplicates: don't reach to real items when creating prep_having. Details: First, let's review some bugs fixed in older 5.6 versions. ========================================================== About the fix for 12603457 and 12603141: WHERE condition is "WHERE field". JOIN::conds is made, by change_item_tree(), to point to item_outer_ref which points to item_field. At fix_prepare_information() we do: prep_where= JOIN::conds When we rollback_item_tree_changes(), we restore conds, which does not modify prep_where (only JOIN::conds was registered in the "list of changes to roll back"...). When next execution starts reinit_stmt_before_use() copies prep_where into select_lex->where, which thus gets the now-destroyed item_outer_ref, later this crashes. Solution which was chosen: make reinit_stmt_before_use() copy prep_where->real_item(), to reach to the field, avoiding the now-destroyed item_outer_ref. For uniformity, the same was done for prep_having. About the fix for 12582849: (cousin of above) WHERE condition is OR. One argument of OR is made to point to item_outer_ref which points to item_field. At JOIN:: optimize: prep_where= copy of conds => makes a copy of OR (a new item_cond_or). When we rollback_item_tree_changes(), we restore one pointer argument of the original OR, which does not modify the pointer argument of the copy of this OR (do not modify prep_where); the now-destroyed item_outer_ref remains in prep_where. Then it crashes at next execution. Solution which was chosen: make the copy code (copy_andor_structure) use real_item(). About the fix for Bug #16163596: regression caused by the fix for 12603457 and 12603141. In sql_yacc, "HAVING a" leads to the creation of an Item_ref named "a". In having->fix_fields(), we find that "a" is actually a field, so we make the Item_ref's ref point to this field. At next execution, reinit_stmt_before_use() uses real_item() which makes the HAVING be item_field, not Item_ref anymore. This Item_ref should have staid, it's a permanent, parse-time item. There should never be Item_field in HAVING, see how sql_yacc creates Item_ref instead of Item_field if in HAVING. Removing this item_ref confused ROLLUP. Solution which was chosen: undo the HAVING part of the fix for 12603457 and 12603141. Now the new bugs. ================= 16317817: HAVING condition is OR. We resolve GROUP BY: - initially order->item points to order->item_ptr which contains (per parser) Item_field. - in find_order_in_list(), *order_item (i.e. order->item_ptr) is made to point to item_outer_ref, this change done in fix_fields() goes through register_item_tree_change(). - again in find_order_in_list(), we append this item to ref_pointer_array, and make order->item point to this new cell of the array. One argument of OR in HAVING is item_ref (per parser). We resolve HAVING: this argument of OR is made to reference order->item (id est: the ref_pointer_array cell which references the item of GROUP BY). Keep in mind that Item_ref is Item**, not Item*. Then we rollback_item_tree_changes(), which effectively restores order->item_ptr. reinit_stmt_before_use() restores order -> item to point to order -> item_ptr. So GROUP BY is fine. But the item_ref in HAVING still references the cell of ref_pointer_array which was not restored. reinit_stmt_before_use() makes a copy of HAVING, which uses copy_andor_structure, which reaches to the real items (per the fix for 12582849), but as HAVING is "item_ref to destroyed item_outer_ref to item_field", this crashes. If it would not reach to the real item, the copy would stop at the item_ref, then the soon-coming name resolution would create a new item_outer_ref and point the item_ref to it, all fine. 16739050: similar to above 16317685: HAVING condition is OR. In HAVING, one Item_ref is made to reference COUNT; reinit_stmt_before_use(), using copy_andor_structure() which uses real_item(), changes HAVING to COUNT, then ROLLUP crashes. PROPOSED SOLUTION ================= Avoid using real_item() in copy_andor_structure() when copying HAVING, because it's only causing problems. Keep real_item() when copying WHERE, in 5.6. We cannot remove it, because of the "second copy" done in JOIN::optimize(): sel->prep_where= conds ? conds->copy_andor_structure(thd, true) : NULL; which creates new AND/OR items which "argument pointers" will not be properly restored (re-read 12582849 above), thus forcing us to reach to the real items. In WL#7082 (5.7), the second copy will go away, so real_item() can be removed (done in my prototype).
1 parent bb63a58 commit 0dea7c9

6 files changed

Lines changed: 40 additions & 13 deletions

File tree

‎sql/item.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5390,7 +5390,7 @@ bool Item_field::fix_fields(THD *thd, Item **reference)
53905390
It's not an Item_field in the select list so we must make a new
53915391
Item_ref to point to the Item in the select list and replace the
53925392
Item_field created by the parser with the new Item_ref.
5393-
5393+
Ex: SELECT func1(col) as c ... ORDER BY func2(c);
53945394
NOTE: If we are fixing an alias reference inside ORDER/GROUP BY
53955395
item tree, then we use new Item_ref as an intermediate value
53965396
to resolve referenced item only.

‎sql/item.h‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1338,7 +1338,13 @@ class Item
13381338
*/
13391339
virtual void no_rows_in_result() {}
13401340
virtual Item *copy_or_same(THD *thd) { return this; }
1341-
virtual Item *copy_andor_structure(THD *thd) { return this; }
1341+
/**
1342+
@param real_items True <=> in the copy, replace any Item_ref with its
1343+
real_item()
1344+
@todo this argument should be always false and removed in WL#7082.
1345+
*/
1346+
virtual Item *copy_andor_structure(THD *thd, bool real_items= false)
1347+
{ return real_items ? real_item() : this; }
13421348
virtual Item *real_item() { return this; }
13431349
virtual Item *get_tmp_table_item(THD *thd) { return copy_or_same(thd); }
13441350

‎sql/item_cmpfunc.cc‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4739,11 +4739,12 @@ Item_cond::Item_cond(THD *thd, Item_cond *item)
47394739
}
47404740

47414741

4742-
void Item_cond::copy_andor_arguments(THD *thd, Item_cond *item)
4742+
void Item_cond::copy_andor_arguments(THD *thd, Item_cond *item, bool real_items)
47434743
{
47444744
List_iterator_fast<Item> li(item->list);
47454745
while (Item *it= li++)
4746-
list.push_back(it->real_item()->copy_andor_structure(thd));
4746+
list.push_back((real_items ? it->real_item() : it)->
4747+
copy_andor_structure(thd, real_items));
47474748
}
47484749

47494750

‎sql/item_cmpfunc.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#ifndef ITEM_CMPFUNC_INCLUDED
22
#define ITEM_CMPFUNC_INCLUDED
33

4-
/* Copyright (c) 2000, 2012, Oracle and/or its affiliates. All rights reserved.
4+
/* Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved.
55
66
This program is free software; you can redistribute it and/or modify
77
it under the terms of the GNU General Public License as published by
@@ -1682,7 +1682,7 @@ class Item_cond :public Item_bool_func
16821682
friend int setup_conds(THD *thd, TABLE_LIST *tables, TABLE_LIST *leaves,
16831683
Item **conds);
16841684
void top_level_item() { abort_on_null=1; }
1685-
void copy_andor_arguments(THD *thd, Item_cond *item);
1685+
void copy_andor_arguments(THD *thd, Item_cond *item, bool real_items= false);
16861686
bool walk(Item_processor processor, bool walk_subquery, uchar *arg);
16871687
Item *transform(Item_transformer transformer, uchar *arg);
16881688
void traverse_cond(Cond_traverser, void *arg, traverse_order order);
@@ -1873,11 +1873,11 @@ class Item_cond_and :public Item_cond
18731873
enum Functype functype() const { return COND_AND_FUNC; }
18741874
longlong val_int();
18751875
const char *func_name() const { return "and"; }
1876-
Item* copy_andor_structure(THD *thd)
1876+
Item* copy_andor_structure(THD *thd, bool real_items)
18771877
{
18781878
Item_cond_and *item;
18791879
if ((item= new Item_cond_and(thd, this)))
1880-
item->copy_andor_arguments(thd, this);
1880+
item->copy_andor_arguments(thd, this, real_items);
18811881
return item;
18821882
}
18831883
Item *neg_transformer(THD *thd);
@@ -1902,11 +1902,11 @@ class Item_cond_or :public Item_cond
19021902
enum Functype functype() const { return COND_OR_FUNC; }
19031903
longlong val_int();
19041904
const char *func_name() const { return "or"; }
1905-
Item* copy_andor_structure(THD *thd)
1905+
Item* copy_andor_structure(THD *thd, bool real_items)
19061906
{
19071907
Item_cond_or *item;
19081908
if ((item= new Item_cond_or(thd, this)))
1909-
item->copy_andor_arguments(thd, this);
1909+
item->copy_andor_arguments(thd, this, real_items);
19101910
return item;
19111911
}
19121912
Item *neg_transformer(THD *thd);

‎sql/sql_lex.cc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3657,6 +3657,14 @@ void st_select_lex::fix_prepare_information(THD *thd, Item **conds,
36573657
/*
36583658
In "WHERE outer_field", *conds may be an Item_outer_ref allocated in
36593659
the execution memroot.
3660+
@todo change this line in WL#7082. Currently, when we execute a SP,
3661+
containing "SELECT (SELECT ... WHERE t1.col) FROM t1",
3662+
resolution may make *conds equal to an Item_outer_ref, then below
3663+
*conds becomes Item_field, which then goes straight on to execution,
3664+
undoing the effects of putting Item_outer_ref in the first place...
3665+
With a PS the problem is not as severe, as after the code below we
3666+
don't go to execution: a next execution will do a new name resolution
3667+
which will create Item_outer_ref again.
36603668
*/
36613669
prep_where= (*conds)->real_item();
36623670
*conds= where= prep_where->copy_andor_structure(thd);

‎sql/sql_optimizer.cc‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -218,9 +218,21 @@ JOIN::optimize()
218218
}
219219
build_bitmap_for_nested_joins(join_list, 0);
220220

221-
// Copied from st_select_lex::fix_prepare_information():
221+
/*
222+
After permanent transformations above, prep_where created in
223+
st_select_lex::fix_prepare_information() is out-of-date, we need to
224+
refresh it.
225+
For that We must copy "conds" because it contains AND/OR items in a
226+
non-permanent memroot. And this copy must contain real items only,
227+
because the new AND/OR items will not have their argument pointers
228+
restored by rollback_item_tree_changes().
229+
@see st_select_lex::fix_prepare_information() for problems with this.
230+
@todo in WL#7082 move transformations above to before
231+
st_select_lex::fix_prepare_information(), and remove this second copy
232+
below.
233+
*/
222234
sel->prep_where=
223-
conds ? conds->real_item()->copy_andor_structure(thd) : NULL;
235+
conds ? conds->copy_andor_structure(thd, true) : NULL;
224236

225237
if (arena)
226238
thd->restore_active_arena(arena, &backup);
@@ -2759,7 +2771,7 @@ static bool record_join_nest_info(st_select_lex *select,
27592771
while ((table= li++))
27602772
{
27612773
table->prep_join_cond= table->join_cond() ?
2762-
table->join_cond()->copy_andor_structure(select->join->thd) : NULL;
2774+
table->join_cond()->copy_andor_structure(select->join->thd, true) : NULL;
27632775

27642776
if (table->nested_join == NULL)
27652777
continue;

0 commit comments

Comments
 (0)