diff --git a/include/field.h b/include/field.h index 7eee5c79..dd0033eb 100644 --- a/include/field.h +++ b/include/field.h @@ -606,7 +606,6 @@ struct filter_node_t { bool isOperator; filter_node_t* left = nullptr; filter_node_t* right = nullptr; - filter_tree_metrics* metrics = nullptr; filter_node_t(filter filter_exp) : filter_exp(std::move(filter_exp)), @@ -623,7 +622,6 @@ struct filter_node_t { right(right) {} ~filter_node_t() { - delete metrics; delete left; delete right; } diff --git a/include/index.h b/include/index.h index ff8431c8..62e75180 100644 --- a/include/index.h +++ b/include/index.h @@ -468,17 +468,17 @@ private: void numeric_not_equals_filter(num_tree_t* const num_tree, const int64_t value, const uint32_t& context_ids_length, - const uint32_t* context_ids, + uint32_t* const& context_ids, size_t& ids_len, uint32_t*& ids) const; bool field_is_indexed(const std::string& field_name) const; - Option do_filtering(filter_node_t* const root, - filter_result_t& result, - const std::string& collection_name = "", - const uint32_t& context_ids_length = 0, - const uint32_t* context_ids = nullptr) const; + Option _do_filtering(filter_node_t* const root, + filter_result_t& result, + const std::string& collection_name = "", + const uint32_t& context_ids_length = 0, + uint32_t* const& context_ids = nullptr) const; void aproximate_numerical_match(num_tree_t* const num_tree, const NUM_COMPARATOR& comparator, @@ -488,7 +488,9 @@ private: Option recursive_filter(filter_node_t* const root, filter_result_t& result, - const std::string& collection_name = "") const; + const std::string& collection_name = "", + const uint32_t& context_ids_length = 0, + uint32_t* const& context_ids = nullptr) const; Option adaptive_filter(filter_node_t* const filter_tree_root, filter_result_t& result, @@ -689,17 +691,13 @@ public: filter_result_t& filter_result, const std::string& collection_name = "") const; - Option _rearranging_recursive_filter(filter_node_t* const filter_tree_root, - filter_result_t& result, - const std::string& collection_name = "") const; - - Option _rearrange_filter_tree(filter_node_t* const root, + Option rearrange_filter_tree(filter_node_t* const root, uint32_t& filter_ids_length, const std::string& collection_name = "") const; Option _approximate_filter_ids(const filter& a_filter, - uint32_t& filter_ids_length, - const std::string& collection_name = "") const; + uint32_t& filter_ids_length, + const std::string& collection_name = "") const; Option do_reference_filtering_with_lock(filter_node_t* const filter_tree_root, filter_result_t& filter_result, diff --git a/include/num_tree.h b/include/num_tree.h index 280f47dd..5406a109 100644 --- a/include/num_tree.h +++ b/include/num_tree.h @@ -34,7 +34,7 @@ public: void range_inclusive_contains(const int64_t& start, const int64_t& end, const uint32_t& context_ids_length, - const uint32_t*& context_ids, + uint32_t* const& context_ids, size_t& result_ids_len, uint32_t*& result_ids) const; @@ -50,7 +50,7 @@ public: void contains(const NUM_COMPARATOR& comparator, const int64_t& value, const uint32_t& context_ids_length, - const uint32_t*& context_ids, + uint32_t* const& context_ids, size_t& result_ids_len, uint32_t*& result_ids) const; }; \ No newline at end of file diff --git a/src/field.cpp b/src/field.cpp index 129c7512..c7297359 100644 --- a/src/field.cpp +++ b/src/field.cpp @@ -384,9 +384,7 @@ Option toFilter(const std::string expression, Option toParseTree(std::queue& postfix, filter_node_t*& root, const tsl::htrie_map& search_schema, const Store* store, - const std::string& doc_id_prefix, - int& and_operator_count, - int& or_operator_count) { + const std::string& doc_id_prefix) { std::stack nodeStack; bool is_successful = true; std::string error_message; @@ -413,7 +411,6 @@ Option toParseTree(std::queue& postfix, filter_node_t*& root, auto operandA = nodeStack.top(); nodeStack.pop(); - expression == "&&" ? and_operator_count++ : or_operator_count++; filter_node = new filter_node_t(expression == "&&" ? AND : OR, operandA, operandB); } else { filter filter_exp; @@ -502,22 +499,15 @@ Option filter::parse_filter_query(const std::string& filter_query, return toPostfix_op; } - int postfix_size = (int) postfix.size(), and_operator_count = 0, or_operator_count = 0; Option toParseTree_op = toParseTree(postfix, root, search_schema, store, - doc_id_prefix, - and_operator_count, - or_operator_count); + doc_id_prefix); if (!toParseTree_op.ok()) { return toParseTree_op; } - root->metrics = new filter_tree_metrics{static_cast(postfix_size - (and_operator_count + or_operator_count)), - and_operator_count, - or_operator_count}; - return Option(true); } diff --git a/src/index.cpp b/src/index.cpp index 124cf567..73b0cd08 100644 --- a/src/index.cpp +++ b/src/index.cpp @@ -1452,7 +1452,7 @@ void Index::search_candidates(const uint8_t & field_id, bool field_is_array, void Index::numeric_not_equals_filter(num_tree_t* const num_tree, const int64_t value, const uint32_t& context_ids_length, - const uint32_t* context_ids, + uint32_t* const& context_ids, size_t& ids_len, uint32_t*& ids) const { uint32_t* to_exclude_ids = nullptr; @@ -1491,11 +1491,11 @@ bool Index::field_is_indexed(const std::string& field_name) const { geopoint_index.count(field_name) != 0; } -Option Index::do_filtering(filter_node_t* const root, - filter_result_t& result, - const std::string& collection_name, - const uint32_t& context_ids_length, - const uint32_t* context_ids) const { +Option Index::_do_filtering(filter_node_t* const root, + filter_result_t& result, + const std::string& collection_name, + const uint32_t& context_ids_length, + uint32_t* const& context_ids) const { // auto begin = std::chrono::high_resolution_clock::now(); const filter a_filter = root->filter_exp; @@ -1953,8 +1953,8 @@ void Index::aproximate_numerical_match(num_tree_t* const num_tree, } Option Index::_approximate_filter_ids(const filter& a_filter, - uint32_t& filter_ids_length, - const std::string& collection_name) const { + uint32_t& filter_ids_length, + const std::string& collection_name) const { if (!a_filter.referenced_collection_name.empty()) { auto& cm = CollectionManager::get_instance(); auto collection = cm.get_collection(a_filter.referenced_collection_name); @@ -2054,7 +2054,7 @@ Option Index::_approximate_filter_ids(const filter& a_filter, return Option(true); } -Option Index::_rearrange_filter_tree(filter_node_t* const root, +Option Index::rearrange_filter_tree(filter_node_t* const root, uint32_t& filter_ids_length, const std::string& collection_name) const { if (root == nullptr) { @@ -2064,7 +2064,7 @@ Option Index::_rearrange_filter_tree(filter_node_t* const root, if (root->isOperator) { uint32_t l_filter_ids_length = 0; if (root->left != nullptr) { - auto rearrange_op = _rearrange_filter_tree(root->left, l_filter_ids_length, collection_name); + auto rearrange_op = rearrange_filter_tree(root->left, l_filter_ids_length, collection_name); if (!rearrange_op.ok()) { return rearrange_op; } @@ -2072,7 +2072,7 @@ Option Index::_rearrange_filter_tree(filter_node_t* const root, uint32_t r_filter_ids_length = 0; if (root->right != nullptr) { - auto rearrange_op = _rearrange_filter_tree(root->right, r_filter_ids_length, collection_name); + auto rearrange_op = rearrange_filter_tree(root->right, r_filter_ids_length, collection_name); if (!rearrange_op.ok()) { return rearrange_op; } @@ -2095,18 +2095,6 @@ Option Index::_rearrange_filter_tree(filter_node_t* const root, return Option(true); } -Option Index::_rearranging_recursive_filter(filter_node_t* const filter_tree_root, - filter_result_t& result, - const std::string& collection_name) const { - uint32_t filter_ids_length = 0; - auto rearrange_op = _rearrange_filter_tree(filter_tree_root, filter_ids_length, collection_name); - if (!rearrange_op.ok()) { - return rearrange_op; - } - - return recursive_filter(filter_tree_root, result, collection_name); -} - void copy_reference_ids(filter_result_t& from, filter_result_t& to) { if (to.count > 0 && !from.reference_filter_results.empty()) { for (const auto &item: from.reference_filter_results) { @@ -2132,7 +2120,9 @@ void copy_reference_ids(filter_result_t& from, filter_result_t& to) { Option Index::recursive_filter(filter_node_t* const root, filter_result_t& result, - const std::string& collection_name) const { + const std::string& collection_name, + const uint32_t& context_ids_length, + uint32_t* const& context_ids) const { if (root == nullptr) { return Option(true); } @@ -2140,7 +2130,7 @@ Option Index::recursive_filter(filter_node_t* const root, if (root->isOperator) { filter_result_t l_result; if (root->left != nullptr) { - auto filter_op = recursive_filter(root->left, l_result , collection_name); + auto filter_op = recursive_filter(root->left, l_result , collection_name, context_ids_length, context_ids); if (!filter_op.ok()) { return filter_op; } @@ -2148,7 +2138,7 @@ Option Index::recursive_filter(filter_node_t* const root, filter_result_t r_result; if (root->right != nullptr) { - auto filter_op = recursive_filter(root->right, r_result , collection_name); + auto filter_op = recursive_filter(root->right, r_result , collection_name, context_ids_length, context_ids); if (!filter_op.ok()) { return filter_op; } @@ -2173,7 +2163,7 @@ Option Index::recursive_filter(filter_node_t* const root, return Option(true); } - return do_filtering(root, result, collection_name); + return _do_filtering(root, result, collection_name, context_ids_length, context_ids); } Option Index::adaptive_filter(filter_node_t* const filter_tree_root, @@ -2183,16 +2173,13 @@ Option Index::adaptive_filter(filter_node_t* const filter_tree_root, return Option(true); } - auto metrics = filter_tree_root->metrics; - if (metrics != nullptr && - metrics->filter_exp_count > 2 && - metrics->and_operator_count > 0 && - // If there are more || in the filter tree than &&, we'll not gain much by rearranging the filter tree. - ((float) metrics->or_operator_count / (float) metrics->and_operator_count < 0.5)) { - return _rearranging_recursive_filter(filter_tree_root, result, collection_name); - } else { - return recursive_filter(filter_tree_root, result, collection_name); + uint32_t filter_ids_length = 0; + auto op = rearrange_filter_tree(filter_tree_root, filter_ids_length, collection_name); + if (!op.ok()) { + return op; } + + return recursive_filter(filter_tree_root, result, collection_name); } Option Index::do_filtering_with_lock(filter_node_t* const filter_tree_root, @@ -2252,7 +2239,7 @@ Option Index::get_approximate_reference_filter_ids_with_lock(filter_node_t uint32_t& filter_ids_length) const { std::shared_lock lock(mutex); - return _rearrange_filter_tree(filter_tree_root, filter_ids_length); + return rearrange_filter_tree(filter_tree_root, filter_ids_length); } Option Index::run_search(search_args* search_params, const std::string& collection_name) { diff --git a/src/num_tree.cpp b/src/num_tree.cpp index 5a1b95d3..1bcdbc9f 100644 --- a/src/num_tree.cpp +++ b/src/num_tree.cpp @@ -75,7 +75,7 @@ bool num_tree_t::range_inclusive_contains(const int64_t& start, const int64_t& e void num_tree_t::range_inclusive_contains(const int64_t& start, const int64_t& end, const uint32_t& context_ids_length, - const uint32_t*& context_ids, + uint32_t* const& context_ids, size_t& result_ids_len, uint32_t*& result_ids) const { if (int64map.empty()) { @@ -251,7 +251,7 @@ void num_tree_t::remove(uint64_t value, uint32_t id) { void num_tree_t::contains(const NUM_COMPARATOR& comparator, const int64_t& value, const uint32_t& context_ids_length, - const uint32_t*& context_ids, + uint32_t* const& context_ids, size_t& result_ids_len, uint32_t*& result_ids) const { if (int64map.empty()) { diff --git a/test/collection_specific_more_test.cpp b/test/collection_specific_more_test.cpp index b34b9973..2e9369cf 100644 --- a/test/collection_specific_more_test.cpp +++ b/test/collection_specific_more_test.cpp @@ -2076,8 +2076,8 @@ TEST_F(CollectionSpecificMoreTest, RearrangingFilterTree) { ASSERT_TRUE(root->left == nullptr); ASSERT_TRUE(root->right == nullptr); - filter_result_t result; - coll->_get_index()->_rearranging_recursive_filter(filter_tree_root, result); + uint32_t count = 0; + coll->_get_index()->rearrange_filter_tree(filter_tree_root, count); // && // / \ @@ -2199,7 +2199,7 @@ TEST_F(CollectionSpecificMoreTest, ApproxFilterMatchCount) { coll->get_schema(), store, doc_id_prefix, filter_tree_root); ASSERT_TRUE(filter_op.ok()); - coll->_get_index()->_rearrange_filter_tree(filter_tree_root, approx_count); + coll->_get_index()->rearrange_filter_tree(filter_tree_root, approx_count); ASSERT_EQ(approx_count, 3); delete filter_tree_root;