From efac704f1bf16ab63631343d7d9036b608e53294 Mon Sep 17 00:00:00 2001 From: Harpreet Sangar Date: Sat, 17 Jun 2023 12:39:18 +0530 Subject: [PATCH] Support id != --- include/filter.h | 2 +- src/filter.cpp | 5 ++++- src/filter_result_iterator.cpp | 20 ++++++++++++++----- test/collection_filtering_test.cpp | 32 ++++++++++++++++++++---------- 4 files changed, 42 insertions(+), 17 deletions(-) diff --git a/include/filter.h b/include/filter.h index f52b086f..01469d5e 100644 --- a/include/filter.h +++ b/include/filter.h @@ -19,7 +19,7 @@ struct filter { std::string field_name; std::vector values; std::vector comparators; - // Would be set when `field: != ...` is encountered with a string field or `field: != [ ... ]` is encountered in the + // Would be set when `field: != ...` is encountered with id/string field or `field: != [ ... ]` is encountered in the // case of int and float fields. During filtering, all the results of matching the field against the values are // aggregated and then this flag is checked if negation on the aggregated result is required. bool apply_not_equals = false; diff --git a/src/filter.cpp b/src/filter.cpp index c152d77f..5d94c66c 100644 --- a/src/filter.cpp +++ b/src/filter.cpp @@ -422,7 +422,10 @@ Option toFilter(const std::string expression, id_comparator = EQUALS; while (++filter_value_index < raw_value.size() && raw_value[filter_value_index] == ' '); } else if (raw_value.size() >= 2 && raw_value[0] == '!' && raw_value[1] == '=') { - return Option(400, "Not equals filtering is not supported on the `id` field."); + id_comparator = NOT_EQUALS; + filter_exp.apply_not_equals = true; + filter_value_index++; + while (++filter_value_index < raw_value.size() && raw_value[filter_value_index] == ' '); } if (filter_value_index != 0) { raw_value = raw_value.substr(filter_value_index); diff --git a/src/filter_result_iterator.cpp b/src/filter_result_iterator.cpp index 25cfd2c3..42816867 100644 --- a/src/filter_result_iterator.cpp +++ b/src/filter_result_iterator.cpp @@ -620,11 +620,6 @@ void filter_result_iterator_t::init() { } if (a_filter.field_name == "id") { - if (a_filter.values.empty()) { - is_valid = false; - return; - } - // we handle `ids` separately std::vector result_ids; for (const auto& id_str : a_filter.values) { @@ -637,6 +632,16 @@ void filter_result_iterator_t::init() { filter_result.docs = new uint32_t[result_ids.size()]; std::copy(result_ids.begin(), result_ids.end(), filter_result.docs); + if (a_filter.apply_not_equals) { + apply_not_equals(index->seq_ids->uncompress(), index->seq_ids->num_ids(), + filter_result.docs, filter_result.count); + } + + if (filter_result.count == 0) { + is_valid = false; + return; + } + seq_id = filter_result.docs[result_index]; is_filter_result_initialized = true; approx_filter_ids_length = filter_result.count; @@ -1660,6 +1665,11 @@ void filter_result_iterator_t::compute_result() { apply_not_equals(index->seq_ids->uncompress(), index->seq_ids->num_ids(), filter_result.docs, filter_result.count); } + if (filter_result.count == 0) { + is_valid = false; + return; + } + result_index = 0; seq_id = filter_result.docs[result_index]; is_filter_result_initialized = true; diff --git a/test/collection_filtering_test.cpp b/test/collection_filtering_test.cpp index 988b035a..b3a5e600 100644 --- a/test/collection_filtering_test.cpp +++ b/test/collection_filtering_test.cpp @@ -1231,6 +1231,16 @@ TEST_F(CollectionFilteringTest, FilteringViaDocumentIds) { ASSERT_EQ(1, results["hits"].size()); ASSERT_STREQ("123", results["hits"][0]["document"]["id"].get().c_str()); + results = coll1->search("*", + {}, "id: != 123", + {}, sort_fields, {0}, 10, 1, FREQUENCY, {true}).get(); + + ASSERT_EQ(3, results["found"].get()); + ASSERT_EQ(3, results["hits"].size()); + ASSERT_STREQ("125", results["hits"][0]["document"]["id"].get().c_str()); + ASSERT_STREQ("127", results["hits"][1]["document"]["id"].get().c_str()); + ASSERT_STREQ("129", results["hits"][2]["document"]["id"].get().c_str()); + // single ID with backtick results = coll1->search("*", @@ -1283,6 +1293,14 @@ TEST_F(CollectionFilteringTest, FilteringViaDocumentIds) { ASSERT_STREQ("125", results["hits"][1]["document"]["id"].get().c_str()); ASSERT_STREQ("127", results["hits"][2]["document"]["id"].get().c_str()); + results = coll1->search("*", + {}, "id:!= [123,125] && num_employees: <300", + {}, sort_fields, {0}, 10, 1, FREQUENCY, {true}).get(); + + ASSERT_EQ(1, results["found"].get()); + ASSERT_EQ(1, results["hits"].size()); + ASSERT_STREQ("127", results["hits"][0]["document"]["id"].get().c_str()); + // empty id list not allowed auto res_op = coll1->search("*", {}, "id:=", {}, sort_fields, {0}, 10, 1, FREQUENCY, {true}); ASSERT_FALSE(res_op.ok()); @@ -1296,13 +1314,6 @@ TEST_F(CollectionFilteringTest, FilteringViaDocumentIds) { ASSERT_FALSE(res_op.ok()); ASSERT_EQ("Error with filter field `id`: Filter value cannot be empty.", res_op.error()); - // not equals is not supported yet - res_op = coll1->search("*", - {}, "id:!= [123,125] && num_employees: <300", - {}, sort_fields, {0}, 10, 1, FREQUENCY, {true}); - ASSERT_FALSE(res_op.ok()); - ASSERT_EQ("Not equals filtering is not supported on the `id` field.", res_op.error()); - // when no IDs exist results = coll1->search("*", {}, "id: [1000] && num_employees: <300", @@ -1397,9 +1408,10 @@ TEST_F(CollectionFilteringTest, NumericalFilteringWithArray) { TEST_F(CollectionFilteringTest, NegationOperatorBasics) { Collection *coll1; - std::vector fields = {field("title", field_types::STRING, false), - field("artist", field_types::STRING, false), - field("points", field_types::INT32, false),}; + std::vector fields = { + field("title", field_types::STRING, false), + field("artist", field_types::STRING, false), + field("points", field_types::INT32, false),}; coll1 = collectionManager.get_collection("coll1").get(); if(coll1 == nullptr) {