Support equal/not_equal on a link column against a Mixed value

CHANGELOG.md

@@ -1,7 +1,7 @@
 # NEXT RELEASE
 
 ### Enhancements
-* None.
+* Support equal/not_equal on Query, where column is Link/LinkList and value is a Mixed containing a link.
 
 ### Fixed
 * <How to hit and notice issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)

src/realm/parser/driver.cpp

@@ -409,12 +409,12 @@ Query EqualityNode::visit(ParserDriver* drv)
             auto link_column = dynamic_cast<const Columns<Link>*>(left.get());
             if (link_column && link_column->link_map().get_nb_hops() == 1 &&
                 link_column->get_comparison_type() == ExpressionComparisonType::Any) {
-                // We can fall back to Query::links_to for != and == operations on links, but only
-                // for == on link lists. This is because negating query.links_to() is equivalent to
-                // to "ALL linklist != row" rather than the "ANY linklist != row" semantics we're after.
+                // We can use equal/not_equal and get a LinksToNode based query
                 if (op == CompareNode::EQUAL) {
-                    return drv->m_base_table->where().links_to(link_column->link_map().get_first_column_key(),
-                                                               val.get<ObjKey>());
+                    return drv->m_base_table->where().equal(link_column->link_map().get_first_column_key(), val);
+                }
+                else if (op == CompareNode::NOT_EQUAL) {
+                    return drv->m_base_table->where().not_equal(link_column->link_map().get_first_column_key(), val);
                 }
             }
         }

src/realm/query.cpp

@@ -406,10 +406,29 @@ std::unique_ptr<ParentNode> make_condition_node(const Table& table, ColKey colum
         case type_UUID: {
             return MakeConditionNode<UUIDNode<Cond>>::make(column_key, value);
         }
-        default: {
-            throw_type_mismatch_error();
-        }
+        case type_Link:
+        case type_LinkList:
+            if constexpr (std::is_same_v<T, Mixed> && realm::is_any_v<Cond, Equal, NotEqual>) {
+                ObjKey key;
+                if (value.is_type(type_Link)) {
+                    key = value.template get<ObjKey>();
+                }
+                else if (value.is_type(type_TypedLink)) {
+                    ObjLink link = value.get_link();
+                    auto target_table = table.get_link_target(column_key);
+                    if (target_table->get_key() != link.get_table_key()) {
+                        // This will never match
+                        return std::unique_ptr<ParentNode>{new ExpressionNode(std::make_unique<FalseExpression>())};
+                    }
+                    key = link.get_obj_key();
+                }
+                return std::unique_ptr<ParentNode>{new LinksToNode<Cond>(column_key, key)};
+            }
+            break;
+        default:
+            break;
     }
+    throw_type_mismatch_error();
 }
 
 template <class Cond>
@@ -537,7 +556,7 @@ Query& Query::between(ColKey column_key, int from, int to)
 
 Query& Query::links_to(ColKey origin_column_key, ObjKey target_key)
 {
-    add_node(std::unique_ptr<ParentNode>(new LinksToNode(origin_column_key, target_key)));
+    add_node(std::unique_ptr<ParentNode>(new LinksToNode<Equal>(origin_column_key, target_key)));
     return *this;
 }
 
@@ -549,7 +568,7 @@ Query& Query::links_to(ColKey origin_column_key, ObjLink target_link)
 
 Query& Query::links_to(ColKey origin_column, const std::vector<ObjKey>& target_keys)
 {
-    add_node(std::unique_ptr<ParentNode>(new LinksToNode(origin_column, target_keys)));
+    add_node(std::unique_ptr<ParentNode>(new LinksToNode<Equal>(origin_column, target_keys)));
     return *this;
 }
 

src/realm/query_engine.cpp

@@ -691,3 +691,70 @@ ExpressionNode::ExpressionNode(const ExpressionNode& from)
     , m_expression(from.m_expression->clone())
 {
 }
