RCORE-2098: Fix issue with OR of two IN queries (#7628)

* Fix issue with OR of two IN queries
* same fix for other node types, fix combination for empty string

---------

Co-authored-by: James Stone <[email protected]>

Author: jedelbo

CHANGELOG.md

@@ -6,7 +6,8 @@
 
 ### Fixed
 * <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
-* None.
+* Fix assertion failure or wrong results when evaluating a RQL query with multiple IN conditions on the same property. Applies to non-indexed int/string/ObjectId/UUID properties, or if they were indexed and had > 100 conditions. ((RCORE-2098) [PR #7628](https://github.com/realm/realm-core/pull/7628) since v14.6.0).
+* Fixed a bug when running a IN query (or a query of the pattern `x == 1 OR x == 2 OR x == 3`) when evaluating on a string property with an empty string in the search condition. Matches with an empty string would have been evaluated as if searching for a null string instead. ([PR #7628](https://github.com/realm/realm-core/pull/7628) since v10.0.0-beta.9)
 
 ### Breaking changes
 * None.

src/realm/query_engine.cpp

@@ -417,17 +417,31 @@ bool StringNode<Equal>::do_consume_condition(ParentNode& node)
 
     auto& other = static_cast<StringNode<Equal>&>(node);
     REALM_ASSERT(m_condition_column_key == other.m_condition_column_key);
-    REALM_ASSERT(other.m_needles.empty());
+
     if (m_needles.empty()) {
         m_needles.insert(m_string_value);
     }
-    if (auto& str = other.m_value) {
-        m_needle_storage.push_back(std::make_unique<char[]>(str->size()));
-        std::copy(str->data(), str->data() + str->size(), m_needle_storage.back().get());
-        m_needles.insert(StringData(m_needle_storage.back().get(), str->size()));
+    auto add_string = [&](const StringData& str) {
+        if (m_needles.count(str) == 0) {
+            if (str.size()) {
+                m_needle_storage.push_back(std::make_unique<char[]>(str.size()));
+                std::copy(str.data(), str.data() + str.size(), m_needle_storage.back().get());
+                m_needles.insert(StringData(m_needle_storage.back().get(), str.size()));
+            }
+            else {
+                // this code path is different because we need to
+                // distinguish null from the empty string
+                m_needles.insert(str);
+            }
+        }
+    };
+    if (!other.m_needles.empty()) {
+        for (const auto& str : other.m_needles) {
+            add_string(str);
+        }
     }
     else {
-        m_needles.emplace();
+        add_string(other.m_string_value);
     }
     return true;
 }

src/realm/query_engine.hpp

@@ -532,11 +532,17 @@ class IntegerNode<LeafType, Equal> : public IntegerNodeBase<LeafType> {
     {
         auto& other = static_cast<ThisType&>(node);
         REALM_ASSERT(this->m_condition_column_key == other.m_condition_column_key);
-        REALM_ASSERT(other.m_needles.empty());
         if (m_needles.empty()) {
             m_needles.insert(this->m_value);
         }
-        m_needles.insert(other.m_value);
+        if (other.m_needles.empty()) {
+            m_needles.insert(other.m_value);
+        }
+        else {
+            for (const auto& val : other.m_needles) {
+                m_needles.insert(val);
+            }
+        }
         return true;
     }
 
@@ -1294,11 +1300,17 @@ class FixedBytesNode<Equal, ObjectType, ArrayType> : public FixedBytesNodeBase<O
     {
         auto& other = static_cast<ThisType&>(node);
         REALM_ASSERT(this->m_condition_column_key == other.m_condition_column_key);
-        REALM_ASSERT(other.m_needles.empty());
         if (m_needles.empty()) {
             m_needles.insert(this->m_value_is_null ? std::nullopt : std::make_optional(this->m_value));
         }
-        m_needles.insert(other.m_value_is_null ? std::nullopt : std::make_optional(other.m_value));
+        if (other.m_needles.empty()) {
+            m_needles.insert(other.m_value_is_null ? std::nullopt : std::make_optional(other.m_value));
+        }
+        else {
+            for (const auto& val : other.m_needles) {
+                m_needles.insert(val);
+            }
+        }
         return true;
     }
 

test/test_parser.cpp

@@ -4093,6 +4093,25 @@ TEST(Parser_OperatorIN)
                    CHECK_EQUAL(e.what(), "The keypath following 'IN' must contain a list. Found 'fav_item.price'"));
 }
 
+TEST(Parser_OrOfIn)
+{
+    Group g;
+
+    TableRef persons = g.add_table("class_Person");
+    constexpr bool nullable = true;
+    auto col_name = persons->add_column(type_String, "name", nullable);
+    persons->create_object().set(col_name, "Ani");
+    persons->create_object().set(col_name, "Teddy");
+    persons->create_object().set(col_name, "Poly");
+    persons->create_object().set(col_name, ""); // empty string
+    persons->create_object();                   // null value
+
+    verify_query(test_context, persons, "name IN {'Ani', 'Teddy'} OR name IN {'Poly', 'Teddy'}", 3);
+    verify_query(test_context, persons, "name IN {'Ani', 'Teddy'} OR name IN {'Poly', 'Teddy'} OR name IN {null}", 4);
+    verify_query(test_context, persons,
+                 "name IN {'Ani', 'Teddy'} OR name IN {'Poly', 'Teddy'} OR name IN {null} OR name IN {''}", 5);
+}
+
 TEST(Parser_KeyPathSubstitution)
 {
     Group g;

test/test_query2.cpp

@@ -6007,6 +6007,14 @@ TEST_TYPES(Query_ManyIn, Prop<Int>, Prop<String>, Prop<Float>, Prop<Double>, Pro
     bool order = first == mixed_vals[1];
     CHECK_EQUAL(first, order ? mixed_vals[1] : mixed_vals[2]);
     CHECK_EQUAL(second, order ? mixed_vals[2] : mixed_vals[1]);
+    size_t count_of_two_ins = t->where()
+                                  .in(col, mixed_vals.data() + 1, mixed_vals.data() + 2)
+                                  .Or()
+                                  .in(col, mixed_vals.data() + 2, mixed_vals.data() + 3)
+                                  .count();
+    size_t count_of_one_in = t->where().in(col, mixed_vals.data() + 1, mixed_vals.data() + 3).count();
+    CHECK_EQUAL(count_of_one_in, 2);
+    CHECK_EQUAL(count_of_two_ins, 2);
 }
 
 TEST(Query_ManyIntConditionsAgg)