Re: [Maria-developers] [Commits] f87dddaaa95: MDEV-7865: Server crashes in Item_equal_iterator<List_iterator_fast, Item>::get_curr_field on query
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
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
participants (1)
-
Sergey Petrunia