maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

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).

> +          }
> +      }
>        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.

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