From b3533b5967259cf8c67080bd4ddc3ed8fa43bc65 Mon Sep 17 00:00:00 2001
From: Harpreet Sangar <happy_san@protonmail.com>
Date: Mon, 27 Mar 2023 08:25:15 +0530
Subject: [PATCH] Refactor `valid(id)`.

---
 include/filter.h     |  9 +++++++--
 src/filter.cpp       | 38 ++++++++++++++++++++++++++++----------
 test/filter_test.cpp | 16 +++++++++-------
 3 files changed, 44 insertions(+), 19 deletions(-)

diff --git a/include/filter.h b/include/filter.h
index c5a20f11..9bb78deb 100644
--- a/include/filter.h
+++ b/include/filter.h
@@ -77,8 +77,13 @@ public:
     /// Returns true when doc and reference hold valid values. Used in conjunction with next() and skip_to(id).
     [[nodiscard]] bool valid();
 
-    /// Returns true when id is a match to the filter. Handles moving the individual iterators internally.
-    [[nodiscard]] bool valid(uint32_t id);
+    /// Returns a tri-state:
+    ///     0: id is not valid
+    ///     1: id is valid
+    ///    -1: end of iterator
+    ///
+    ///  Handles moving the individual iterators internally.
+    [[nodiscard]] int valid(uint32_t id);
 
     /// Advances the iterator to get the next value of doc and reference. The iterator may become invalid during this
     /// operation.
diff --git a/src/filter.cpp b/src/filter.cpp
index 16e52c49..44393d8d 100644
--- a/src/filter.cpp
+++ b/src/filter.cpp
@@ -437,20 +437,38 @@ void filter_result_iterator_t::skip_to(uint32_t id) {
     }
 }
 
-bool filter_result_iterator_t::valid(uint32_t id) {
+int filter_result_iterator_t::valid(uint32_t id) {
     if (!is_valid) {
-        return false;
+        return -1;
     }
 
     if (filter_node->isOperator) {
+        auto left_valid = left_it->valid(id), right_valid = right_it->valid(id);
+
         if (filter_node->filter_operator == AND) {
-            auto and_is_valid = left_it->valid(id) && right_it->valid(id);
             is_valid = left_it->is_valid && right_it->is_valid;
-            return and_is_valid;
+
+            if (left_valid  < 1 || right_valid < 1) {
+                if (left_valid == -1 || right_valid == -1) {
+                    return -1;
+                }
+
+                return 0;
+            }
+
+            return 1;
         } else {
-            auto or_is_valid = left_it->valid(id) || right_it->valid(id);
             is_valid = left_it->is_valid || right_it->is_valid;
-            return or_is_valid;
+
+            if (left_valid < 1 && right_valid < 1) {
+                if (left_valid == -1 && right_valid == -1) {
+                    return -1;
+                }
+
+                return 0;
+            }
+
+            return 1;
         }
     }
 
@@ -458,21 +476,21 @@ bool filter_result_iterator_t::valid(uint32_t id) {
         // Even when iterator becomes invalid, we keep it marked as valid since we are evaluating not equals.
         if (!valid()) {
             is_valid = true;
-            return is_valid;
+            return 1;
         }
 
         skip_to(id);
 
         if (!is_valid) {
             is_valid = true;
-            return is_valid;
+            return 1;
         }
 
-        return seq_id != id;
+        return seq_id != id ? 1 : 0;
     }
 
     skip_to(id);
-    return is_valid && seq_id == id;
+    return is_valid ? (seq_id == id ? 1 : 0) : -1;
 }
 
 Option<bool> filter_result_iterator_t::init_status() {
diff --git a/test/filter_test.cpp b/test/filter_test.cpp
index 935efff8..e607e03b 100644
--- a/test/filter_test.cpp
+++ b/test/filter_test.cpp
@@ -137,7 +137,7 @@ TEST_F(FilterTest, FilterTreeIterator) {
     auto iter_exact_match_multi_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root);
     ASSERT_TRUE(iter_exact_match_multi_test.init_status().ok());
 
-    std::vector<uint32_t> expected = {0, 2, 3, 4};
+    std::vector<int> expected = {0, 2, 3, 4};
     for (auto const& i : expected) {
         ASSERT_TRUE(iter_exact_match_multi_test.valid());
         ASSERT_EQ(i, iter_exact_match_multi_test.seq_id);
@@ -254,9 +254,10 @@ TEST_F(FilterTest, FilterTreeIterator) {
     auto iter_validate_ids_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root);
     ASSERT_TRUE(iter_validate_ids_test.init_status().ok());
 
-    expected = {0, 2, 4, 5};
-    for (auto const& i : expected) {
-        ASSERT_TRUE(iter_validate_ids_test.valid(i));
+    std::vector<int> validate_ids = {0, 1, 2, 3, 4, 5, 6};
+    expected = {1, 0, 1, 0, 1, 1, -1};
+    for (uint32_t i = 0; i < validate_ids.size(); i++) {
+        ASSERT_EQ(expected[i], iter_validate_ids_test.valid(validate_ids[i]));
     }
 
     delete filter_tree_root;
@@ -269,9 +270,10 @@ TEST_F(FilterTest, FilterTreeIterator) {
                                                                              filter_tree_root);
     ASSERT_TRUE(iter_validate_ids_not_equals_filter_test.init_status().ok());
 
-    expected = {1, 3, 5};
-    for (auto const& i : expected) {
-        ASSERT_TRUE(iter_validate_ids_not_equals_filter_test.valid(i));
+    validate_ids = {0, 1, 2, 3, 4, 5, 6, 7, 100};
+    expected = {0, 1, 0, 1, 0, 1, 1, 1, 1};
+    for (uint32_t i = 0; i < validate_ids.size(); i++) {
+        ASSERT_EQ(expected[i], iter_validate_ids_not_equals_filter_test.valid(validate_ids[i]));
     }
 
     delete filter_tree_root;