Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix query on size of a single link over links #6918

Merged
merged 2 commits into from
Aug 24, 2023
Merged

Fix query on size of a single link over links #6918

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a6a84f1
Fix query on size of a single link over links
jedelbo Aug 23, 2023
912c73a
Rephrase release note
jedelbo Aug 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

### Fixed
* <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
* None.
* Crash when querying the size of a Object property in a linked/linked to object ([#6915](https://github.com/realm/realm-core/issues/6915), since v13.17.2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What linked/linked stands for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is not entirely readable. The query should be on the size of an Object property on an object that is linked from the base object. The link can either be forward or backward. I will try to rephrase


### Breaking changes
* None.
Expand Down
7 changes: 6 additions & 1 deletion src/realm/query_expression.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,11 +519,16 @@ void LinkCount::evaluate(size_t index, ValueBase& destination)
const Obj obj = m_link_map.get_target_table()->get_object(links[i]);
auto val = obj._get<int64_t>(m_column_key.get_index());
size_t s;
if (val & 1) {
if (m_column_key.get_type() == col_type_Link && !m_column_key.is_collection()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I read this correctly? You are trying to avoid to deal with collections of links? Right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. We must handle single link columns separately. That was the problem.

// It is a single link column
s = (val == 0) ? 0 : 1;
}
else if (val & 1) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can you be sure that, at this point, we are dealing with a simple backlink column that points to one value? I guess what I am trying to say, is that... the logic behind val == 0 vs val == 1 is a little bit obscure, does val represents a simple boolean flag?....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a bit obscure. The only case where we can have a tagged value here is when we are dealing with a backlink column with a single link. All other cases we have a ref to bplustree of links.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK maybe we can use a comment here. Describing this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is actually a comment

// It is a backlink column with just one value
s = 1;
}
else {
// This is some kind of collection or backlink column
s = _impl::get_collection_size_from_ref(to_ref(val), alloc);
}
destination.set(i, int64_t(s));
Expand Down
13 changes: 12 additions & 1 deletion test/test_query.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5640,6 +5640,7 @@ TEST(Query_NestedLinkCount)
table->add_column_list(*table, "children");
table->add_column_dictionary(*table, "dictionary");
table->add_column_list(*table, "list");
table->add_column(*table, "Object");

Obj o1 = table->create_object_with_primary_key(1);
Obj o2 = table->create_object_with_primary_key(2);
Expand All @@ -5650,6 +5651,10 @@ TEST(Query_NestedLinkCount)
dict.insert("key", o3);
o3.get_linklist("children").add(o2.get_key());

o1.set("Object", o1.get_key());
o2.set("Object", o2.get_key());
o3.set("Object", o2.get_key());

auto q = table->query("children.list.@size == 0");
CHECK_EQUAL(q.count(), 2);

Expand All @@ -5676,7 +5681,13 @@ TEST(Query_NestedLinkCount)
dict.insert("key4", o3);
q = table->query("@links.TestClass.children.dictionary.@size == 5");
CHECK_EQUAL(q.count(), 1); // Only o2
}

q = table->query("@links.TestClass.Object.@size > 0");
CHECK_EQUAL(q.count(), 2);
q = table->query("@links.TestClass.Object.Object.@size > 0");
CHECK_EQUAL(q.count(), 2);
q = table->query("Object.@links.TestClass.Object.@size > 0");
CHECK_EQUAL(q.count(), 3);
}

#endif // TEST_QUERY