← Back to team overview

maria-developers team mailing list archive

Re: Rev 2764: * Better self-recursion protection in Item_subselect::fix_fields. in file:///home/psergey/dev/maria-5.3-subqueries-r7/

 

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@xxxxxxxxxxxx-20100221033618-83dgm2h9ingzmhcc
> parent: psergey@xxxxxxxxxxxx-20100220082329-9esvom4n6mpgeqvk
> committer: Sergey Petrunya <psergey@xxxxxxxxxxxx>
> 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



References