[Maria-developers] Rev 2764: * Better self-recursion protection in Item_subselect::fix_fields. in file:///home/psergey/dev/maria-5.3-subqueries-r7/
At file:///home/psergey/dev/maria-5.3-subqueries-r7/ ------------------------------------------------------------ revno: 2764 revision-id: psergey@askmonty.org-20100221033618-83dgm2h9ingzmhcc parent: psergey@askmonty.org-20100220082329-9esvom4n6mpgeqvk committer: Sergey Petrunya <psergey@askmonty.org> branch nick: maria-5.3-subqueries-r7 timestamp: Sun 2010-02-21 05:36:18 +0200 message: * Better self-recursion protection in Item_subselect::fix_fields. Don't go into branch that calls upper_refs.empty() more than once per PREPARE or EXECUTE * Avoid crashing when processing references to outside from subquery's HAVING (will explain in more details in email) === modified file 'sql/item.h' --- a/sql/item.h 2010-02-17 10:05:27 +0000 +++ b/sql/item.h 2010-02-21 03:36:18 +0000 @@ -2378,7 +2378,12 @@ return ref ? (*ref)->real_item() : this; } bool walk(Item_processor processor, bool walk_subquery, uchar *arg) - { return (*ref)->walk(processor, walk_subquery, arg); } + { + if (ref && *ref) + return (*ref)->walk(processor, walk_subquery, arg); + else + return FALSE; + } bool enumerate_field_refs_processor(uchar *arg) { return (*ref)->enumerate_field_refs_processor(arg); } virtual void print(String *str, enum_query_type query_type); === modified file 'sql/item_subselect.cc' --- a/sql/item_subselect.cc 2010-02-20 08:23:29 +0000 +++ b/sql/item_subselect.cc 2010-02-21 03:36:18 +0000 @@ -186,7 +186,6 @@ changed= 1; inside_first_fix_fields= FALSE; - done_first_fix_fields= FALSE; if (!res) { @@ -218,12 +217,14 @@ if (!(*ref)->fixed) ret= (*ref)->fix_fields(thd, ref); thd->where= save_where; + done_first_fix_fields= FALSE; return ret; } // Is it one field subselect? if (engine->cols() > max_columns) { my_error(ER_OPERAND_COLUMNS, MYF(0), 1); + done_first_fix_fields= FALSE; return TRUE; } fix_length_and_dec(); @@ -240,6 +241,7 @@ fixed= 1; err: + done_first_fix_fields= FALSE; thd->where= save_where; return res; } @@ -282,6 +284,7 @@ return FALSE; } + /* Adjust attributes after our parent select has been merged into grandparent @@ -310,6 +313,7 @@ parent_select= new_parent; } + class Field_fixer: public Field_enumerator { public:
On Sun, Feb 21, 2010 at 06:36:21AM +0300, Sergey Petrunya wrote:
At file:///home/psergey/dev/maria-5.3-subqueries-r7/
------------------------------------------------------------ revno: 2764 revision-id: psergey@askmonty.org-20100221033618-83dgm2h9ingzmhcc parent: psergey@askmonty.org-20100220082329-9esvom4n6mpgeqvk committer: Sergey Petrunya <psergey@askmonty.org> branch nick: maria-5.3-subqueries-r7 timestamp: Sun 2010-02-21 05:36:18 +0200 message: * Better self-recursion protection in Item_subselect::fix_fields. Don't go into branch that calls upper_refs.empty() more than once per PREPARE or EXECUTE * Avoid crashing when processing references to outside from subquery's HAVING (will explain in more details in email)
Here it goes: The problem is that there are cases where we get mark_as_dependent() call with the item being an Item_ref that has (and will continue to have) fixed==0, ref==0. Consider this: select t1.col1 from t1 where t1.col2 in (select t2.col2 from t2 group by t2.col1, t2.col2 having col_t1 <= 10); prepare s from 'select t1.col1 from t1 where t1.col2 in (select t2.col2 from t2 group by t2.col1, t2.col2 having col_t1 <= 10)'; execute s; The above has a problem on EXECUTE call: we get two mark_as_dependent() calls like this: Breakpoint 13, Item_subselect::mark_as_dependent (this=0xab3ea28, thd=0xa2724e8, select=0xab3d4bc, item=0xab3e850) at item_subselect.cc:273 (gdb) p item $254 = (Item_ref *) 0xab3e850 (gdb) p item->fixed $255 = 0 '\0' (gdb) p item->ref $256 = (Item **) 0x0 (gdb) c Continuing. Breakpoint 13, Item_subselect::mark_as_dependent (this=0xab3ea28, thd=0xa2724e8, select=0xab3d4bc, item=0xaaffe70) at item_subselect.cc:273 (gdb) p item $257 = (Item_field *) 0xaaffe70 (gdb) p item->fixed $258 = 1 '\001' (gdb) p item->field->field_name $259 = 0xab1540b "col_t1" (gdb) p item->field->table_name $260 = (const char **) 0xab06db4 (gdb) p *item->field->table_name $261 = 0xaa6f630 "t1" The first call is the problem. the passed item has fixed=0, ref=0, i.e. it is invalid and doesn't refer to anything. It is not possible to fix the problem in mark_as_dependent() or several stack frames further as there we'll have item==mark_item=resolved_item. Interestingly, we get a second mark_as_dependent() call for the same outside reference and that is what saves us, because the second call will brings item==Item_field(t1.col_t1) which is what we need. This patch is a cop-out solution: we still put the defective Item_ref item into Item_subselect::upper_refs and only make sure that calls to defective_item->walk() will have no effect: === modified file 'sql/item.h' --- sql/item.h 2010-02-17 10:05:27 +0000 +++ sql/item.h 2010-02-20 19:49:19 +0000 @@ -2378,7 +2378,12 @@ public: return ref ? (*ref)->real_item() : this; } bool walk(Item_processor processor, bool walk_subquery, uchar *arg) - { return (*ref)->walk(processor, walk_subquery, arg); } + { + if (ref && *ref) + return (*ref)->walk(processor, walk_subquery, arg); + else + return FALSE; + } BR Sergey -- Sergey Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog
participants (1)
-
Sergey Petrunya