+
+template <>
+size_t LinksToNode<Equal>::find_first_local(size_t start, size_t end)
+{
+    if (m_column_type == col_type_LinkList || m_condition_column_key.is_set()) {
+
+        // LinkLists never contain null
+        if (!m_target_keys[0] && m_target_keys.size() == 1 && start != end)
+            return not_found;
+
+        BPlusTree<ObjKey> links(m_table.unchecked_ptr()->get_alloc());
+        for (size_t i = start; i < end; i++) {
+            if (ref_type ref = static_cast<const ArrayList*>(m_leaf_ptr)->get(i)) {
+                links.init_from_ref(ref);
+                for (auto& key : m_target_keys) {
+                    if (key) {
+                        if (links.find_first(key) != not_found)
+                            return i;
+                    }
+                }
+            }
+        }
+    }
+    else if (m_column_type == col_type_Link) {
+        for (auto& key : m_target_keys) {
+            auto pos = static_cast<const ArrayKey*>(m_leaf_ptr)->find_first(key, start, end);
+            if (pos != realm::npos) {
+                return pos;
+            }
+        }
+    }
+
+    return not_found;
+}
+
+template <>
+size_t LinksToNode<NotEqual>::find_first_local(size_t start, size_t end)
+{
+    // NotEqual only makes sense for a single value
+    REALM_ASSERT(m_target_keys.size() == 1);
+    ObjKey key = m_target_keys[0];
+
+    if (m_column_type == col_type_LinkList || m_condition_column_key.is_set()) {
+        BPlusTree<ObjKey> links(m_table.unchecked_ptr()->get_alloc());
+        for (size_t i = start; i < end; i++) {
+            if (ref_type ref = static_cast<const ArrayList*>(m_leaf_ptr)->get(i)) {
+                links.init_from_ref(ref);
+                auto sz = links.size();
+                for (size_t j = 0; j < sz; j++) {
+                    if (links.get(j) != key) {
+                        return i;
+                    }
+                }
+            }
+        }
+    }
+    else if (m_column_type == col_type_Link) {
+        auto leaf = static_cast<const ArrayKey*>(m_leaf_ptr);
+        for (size_t i = start; i < end; i++) {
+            if (leaf->get(i) != key) {
+                return i;
+            }
+        }
+    }
+
+    return not_found;
+}

src/realm/query_engine.hpp

@@ -2338,20 +2338,21 @@ class ExpressionNode : public ParentNode {
 };
 
 
-class LinksToNode : public ParentNode {
+class LinksToNodeBase : public ParentNode {
 public:
-    LinksToNode(ColKey origin_column_key, ObjKey target_key)
-        : LinksToNode(origin_column_key, std::vector<ObjKey>{target_key})
+    LinksToNodeBase(ColKey origin_column_key, ObjKey target_key)
+        : LinksToNodeBase(origin_column_key, std::vector<ObjKey>{target_key})
     {
     }
 
-    LinksToNode(ColKey origin_column_key, const std::vector<ObjKey>& target_keys)
+    LinksToNodeBase(ColKey origin_column_key, const std::vector<ObjKey>& target_keys)
         : m_target_keys(target_keys)
     {
         m_dT = 50.0;
         m_condition_column_key = origin_column_key;
         m_column_type = origin_column_key.get_type();
         REALM_ASSERT(m_column_type == col_type_Link || m_column_type == col_type_LinkList);
+        REALM_ASSERT(!m_target_keys.empty());
     }
 
     void cluster_changed() override
@@ -2376,48 +2377,7 @@ class LinksToNode : public ParentNode {
                util::serializer::print_value(m_target_keys[0]);
     }
 
-    virtual std::string describe_condition() const override
-    {
-        return "==";
-    }
-
-    size_t find_first_local(size_t start, size_t end) override
-    {
-        if (m_column_type == col_type_LinkList || m_condition_column_key.is_set()) {
-            BPlusTree<ObjKey> links(m_table.unchecked_ptr()->get_alloc());
-            for (size_t i = start; i < end; i++) {
-                if (ref_type ref = static_cast<const ArrayList*>(m_leaf_ptr)->get(i)) {
-                    links.init_from_ref(ref);
-                    for (auto& key : m_target_keys) {
-                        if (key) {
-                            if (links.find_first(key) != not_found)
-                                return i;
-                        }
-                    }
-                }
-            }
-        }
-        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
-                    auto pos = static_cast<const ArrayKey*>(m_leaf_ptr)->find_first(key, start, end);
-                    if (pos != realm::npos) {
-                        return pos;
-                    }
-                }
-            }
-        }
-
-        return not_found;
-    }
-
-    std::unique_ptr<ParentNode> clone() const override
-    {
-        return std::unique_ptr<ParentNode>(new LinksToNode(*this));
-    }
-
-private:
+protected:
     std::vector<ObjKey> m_target_keys;
     ColumnType m_column_type;
     using LeafPtr = std::unique_ptr<ArrayPayload, PlacementDelete>;
