maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

Discussed this on the optimizer call, please find the notes below.

On Wed, Nov 08, 2017 at 01:09:37PM +0300, Sergey Petrunia wrote:
> On Sat, Oct 28, 2017 at 12:47:03AM +0530, Varun wrote:
> > revision-id: f87dddaaa95f9e7fc597c8839b539cc6f816c750 (mariadb-5.5.56-91-gf87dddaaa95)
> > parent(s): fb5fe497e526f0677a7e2f86f34237e7efd4b806
> > author: Varun Gupta
> > committer: Varun Gupta
> > timestamp: 2017-10-28 00:46:43 +0530
> > message:
> > 
> > MDEV-7865: Server crashes in Item_equal_iterator<List_iterator_fast, Item>::get_curr_field on query
> >            with impossible condition and OR/AND expressions
> > 
> > When multiple-equalites are merged into one in the function Item_equal::merge_with_check, then the fields that are merged from one
> > equality to the other still point to the previous mulitple_equality while the should be pointing to the new equality
> > after the merge is done
> ..
> > diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
> > index ebe088e5092..8c79359ce88 100644
> > --- a/sql/item_cmpfunc.cc
> > +++ b/sql/item_cmpfunc.cc
> > @@ -5832,7 +5832,18 @@ bool Item_equal::merge_with_check(Item_equal *item, bool save_merged)
> >    if (intersected)
> >    {
> >      if (!save_merged)
> > +    {
> > +      Item *item_it;
> > +      fi.rewind();
> > +      while ((item_it= fi++))
> > +      {
> Wrong indentation
> 
> > +          if (!contains(fi.get_curr_field()))
> > +          {
> > +            item_it->set_item_equal(this);
> 
> I am not sure if this can actually make a difference, but is there any reason
> we don't just call set_item_equal unconditionally here?
> 
> My concern is that f.get_curr_field() checks for Field* objects, while 
> set_item_equal() call applies to Items (either Item_field or 
> Item_direct_view_ref)
> 
> What if there are two different Item objects that refer to the same field?
> (this looks like it should not happen but I am not 100% certain about this).
>
There cannot be two Item_field objects that refer to the same field.

According to Igor, an Item_equal object cannot have both $item= Item_field
and an Item_*ref wich refers to the $item.

> > +          }
> > +      }
> >        merge(item);
> > +    }
> >      else
> >      {
> >        Item *c= item->get_const();
> > @@ -5843,9 +5854,12 @@ bool Item_equal::merge_with_check(Item_equal *item, bool save_merged)
> >          Item *item;
> >          fi.rewind();
> >          while ((item= fi++))
> > -	{
> > +        {
> >            if (!contains(fi.get_curr_field()))
> > +          {
> >              add(item);
> > +            item->set_item_equal(this);
> > +          }
> 
> here, we have save_merged=true, that is, the old Item_equal will still be used.
> 
> Is this ok that we call set_item_equal($new_equality) for its members?
> 
> If an object is a participant in two Item_equal, which one should it point to?
> the "most specific" one?  I'll try to ask Igor about this today.

Asked, there is a concern about how item->item_equal is used.

In the worst case, we will need to do a deep copy, that is, clone the
Item_field (or Item_direct_view_ref) objects that participate in the
multiple-equalities.

(after writing this down, it struck me that if we create a new list of
Item_field objects for addition into the Item_equal, will we need to adjust
some pointers somewhere to point to the new objects? This seems to be more 
complex than the previous paragraph says)


BR
 Sergei
-- 
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog