diff --git a/include/filter.h b/include/filter.h index 643c5006..c5a20f11 100644 --- a/include/filter.h +++ b/include/filter.h @@ -42,22 +42,20 @@ private: void doc_matching_string_filter(); public: - uint32_t doc; + uint32_t seq_id = 0; // Collection name -> references std::map reference; - Option status; + Option status = Option(true); explicit filter_result_iterator_t(const std::string& collection_name, - const Index* index, filter_node_t* filter_node, - Option& status) : + const Index* index, filter_node_t* filter_node) : collection_name(collection_name), index(index), - filter_node(filter_node), - status(status) { + filter_node(filter_node) { // Generate the iterator tree and then initialize each node. if (filter_node->isOperator) { - left_it = new filter_result_iterator_t(collection_name, index, filter_node->left, status); - right_it = new filter_result_iterator_t(collection_name, index, filter_node->right, status); + left_it = new filter_result_iterator_t(collection_name, index, filter_node->left); + right_it = new filter_result_iterator_t(collection_name, index, filter_node->right); } init(); @@ -73,6 +71,9 @@ public: delete right_it; } + /// Returns the status of the initialization of iterator tree. + Option init_status(); + /// Returns true when doc and reference hold valid values. Used in conjunction with next() and skip_to(id). [[nodiscard]] bool valid(); diff --git a/src/filter.cpp b/src/filter.cpp index 2c289761..16e52c49 100644 --- a/src/filter.cpp +++ b/src/filter.cpp @@ -5,7 +5,7 @@ void filter_result_iterator_t::and_filter_iterators() { while (left_it->is_valid && right_it->is_valid) { - while (left_it->doc < right_it->doc) { + while (left_it->seq_id < right_it->seq_id) { left_it->next(); if (!left_it->is_valid) { is_valid = false; @@ -13,7 +13,7 @@ void filter_result_iterator_t::and_filter_iterators() { } } - while (left_it->doc > right_it->doc) { + while (left_it->seq_id > right_it->seq_id) { right_it->next(); if (!right_it->is_valid) { is_valid = false; @@ -21,8 +21,8 @@ void filter_result_iterator_t::and_filter_iterators() { } } - if (left_it->doc == right_it->doc) { - doc = left_it->doc; + if (left_it->seq_id == right_it->seq_id) { + seq_id = left_it->seq_id; reference.clear(); for (const auto& item: left_it->reference) { @@ -41,8 +41,8 @@ void filter_result_iterator_t::and_filter_iterators() { void filter_result_iterator_t::or_filter_iterators() { if (left_it->is_valid && right_it->is_valid) { - if (left_it->doc < right_it->doc) { - doc = left_it->doc; + if (left_it->seq_id < right_it->seq_id) { + seq_id = left_it->seq_id; reference.clear(); for (const auto& item: left_it->reference) { @@ -52,8 +52,8 @@ void filter_result_iterator_t::or_filter_iterators() { return; } - if (left_it->doc > right_it->doc) { - doc = right_it->doc; + if (left_it->seq_id > right_it->seq_id) { + seq_id = right_it->seq_id; reference.clear(); for (const auto& item: right_it->reference) { @@ -63,7 +63,7 @@ void filter_result_iterator_t::or_filter_iterators() { return; } - doc = left_it->doc; + seq_id = left_it->seq_id; reference.clear(); for (const auto& item: left_it->reference) { @@ -77,7 +77,7 @@ void filter_result_iterator_t::or_filter_iterators() { } if (left_it->is_valid) { - doc = left_it->doc; + seq_id = left_it->seq_id; reference.clear(); for (const auto& item: left_it->reference) { @@ -88,7 +88,7 @@ void filter_result_iterator_t::or_filter_iterators() { } if (right_it->is_valid) { - doc = right_it->doc; + seq_id = right_it->seq_id; reference.clear(); for (const auto& item: right_it->reference) { @@ -105,7 +105,7 @@ void filter_result_iterator_t::doc_matching_string_filter() { // If none of the filter value iterators are valid, mark this node as invalid. bool one_is_valid = false; - // Since we do OR between filter values, the lowest doc id from all is selected. + // Since we do OR between filter values, the lowest seq_id id from all is selected. uint32_t lowest_id = UINT32_MAX; for (auto& filter_value_tokens : posting_list_iterators) { @@ -121,7 +121,7 @@ void filter_result_iterator_t::doc_matching_string_filter() { } if (one_is_valid) { - doc = lowest_id; + seq_id = lowest_id; } is_valid = one_is_valid; @@ -139,10 +139,10 @@ void filter_result_iterator_t::next() { right_it->next(); and_filter_iterators(); } else { - if (left_it->doc == doc && right_it->doc == doc) { + if (left_it->seq_id == seq_id && right_it->seq_id == seq_id) { left_it->next(); right_it->next(); - } else if (left_it->doc == doc) { + } else if (left_it->seq_id == seq_id) { left_it->next(); } else { right_it->next(); @@ -163,7 +163,7 @@ void filter_result_iterator_t::next() { return; } - doc = filter_result.docs[result_index]; + seq_id = filter_result.docs[result_index]; reference.clear(); for (auto const& item: filter_result.reference_filter_results) { reference[item.first] = item.second[result_index]; @@ -178,7 +178,7 @@ void filter_result_iterator_t::next() { return; } - doc = filter_result.docs[result_index]; + seq_id = filter_result.docs[result_index]; return; } @@ -194,7 +194,7 @@ void filter_result_iterator_t::next() { for (uint32_t i = 0; i < posting_list_iterators.size(); i++) { auto& filter_value_tokens = posting_list_iterators[i]; - if (filter_value_tokens[0].valid() && filter_value_tokens[0].id() == doc) { + if (filter_value_tokens[0].valid() && filter_value_tokens[0].id() == seq_id) { for (auto& iter: filter_value_tokens) { iter.next(); } @@ -391,7 +391,7 @@ void filter_result_iterator_t::skip_to(uint32_t id) { return; } - doc = filter_result.docs[result_index]; + seq_id = filter_result.docs[result_index]; reference.clear(); for (auto const& item: filter_result.reference_filter_results) { reference[item.first] = item.second[result_index]; @@ -408,7 +408,7 @@ void filter_result_iterator_t::skip_to(uint32_t id) { return; } - doc = filter_result.docs[result_index]; + seq_id = filter_result.docs[result_index]; return; } @@ -468,9 +468,19 @@ bool filter_result_iterator_t::valid(uint32_t id) { return is_valid; } - return doc != id; + return seq_id != id; } skip_to(id); - return is_valid && doc == id; + return is_valid && seq_id == id; +} + +Option filter_result_iterator_t::init_status() { + if (filter_node->isOperator) { + auto left_status = left_it->init_status(); + + return !left_status.ok() ? left_status : right_it->init_status(); + } + + return status; } diff --git a/test/filter_test.cpp b/test/filter_test.cpp index 3c4b94a5..935efff8 100644 --- a/test/filter_test.cpp +++ b/test/filter_test.cpp @@ -64,11 +64,10 @@ TEST_F(FilterTest, FilterTreeIterator) { filter_tree_root); ASSERT_TRUE(filter_op.ok()); - Option iter_op(true); - auto iter_no_match_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root, iter_op); + auto iter_no_match_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root); + ASSERT_TRUE(iter_no_match_test.init_status().ok()); ASSERT_FALSE(iter_no_match_test.valid()); - ASSERT_TRUE(iter_op.ok()); delete filter_tree_root; filter_tree_root = nullptr; @@ -76,10 +75,10 @@ TEST_F(FilterTest, FilterTreeIterator) { filter_tree_root); ASSERT_TRUE(filter_op.ok()); - auto iter_no_match_multi_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root, iter_op); + auto iter_no_match_multi_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root); + ASSERT_TRUE(iter_no_match_multi_test.init_status().ok()); ASSERT_FALSE(iter_no_match_multi_test.valid()); - ASSERT_TRUE(iter_op.ok()); delete filter_tree_root; filter_tree_root = nullptr; @@ -87,14 +86,15 @@ TEST_F(FilterTest, FilterTreeIterator) { filter_tree_root); ASSERT_TRUE(filter_op.ok()); - auto iter_contains_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root, iter_op); + auto iter_contains_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root); + ASSERT_TRUE(iter_contains_test.init_status().ok()); + for (uint32_t i = 0; i < 5; i++) { ASSERT_TRUE(iter_contains_test.valid()); - ASSERT_EQ(i, iter_contains_test.doc); + ASSERT_EQ(i, iter_contains_test.seq_id); iter_contains_test.next(); } ASSERT_FALSE(iter_contains_test.valid()); - ASSERT_TRUE(iter_op.ok()); delete filter_tree_root; filter_tree_root = nullptr; @@ -102,14 +102,15 @@ TEST_F(FilterTest, FilterTreeIterator) { filter_tree_root); ASSERT_TRUE(filter_op.ok()); - auto iter_contains_multi_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root, iter_op); + auto iter_contains_multi_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root); + ASSERT_TRUE(iter_contains_multi_test.init_status().ok()); + for (uint32_t i = 0; i < 5; i++) { ASSERT_TRUE(iter_contains_multi_test.valid()); - ASSERT_EQ(i, iter_contains_multi_test.doc); + ASSERT_EQ(i, iter_contains_multi_test.seq_id); iter_contains_multi_test.next(); } ASSERT_FALSE(iter_contains_multi_test.valid()); - ASSERT_TRUE(iter_op.ok()); delete filter_tree_root; filter_tree_root = nullptr; @@ -117,14 +118,15 @@ TEST_F(FilterTest, FilterTreeIterator) { filter_tree_root); ASSERT_TRUE(filter_op.ok()); - auto iter_exact_match_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root, iter_op); + auto iter_exact_match_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root); + ASSERT_TRUE(iter_exact_match_test.init_status().ok()); + for (uint32_t i = 0; i < 5; i++) { ASSERT_TRUE(iter_exact_match_test.valid()); - ASSERT_EQ(i, iter_exact_match_test.doc); + ASSERT_EQ(i, iter_exact_match_test.seq_id); iter_exact_match_test.next(); } ASSERT_FALSE(iter_exact_match_test.valid()); - ASSERT_TRUE(iter_op.ok()); delete filter_tree_root; filter_tree_root = nullptr; @@ -132,16 +134,16 @@ TEST_F(FilterTest, FilterTreeIterator) { filter_tree_root); ASSERT_TRUE(filter_op.ok()); - auto iter_exact_match_multi_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root, iter_op); + 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 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.doc); + ASSERT_EQ(i, iter_exact_match_multi_test.seq_id); iter_exact_match_multi_test.next(); } ASSERT_FALSE(iter_exact_match_multi_test.valid()); - ASSERT_TRUE(iter_op.ok()); // delete filter_tree_root; // filter_tree_root = nullptr; @@ -149,17 +151,17 @@ TEST_F(FilterTest, FilterTreeIterator) { // filter_tree_root); // ASSERT_TRUE(filter_op.ok()); // -// auto iter_not_equals_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root, iter_op); +// auto iter_not_equals_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root); +// ASSERT_TRUE(iter_not_equals_test.init_status().ok()); // // std::vector expected = {1, 3}; // for (auto const& i : expected) { // ASSERT_TRUE(iter_not_equals_test.valid()); -// ASSERT_EQ(i, iter_not_equals_test.doc); +// ASSERT_EQ(i, iter_not_equals_test.seq_id); // iter_not_equals_test.next(); // } // // ASSERT_FALSE(iter_not_equals_test.valid()); -// ASSERT_TRUE(iter_op.ok()); delete filter_tree_root; filter_tree_root = nullptr; @@ -167,16 +169,16 @@ TEST_F(FilterTest, FilterTreeIterator) { filter_tree_root); ASSERT_TRUE(filter_op.ok()); - auto iter_skip_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root, iter_op); + auto iter_skip_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root); + ASSERT_TRUE(iter_skip_test.init_status().ok()); ASSERT_TRUE(iter_skip_test.valid()); iter_skip_test.skip_to(3); ASSERT_TRUE(iter_skip_test.valid()); - ASSERT_EQ(4, iter_skip_test.doc); + ASSERT_EQ(4, iter_skip_test.seq_id); iter_skip_test.next(); ASSERT_FALSE(iter_skip_test.valid()); - ASSERT_TRUE(iter_op.ok()); delete filter_tree_root; filter_tree_root = nullptr; @@ -184,14 +186,14 @@ TEST_F(FilterTest, FilterTreeIterator) { filter_tree_root); ASSERT_TRUE(filter_op.ok()); - auto iter_and_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root, iter_op); + auto iter_and_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root); + ASSERT_TRUE(iter_and_test.init_status().ok()); ASSERT_TRUE(iter_and_test.valid()); - ASSERT_EQ(1, iter_and_test.doc); + ASSERT_EQ(1, iter_and_test.seq_id); iter_and_test.next(); ASSERT_FALSE(iter_and_test.valid()); - ASSERT_TRUE(iter_op.ok()); delete filter_tree_root; filter_tree_root = nullptr; @@ -210,17 +212,17 @@ TEST_F(FilterTest, FilterTreeIterator) { auto add_op = coll->add(doc.dump()); ASSERT_TRUE(add_op.ok()); - auto iter_or_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root, iter_op); + auto iter_or_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root); + ASSERT_TRUE(iter_or_test.init_status().ok()); expected = {2, 4, 5}; for (auto const& i : expected) { ASSERT_TRUE(iter_or_test.valid()); - ASSERT_EQ(i, iter_or_test.doc); + ASSERT_EQ(i, iter_or_test.seq_id); iter_or_test.next(); } ASSERT_FALSE(iter_or_test.valid()); - ASSERT_TRUE(iter_op.ok()); delete filter_tree_root; filter_tree_root = nullptr; @@ -228,7 +230,8 @@ TEST_F(FilterTest, FilterTreeIterator) { filter_tree_root); ASSERT_TRUE(filter_op.ok()); - auto iter_skip_complex_filter_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root, iter_op); + auto iter_skip_complex_filter_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root); + ASSERT_TRUE(iter_skip_complex_filter_test.init_status().ok()); ASSERT_TRUE(iter_skip_complex_filter_test.valid()); iter_skip_complex_filter_test.skip_to(4); @@ -236,12 +239,11 @@ TEST_F(FilterTest, FilterTreeIterator) { expected = {4, 5}; for (auto const& i : expected) { ASSERT_TRUE(iter_skip_complex_filter_test.valid()); - ASSERT_EQ(i, iter_skip_complex_filter_test.doc); + ASSERT_EQ(i, iter_skip_complex_filter_test.seq_id); iter_skip_complex_filter_test.next(); } ASSERT_FALSE(iter_skip_complex_filter_test.valid()); - ASSERT_TRUE(iter_op.ok()); delete filter_tree_root; filter_tree_root = nullptr; @@ -249,15 +251,14 @@ TEST_F(FilterTest, FilterTreeIterator) { filter_tree_root); ASSERT_TRUE(filter_op.ok()); - auto iter_validate_ids_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), filter_tree_root, iter_op); + 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)); } - ASSERT_TRUE(iter_op.ok()); - delete filter_tree_root; filter_tree_root = nullptr; filter_op = filter::parse_filter_query("name: James || tags: != gold", coll->get_schema(), store, doc_id_prefix, @@ -265,14 +266,13 @@ TEST_F(FilterTest, FilterTreeIterator) { ASSERT_TRUE(filter_op.ok()); auto iter_validate_ids_not_equals_filter_test = filter_result_iterator_t(coll->get_name(), coll->_get_index(), - filter_tree_root, iter_op); + 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)); } - ASSERT_TRUE(iter_op.ok()); - delete filter_tree_root; } \ No newline at end of file