@@ -2429,14 +2389,32 @@ class LinksToNode : public ParentNode {
     LeafPtr m_array_ptr;
     const ArrayPayload* m_leaf_ptr = nullptr;
 
-    LinksToNode(const LinksToNode& source)
+    LinksToNodeBase(const LinksToNodeBase& source)
         : ParentNode(source)
         , m_target_keys(source.m_target_keys)
         , m_column_type(source.m_column_type)
     {
     }
 };
 
+template <class TConditionFunction>
+class LinksToNode : public LinksToNodeBase {
+public:
+    using LinksToNodeBase::LinksToNodeBase;
+
+    virtual std::string describe_condition() const override
+    {
+        return TConditionFunction::description();
+    }
+
+    std::unique_ptr<ParentNode> clone() const override
+    {
+        return std::unique_ptr<ParentNode>(new LinksToNode<TConditionFunction>(*this));
+    }
+
+    size_t find_first_local(size_t start, size_t end) override;
+};
+
 } // namespace realm
 
 #endif // REALM_QUERY_ENGINE_HPP

src/realm/query_expression.hpp

@@ -3519,20 +3519,17 @@ Query compare(const Subexpr2<Link>& left, const Obj& obj)
         REALM_ASSERT(link_map.get_target_table()->get_key() == obj.get_table()->get_key());
 #ifdef REALM_OLDQUERY_FALLBACK
         if (link_map.get_nb_hops() == 1) {
-            // We can fall back to Query::links_to for != and == operations on links, but only
-            // for == on link lists. This is because negating query.links_to() is equivalent to
-            // to "ALL linklist != row" rather than the "ANY linklist != row" semantics we're after.
-            if (link_map.m_link_types[0] == col_type_Link ||
-                (link_map.m_link_types[0] == col_type_LinkList && std::is_same_v<Operator, Equal>)) {
+            // We can fall back to Query::links_to for != and == operations on links
+            if (link_map.m_link_types[0] == col_type_Link || (link_map.m_link_types[0] == col_type_LinkList)) {
                 ConstTableRef t = column->get_base_table();
                 Query query(t);
 
-                if (std::is_same_v<Operator, NotEqual>) {
-                    // Negate the following `links_to`.
-                    query.Not();
+                if (std::is_same_v<Operator, Equal>) {
+                    return query.equal(link_map.m_link_column_keys[0], obj.get_link());
+                }
+                else {
+                    return query.not_equal(link_map.m_link_column_keys[0], obj.get_link());
                 }
-                query.links_to(link_map.m_link_column_keys[0], obj.get_key());
-                return query;
             }
         }
 #endif

src/realm/table.hpp

@@ -821,7 +821,6 @@ class Table {
     friend class Columns<StringData>;
     friend class ParentNode;
     friend struct util::serializer::SerialisationState;
-    friend class LinksToNode;
     friend class LinkMap;
     friend class LinkView;
     friend class Group;

test/test_query2.cpp

@@ -5181,15 +5181,29 @@ TEST(Query_LinksTo)
 
     source->get_object(source_keys[2]).set(col_link, target_keys[2]);
     source->get_object(source_keys[5]).set(col_link, target_keys[5]);
+    source->get_object(source_keys[8]).set(col_link, target_keys[5]);
     source->get_object(source_keys[9]).set(col_link, target_keys[9]);
 
     q = source->column<Link>(col_link) == target->get_object(target_keys[2]);
     found_key = q.find();
     CHECK_EQUAL(found_key, source_keys[2]);
 
+    q = source->where().equal(col_link, Mixed(target_keys[2]));
+    found_key = q.find();
+    CHECK_EQUAL(found_key, source_keys[2]);
+
     q = source->column<Link>(col_link) == target->get_object(target_keys[5]);
     found_key = q.find();
     CHECK_EQUAL(found_key, source_keys[5]);
+    q = source->where().equal(col_link, Mixed(target_keys[5]));
+    auto tv = q.find_all();
+    CHECK_EQUAL(tv.size(), 2);
+    q = source->where().not_equal(col_link, Mixed(target_keys[5]));
+    tv = q.find_all();
+    CHECK_EQUAL(tv.size(), 8);
+    q = source->where().equal(col_link, Mixed(ObjLink(source->get_key(), target_keys[5]))); // Wrong table
+    tv = q.find_all();
+    CHECK_EQUAL(tv.size(), 0);
 
     q = source->column<Link>(col_link) == target->get_object(target_keys[9]);
     found_key = q.find();
@@ -5200,12 +5214,18 @@ TEST(Query_LinksTo)
     CHECK_EQUAL(found_key, null_key);
 
     q = source->column<Link>(col_link).is_null();
-    auto tv = q.find_all();
-    CHECK_EQUAL(tv.size(), 7);
+    tv = q.find_all();
+    CHECK_EQUAL(tv.size(), 6);
+    q = source->where().equal(col_link, Mixed()); // Null
+    tv = q.find_all();
+    CHECK_EQUAL(tv.size(), 6);
 
     q = source->column<Link>(col_link) != null();
     found_key = q.find();
     CHECK_EQUAL(found_key, source_keys[2]);
+    q = source->where().not_equal(col_link, Mixed()); // Null
+    tv = q.find_all();
+    CHECK_EQUAL(tv.size(), 4);
 
     auto linklist = source->get_object(source_keys[1]).get_linklist_ptr(col_linklist);
     linklist->add(target_keys[6]);
@@ -5225,6 +5245,20 @@ TEST(Query_LinksTo)
     q = source->column<Link>(col_linklist) != target->get_object(target_keys[6]);
     found_key = q.find();
     CHECK_EQUAL(found_key, source_keys[2]);
+
+    q = source->where().equal(col_linklist, Mixed(target_keys[0]));
+    tv = q.find_all();
+    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);
+
+    q = source->where().equal(col_linklist, Mixed());
+    tv = q.find_all();
+    CHECK_EQUAL(tv.size(), 0); // LinkList never matches null
+    q = source->where().not_equal(col_linklist, Mixed());
+    tv = q.find_all();
+    CHECK_EQUAL(tv.size(), 3);
 }
 
 TEST(Query_Group_bug)
@@ -5890,10 +5924,22 @@ TEST(Query_Mixed)
     tv = (table->column<Mixed>(col_any) == table->column<Int>(col_int)).find_all();
     CHECK_EQUAL(tv.size(), 72);
 
-    tv = (origin->column<Mixed>(col_mixed) == Mixed(table->begin()->get_link())).find_all();
+    ObjLink link_to_first = table->begin()->get_link();
+    tv = (origin->column<Mixed>(col_mixed) == Mixed(link_to_first)).find_all();
     CHECK_EQUAL(tv.size(), 4);
-    tv = (origin->where().links_to(col_mixed, table->begin()->get_link())).find_all();
+    tv = (origin->where().links_to(col_mixed, link_to_first)).find_all();
     CHECK_EQUAL(tv.size(), 4);
+    tv = (origin->where().equal(col_link, Mixed(link_to_first))).find_all();
+    CHECK_EQUAL(tv.size(), 1);
+    tv = (origin->where().equal(col_links, Mixed(link_to_first))).find_all();
+    CHECK_EQUAL(tv.size(), 1);
+    auto q = origin->where().not_equal(col_links, Mixed(link_to_first));
+    auto d = q.get_description();
+    tv = q.find_all();
+    CHECK_EQUAL(tv.size(), 10);
+    q = origin->query(d);
+    tv = q.find_all();
+    CHECK_EQUAL(tv.size(), 10);
     tv = (origin->link(col_links).column<Mixed>(col_any) > 50).find_all();
     CHECK_EQUAL(tv.size(), 5);
     tv = (origin->link(col_link).column<Mixed>(col_any) > 50).find_all();