Support equal/not_equal on a link column against a Mixed value #4597
Conversation
src/realm/query.cpp
Outdated
| ObjLink link = value.get_link(); | ||
| auto target_table = table.get_link_target(column_key); | ||
| if (target_table->get_key() == link.get_table_key()) { | ||
| key = link.get_obj_key(); |
There was a problem hiding this comment.
If the tables don't match, this code will still create matches on null links, but I don't think we want to do that. For mismatching tables, should we also throw the type mismatch error? Or maybe we can allow it but make a FALSEPREDICATE node?
There was a problem hiding this comment.
I changed to return a FalseExpression in case of mismatching tables.
src/realm/query_engine.cpp
Outdated
| else if (m_column_type == col_type_Link) { | ||
| for (auto& key : m_target_keys) { | ||
| if (key) { | ||
| // LinkColumn stores link to row N as the integer N + 1 |
There was a problem hiding this comment.
this comment has become stale and is not relevant to the code anymore, not related to your changes just noticing now
src/realm/query_engine.cpp
Outdated
| ObjKey key = m_target_keys[0]; | ||
|
|
||
| if (m_column_type == col_type_LinkList || m_condition_column_key.is_set()) { | ||
| if (!key) { |
There was a problem hiding this comment.
not sure if the query engine allows this or not, but maybe add an extra check to prevent matching null keys to an empty list
| if (!key) { | |
| if (!key && start != end) { |
There was a problem hiding this comment.
Not sure either. Anyway the early out should be removed. We should not have a match on an empty list.
| CHECK_EQUAL(tv.size(), 2); | ||
| q = source->where().not_equal(col_linklist, Mixed(target_keys[6])); | ||
| tv = q.find_all(); | ||
| CHECK_EQUAL(tv.size(), 2); |
There was a problem hiding this comment.
Could you add some checks for searching for == and != null, across linklist and link? Just making sure we haven't changed behaviour from before.
There was a problem hiding this comment.
Well - this is new functionality. We could not do this before. LinksToNode would never match on a null link. Added some more test involving null.
There was a problem hiding this comment.
Right, good point. Thanks for adding tests anyhow 😄
jedelbo
force-pushed
the
je/mixed-links-to
branch
from
676403e to
da86829
Compare
What, How & Why?
Functionality requested by SDKs
☑️ ToDos