From 285eb2fa5db40823ed136659bf9523936bf606ec Mon Sep 17 00:00:00 2001 From: Krunal Gandhi Date: Tue, 20 Feb 2024 11:21:46 +0000 Subject: [PATCH] fix excluding upper range val in search (#1564) * fix excluding upper range val in search * check lower_range while searching for range --- include/field.h | 21 +++++++++++++++---- src/collection.cpp | 5 +++-- test/collection_faceting_test.cpp | 23 +++++++++++++++++---- test/collection_optimized_faceting_test.cpp | 23 +++++++++++++++++---- 4 files changed, 58 insertions(+), 14 deletions(-) diff --git a/include/field.h b/include/field.h index b74901b8..2a6f00e3 100644 --- a/include/field.h +++ b/include/field.h @@ -699,6 +699,15 @@ struct facet_stats_t { fvsum = 0; }; +struct range_specs_t { + std::string range_label; + int64_t lower_range; + + bool is_in_range(int64_t key) { + return key >= lower_range; + } +}; + struct facet { const std::string field_name; spp::sparse_hash_map result_map; @@ -714,7 +723,7 @@ struct facet { facet_stats_t stats; //dictionary of key=>pair(range_id, range_val) - std::map facet_range_map; + std::map facet_range_map; bool is_range_query; @@ -739,16 +748,20 @@ struct facet { auto it = facet_range_map.lower_bound(key); - if(it != facet_range_map.end()) { + if(it != facet_range_map.end() && it->first == key) { + it++; + } + + if(it != facet_range_map.end() && it->second.is_in_range(key)) { range_pair.first = it->first; - range_pair.second = it->second; + range_pair.second = it->second.range_label; return true; } return false; } - explicit facet(const std::string& field_name, std::map facet_range = {}, + explicit facet(const std::string& field_name, std::map facet_range = {}, bool is_range_q = false, bool sort_by_alpha=false, const std::string& order="", const std::string& sort_by_field="", uint32_t orig_index = 0) : field_name(field_name), facet_range_map(facet_range), diff --git a/src/collection.cpp b/src/collection.cpp index 132a71c9..62e2c726 100644 --- a/src/collection.cpp +++ b/src/collection.cpp @@ -2840,7 +2840,7 @@ Option Collection::search(std::string raw_query, auto facet_range_iter = a_facet.facet_range_map.find(kv.first); if(facet_range_iter != a_facet.facet_range_map.end()){ auto & facet_count = kv.second; - facet_value_t facet_value = {facet_range_iter->second, std::string(), facet_count.count}; + facet_value_t facet_value = {facet_range_iter->second.range_label, std::string(), facet_count.count}; facet_values.emplace_back(facet_value); } else{ @@ -6117,6 +6117,7 @@ Option Collection::parse_facet(const std::string& facet_field, std::vector std::vector> tupVec; auto& range_map = a_facet.facet_range_map; + range_map.clear(); for(const auto& range : result){ //validate each range syntax if(!std::regex_match(range, range_pattern)){ @@ -6192,7 +6193,7 @@ Option Collection::parse_facet(const std::string& facet_field, std::vector return Option(400, error); } - range_map[upper_range] = range_val; + range_map[upper_range] = range_specs_t{range_val, lower_range}; } a_facet.is_range_query = true; diff --git a/test/collection_faceting_test.cpp b/test/collection_faceting_test.cpp index 7df37703..fcd61166 100644 --- a/test/collection_faceting_test.cpp +++ b/test/collection_faceting_test.cpp @@ -1733,6 +1733,21 @@ TEST_F(CollectionFacetingTest, RangeFacetsFloatRange) { ASSERT_EQ(1, results["facet_counts"][0]["counts"].size()); ASSERT_EQ(2, (int) results["facet_counts"][0]["counts"][0]["count"]); ASSERT_EQ("small", results["facet_counts"][0]["counts"][0]["value"]); + + results = coll1->search("*", {}, + "", {"inches(big:[55, 55.6])"}, + {}, {2}, 10, + 1, FREQUENCY, {true}, + 10, spp::sparse_hash_set(), + spp::sparse_hash_set(), 10, "", 30, 4, "", 10, {}, {}, {}, 0, + "", "", {}, 1000, + true, false, true, "", true, + 6000*1000, 4, 7, fallback, 4, {off}, INT16_MAX, INT16_MAX, + 2, 2, false, "", true, 0, max_score, 100, 0, 0, VALUE).get(); + + ASSERT_EQ(1, results["facet_counts"][0]["counts"].size()); + ASSERT_EQ(1, (int) results["facet_counts"][0]["counts"][0]["count"]); + ASSERT_EQ("big", results["facet_counts"][0]["counts"][0]["value"]); } TEST_F(CollectionFacetingTest, RangeFacetsMinMaxRange) { @@ -1771,9 +1786,9 @@ TEST_F(CollectionFacetingTest, RangeFacetsMinMaxRange) { ASSERT_EQ(2, results["facet_counts"][0]["counts"].size()); ASSERT_EQ(2, (int) results["facet_counts"][0]["counts"][0]["count"]); - ASSERT_EQ("small", results["facet_counts"][0]["counts"][0]["value"]); + ASSERT_EQ("large", results["facet_counts"][0]["counts"][0]["value"]); ASSERT_EQ(1, (int) results["facet_counts"][0]["counts"][1]["count"]); - ASSERT_EQ("large", results["facet_counts"][0]["counts"][1]["value"]); + ASSERT_EQ("small", results["facet_counts"][0]["counts"][1]["value"]); results = coll1->search("*", {}, "", {"inches(small:[,55])"}, @@ -1785,7 +1800,7 @@ TEST_F(CollectionFacetingTest, RangeFacetsMinMaxRange) { true, false, true, "", true).get(); ASSERT_EQ(1, results["facet_counts"][0]["counts"].size()); - ASSERT_EQ(2, (int) results["facet_counts"][0]["counts"][0]["count"]); + ASSERT_EQ(1, (int) results["facet_counts"][0]["counts"][0]["count"]); ASSERT_EQ("small", results["facet_counts"][0]["counts"][0]["value"]); } @@ -1824,7 +1839,7 @@ TEST_F(CollectionFacetingTest, RangeFacetRangeLabelWithSpace) { true, false, true, "", true).get(); ASSERT_EQ(1, results["facet_counts"][0]["counts"].size()); - ASSERT_EQ(2, (int) results["facet_counts"][0]["counts"][0]["count"]); + ASSERT_EQ(1, (int) results["facet_counts"][0]["counts"][0]["count"]); ASSERT_EQ("small tvs with display size", results["facet_counts"][0]["counts"][0]["value"]); } diff --git a/test/collection_optimized_faceting_test.cpp b/test/collection_optimized_faceting_test.cpp index 5a580089..0e12a518 100644 --- a/test/collection_optimized_faceting_test.cpp +++ b/test/collection_optimized_faceting_test.cpp @@ -2735,6 +2735,21 @@ TEST_F(CollectionOptimizedFacetingTest, RangeFacetsFloatRange) { ASSERT_EQ(1, results["facet_counts"][0]["counts"].size()); ASSERT_EQ(2, (int) results["facet_counts"][0]["counts"][0]["count"]); ASSERT_EQ("small", results["facet_counts"][0]["counts"][0]["value"]); + + results = coll1->search("*", {}, + "", {"inches(big:[55, 55.6])"}, + {}, {2}, 10, + 1, FREQUENCY, {true}, + 10, spp::sparse_hash_set(), + spp::sparse_hash_set(), 10, "", 30, 4, "", 10, {}, {}, {}, 0, + "", "", {}, 1000, + true, false, true, "", true, + 6000*1000, 4, 7, fallback, 4, {off}, INT16_MAX, INT16_MAX, + 2, 2, false, "", true, 0, max_score, 100, 0, 0, VALUE).get(); + + ASSERT_EQ(1, results["facet_counts"][0]["counts"].size()); + ASSERT_EQ(1, (int) results["facet_counts"][0]["counts"][0]["count"]); + ASSERT_EQ("big", results["facet_counts"][0]["counts"][0]["value"]); } TEST_F(CollectionOptimizedFacetingTest, RangeFacetsMinMaxRange) { @@ -2775,9 +2790,9 @@ TEST_F(CollectionOptimizedFacetingTest, RangeFacetsMinMaxRange) { ASSERT_EQ(2, results["facet_counts"][0]["counts"].size()); ASSERT_EQ(2, (int) results["facet_counts"][0]["counts"][0]["count"]); - ASSERT_EQ("small", results["facet_counts"][0]["counts"][0]["value"]); + ASSERT_EQ("large", results["facet_counts"][0]["counts"][0]["value"]); ASSERT_EQ(1, (int) results["facet_counts"][0]["counts"][1]["count"]); - ASSERT_EQ("large", results["facet_counts"][0]["counts"][1]["value"]); + ASSERT_EQ("small", results["facet_counts"][0]["counts"][1]["value"]); results = coll1->search("*", {}, "", {"inches(small:[,55])"}, @@ -2791,7 +2806,7 @@ TEST_F(CollectionOptimizedFacetingTest, RangeFacetsMinMaxRange) { 2, 2, false, "", true, 0, max_score, 100, 0, 0, VALUE).get(); ASSERT_EQ(1, results["facet_counts"][0]["counts"].size()); - ASSERT_EQ(2, (int) results["facet_counts"][0]["counts"][0]["count"]); + ASSERT_EQ(1, (int) results["facet_counts"][0]["counts"][0]["count"]); ASSERT_EQ("small", results["facet_counts"][0]["counts"][0]["value"]); } @@ -2832,6 +2847,6 @@ TEST_F(CollectionOptimizedFacetingTest, RangeFacetRangeLabelWithSpace) { 2, 2, false, "", true, 0, max_score, 100, 0, 0, VALUE).get(); ASSERT_EQ(1, results["facet_counts"][0]["counts"].size()); - ASSERT_EQ(2, (int) results["facet_counts"][0]["counts"][0]["count"]); + ASSERT_EQ(1, (int) results["facet_counts"][0]["counts"][0]["count"]); ASSERT_EQ("small tvs with display size", results["facet_counts"][0]["counts"][0]["value"]); }