Skip to content

Commit

Permalink
Fix incorrect query results on a LnkLst that does not match table order
Browse files Browse the repository at this point in the history
The optimization added in #3432 to use the search index when performing
equality queries over a link assumed that Queries would be evaluated in
ascending table order, which is not always the case when the query is being
performed on a link list.

Fixes realm/realm-swift#6540.
  • Loading branch information
tgoyne committed May 29, 2020
1 parent 4950874 commit 62813a1
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

### Fixed
* Fixed opening Realms on Apple devices where the file resided on a filesystem that does not support preallocation, such as ExFAT. ([cocoa-6508](https://github.com/realm/realm-cocoa/issues/6508)).
* Fixed incorrect results when querying on a LnkLst where the target property over a link has an index and the LnkLst has a different order from the target table. ([Cocoa #6540](https://github.com/realm/realm-cocoa/issues/6540), since 5.23.6.

### Breaking changes
* None.
Expand Down
8 changes: 8 additions & 0 deletions src/realm/keys.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,18 @@ struct ObjKey {
{
return value < rhs.value;
}
bool operator<=(const ObjKey& rhs) const noexcept
{
return value <= rhs.value;
}
bool operator>(const ObjKey& rhs) const noexcept
{
return value > rhs.value;
}
bool operator>=(const ObjKey& rhs) const noexcept
{
return value >= rhs.value;
}
explicit operator bool() const noexcept
{
return value != -1;
Expand Down
34 changes: 24 additions & 10 deletions src/realm/query_expression.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3901,27 +3901,41 @@ class Compare : public Expression {
size_t find_first(size_t start, size_t end) const override
{
if (m_has_matches) {
if (m_index_get < m_index_end && start < end) {
ObjKey first_key = m_cluster->get_real_key(start);
auto actual_key = m_matches[m_index_get];
if (m_index_end == 0 || start >= end)
return not_found;

ObjKey first_key = m_cluster->get_real_key(start);
ObjKey actual_key;

// Sequential lookup optimization: when the query isn't constrained
// to a LnkLst we'll get find_first() requests in ascending order,
// so we can do a simple linear scan.
if (m_index_get < m_index_end && m_matches[m_index_get] <= first_key) {
actual_key = m_matches[m_index_get];
// skip through keys which are in "earlier" leafs than the one selected by start..end:
while (first_key > actual_key) {
m_index_get++;
if (m_index_get == m_index_end)
return not_found;
actual_key = m_matches[m_index_get];
}
// if actual key is bigger than last key, it is not in this leaf
ObjKey last_key = m_cluster->get_real_key(end - 1);
if (actual_key > last_key)
}
// Otherwise if we get requests out of order we have to do a more
// expensive binary search
else {
auto it = std::lower_bound(m_matches.begin(), m_matches.end(), first_key);
if (it == m_matches.end())
return not_found;
actual_key = *it;
}

// key is known to be in this leaf, so find key whithin leaf keys
return m_cluster->lower_bound_key(ObjKey(actual_key.value - m_cluster->get_offset()));
// if actual key is bigger than last key, it is not in this leaf
ObjKey last_key = start + 1 == end ? first_key : m_cluster->get_real_key(end - 1);
if (actual_key > last_key)
return not_found;

}
return not_found;
// key is known to be in this leaf, so find key whithin leaf keys
return m_cluster->lower_bound_key(ObjKey(actual_key.value - m_cluster->get_offset()));
}

size_t match;
Expand Down
32 changes: 32 additions & 0 deletions test/test_link_query_view.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1097,6 +1097,38 @@ TEST(LinkList_QueryOnLinkListWithDuplicates)
CHECK_EQUAL(target_keys[2], some_rows.get_key(3));
}

TEST(LinkList_QueryOnLinkListWithNonTableOrderThroughLinkWithIndex)
{
Group group;

TableRef target = group.add_table("target");
auto int_col = target->add_column(type_Int, "col1");
target->add_search_index(int_col);
TableRef middle = group.add_table("middle");
auto link_col = middle->add_column_link(type_Link, "link", *target);
TableRef origin = group.add_table("origin");
auto list_col = origin->add_column_link(type_LinkList, "linklist", *middle);

auto target_obj = target->create_object().set_all(1);

std::vector<ObjKey> middle_keys;
middle->create_objects(3, middle_keys);
for (int i = 0; i < 3; ++i)
middle->get_object(middle_keys[i]).set_all(target_obj.get_key());

Obj origin_obj = origin->create_object();
LnkLst list = origin_obj.get_linklist(list_col);

list.add(middle_keys[1]);
list.add(middle_keys[0]);
list.add(middle_keys[2]);

auto unfiltered = middle->where(list).find_all();
CHECK_EQUAL(3, unfiltered.size());
auto filtered = middle->where(list).and_query(middle->link(link_col).column<Int>(int_col) == 1).find_all();
CHECK_EQUAL(3, filtered.size());
}

TEST(LinkList_QueryOnIndexedPropertyOfLinkListSingleMatch)
{
Group group;
Expand Down

0 comments on commit 62813a1

Please sign in to comment.