Skip to content

Commit 4d3ef42

Browse files
authored
feat(search): FT.SEARCH FILTER option (#5830)
Even it is deprecated FILTER can still be used as part of FT.SEARCH command. Add support for it by reconstruction of AST tree. Closes #5822 Signed-off-by: mkaruza <[email protected]>
1 parent 4d3b0bc commit 4d3ef42

File tree

8 files changed

+128
-6
lines changed

8 files changed

+128
-6
lines changed

src/core/search/base.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,19 @@ struct QueryParams {
3838
absl::flat_hash_map<std::string, std::string> params;
3939
};
4040

41+
// Base class for optional search filters
42+
43+
class AstNode;
44+
45+
struct OptionalFilterBase {
46+
virtual bool IsEmpty() const = 0;
47+
virtual AstNode Node(std::string field) = 0;
48+
virtual ~OptionalFilterBase() = default;
49+
};
50+
51+
using OptionalFilters =
52+
absl::flat_hash_map<std::string /*field*/, std::unique_ptr<OptionalFilterBase> /* filter */>;
53+
4154
// Values are either sortable as doubles or strings, or not sortable at all.
4255
using SortableValue = std::variant<std::monostate, double, std::string>;
4356

src/core/search/query_driver.cc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,14 @@ void QueryDriver::Error(const Parser::location_type& loc, std::string_view msg)
2222
VLOG(1) << "Parse error " << loc << ": " << msg;
2323
}
2424

25+
void QueryDriver::SetOptionalFilters(const OptionalFilters* filters) {
26+
if (filters) {
27+
for (auto& [field, filter] : *filters) {
28+
expr_ = AstLogicalNode(std::move(expr_), filter->Node(field), AstLogicalNode::AND);
29+
}
30+
}
31+
}
32+
2533
} // namespace search
2634

2735
} // namespace dfly

src/core/search/query_driver.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ class QueryDriver {
3030
scanner_->SetParams(params);
3131
}
3232

33+
void SetOptionalFilters(const OptionalFilters* filters);
34+
3335
Parser::symbol_type Lex() {
3436
return scanner()->Lex();
3537
}

src/core/search/search.cc

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,14 @@ namespace dfly::search {
3131

3232
namespace {
3333

34-
AstExpr ParseQuery(std::string_view query, const QueryParams* params) {
34+
AstExpr ParseQuery(std::string_view query, const QueryParams* params,
35+
const OptionalFilters* filters) {
3536
QueryDriver driver{};
3637
driver.ResetScanner();
3738
driver.SetParams(params);
3839
driver.SetInput(std::string{query});
3940
(void)Parser (&driver)(); // can throw
41+
driver.SetOptionalFilters(filters);
4042
return driver.Take();
4143
}
4244

@@ -430,6 +432,10 @@ struct BasicSearch {
430432

431433
} // namespace
432434

435+
AstNode OptionalNumericFilter::Node(std::string field) {
436+
return AstFieldNode{"@" + field, AstRangeNode(lo_, false, hi_, false)};
437+
}
438+
433439
string_view Schema::LookupAlias(string_view alias) const {
434440
if (auto it = field_names.find(alias); it != field_names.end())
435441
return it->second;
@@ -611,9 +617,10 @@ const Synonyms* FieldIndices::GetSynonyms() const {
611617
SearchAlgorithm::SearchAlgorithm() = default;
612618
SearchAlgorithm::~SearchAlgorithm() = default;
613619

614-
bool SearchAlgorithm::Init(string_view query, const QueryParams* params) {
620+
bool SearchAlgorithm::Init(string_view query, const QueryParams* params,
621+
const OptionalFilters* filters) {
615622
try {
616-
query_ = make_unique<AstExpr>(ParseQuery(query, params));
623+
query_ = make_unique<AstExpr>(ParseQuery(query, params, filters));
617624
} catch (const Parser::syntax_error& se) {
618625
LOG(INFO) << "Failed to parse query \"" << query << "\":" << se.what();
619626
return false;

src/core/search/search.h

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,35 @@ namespace dfly::search {
2424
struct AstNode;
2525
struct TextIndex;
2626

27+
// Optional FILTER
28+
struct OptionalNumericFilter : public OptionalFilterBase {
29+
OptionalNumericFilter(size_t lo, size_t hi) : empty_(false), lo_(lo), hi_(hi) {
30+
}
31+
32+
bool IsEmpty() const override {
33+
return empty_;
34+
}
35+
36+
AstNode Node(std::string field) override;
37+
38+
void AddRange(size_t lo, size_t hi) {
39+
if (empty_) {
40+
return;
41+
}
42+
if ((hi_ < lo) || (hi < lo_)) {
43+
empty_ = true;
44+
} else {
45+
lo_ = std::max(lo_, lo);
46+
hi_ = std::min(hi_, hi);
47+
}
48+
}
49+
50+
private:
51+
bool empty_;
52+
size_t lo_;
53+
size_t hi_;
54+
};
55+
2756
// Describes a specific index field
2857
struct SchemaField {
2958
enum FieldType { TAG, TEXT, NUMERIC, VECTOR };
@@ -164,8 +193,9 @@ class SearchAlgorithm {
164193
SearchAlgorithm();
165194
~SearchAlgorithm();
166195

167-
// Init with query and return true if successful.
168-
bool Init(std::string_view query, const QueryParams* params);
196+
// Init with query and optional filters and return true if successful.
197+
bool Init(std::string_view query, const QueryParams* params,
198+
const OptionalFilters* filters = nullptr);
169199

170200
SearchResult Search(const FieldIndices* index) const;
171201

src/server/search/doc_index.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,9 @@ struct SearchParams {
113113
std::optional<std::vector<FieldReference>> load_fields;
114114

115115
std::optional<SortOption> sort_option;
116+
117+
search::OptionalFilters optional_filters;
118+
116119
search::QueryParams query_params;
117120

118121
bool ShouldReturnAllFields() const {

src/server/search/search_family.cc

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,20 @@ std::optional<std::string_view> ParseFieldWithAtSign(CmdArgParser* parser) {
344344
return field;
345345
}
346346

347+
void ParseNumericFilter(CmdArgParser* parser, SearchParams* params) {
348+
auto field = ParseField(parser);
349+
size_t lo = parser->Next<size_t>();
350+
size_t hi = parser->Next<size_t>();
351+
if (auto it = params->optional_filters.find(field); it != params->optional_filters.end()) {
352+
search::OptionalNumericFilter* numeric_filter =
353+
dynamic_cast<search::OptionalNumericFilter*>(it->second.get());
354+
numeric_filter->AddRange(lo, hi);
355+
} else {
356+
params->optional_filters.emplace(field,
357+
std::make_unique<search::OptionalNumericFilter>(lo, hi));
358+
}
359+
}
360+
347361
std::vector<FieldReference> ParseLoadOrReturnFields(CmdArgParser* parser, bool is_load) {
348362
// TODO: Change to num_strings. In Redis strings number is expected. For example: LOAD 3 $.a AS a
349363
std::vector<FieldReference> fields;
@@ -396,6 +410,8 @@ ParseResult<SearchParams> ParseSearchParams(CmdArgParser* parser) {
396410
FieldReference field{ParseField(parser)};
397411
params.sort_option =
398412
SearchParams::SortOption{field, parser->Check("DESC") ? SortOrder::DESC : SortOrder::ASC};
413+
} else if (parser->Check("FILTER")) {
414+
ParseNumericFilter(parser, &params);
399415
} else {
400416
// Unsupported parameters are ignored for now
401417
parser->Skip(1);
@@ -1214,7 +1230,7 @@ void SearchFamily::FtSearch(CmdArgList args, const CommandContext& cmd_cntx) {
12141230
return;
12151231

12161232
search::SearchAlgorithm search_algo;
1217-
if (!search_algo.Init(query_str, &params->query_params))
1233+
if (!search_algo.Init(query_str, &params->query_params, &params->optional_filters))
12181234
return builder->SendError("Query syntax error");
12191235

12201236
// Because our coordinator thread may not have a shard, we can't check ahead if the index exists.

src/server/search/search_family_test.cc

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3203,4 +3203,47 @@ TEST_F(SearchFamilyTest, AggregateWithLoadFromSortBySeveralFields) {
32033203
EXPECT_THAT(resp.GetVec(), ElementsAreArray(matchers));
32043204
}
32053205

3206+
TEST_F(SearchFamilyTest, NumericFilter) {
3207+
// Index name, age, height
3208+
Run({"FT.CREATE", "i1", "ON", "HASH", "SCHEMA", "name", "TEXT", "age", "NUMERIC", "height",
3209+
"NUMERIC"});
3210+
3211+
// Index name, age
3212+
Run({"FT.CREATE", "i2", "ON", "HASH", "SCHEMA", "name", "TEXT", "age", "NUMERIC"});
3213+
3214+
Run({"HSET", "id:1", "name", "John", "age", "28", "height", "184"});
3215+
Run({"HSET", "id:2", "name", "Ivan", "age", "30", "height", "180"});
3216+
Run({"HSET", "id:3", "name", "Jon", "age", "25", "height", "182"});
3217+
Run({"HSET", "id:4", "name", "Juan", "age", "32", "height", "186"});
3218+
Run({"HSET", "id:5", "name", "Ioan", "age", "35", "height", "181"});
3219+
3220+
// Filter with non-star query
3221+
auto res = Run({"FT.SEARCH", "i1", "I*", "FILTER", "age", "31", "40"});
3222+
EXPECT_THAT(res, AreDocIds("id:5"));
3223+
3224+
// Filter on ONE NUMERIC index
3225+
res = Run({"FT.SEARCH", "i1", "*", "FILTER", "age", "25", "28"});
3226+
EXPECT_THAT(res, AreDocIds("id:1", "id:3"));
3227+
3228+
// Filter on TWO NUMERIC indexes
3229+
res =
3230+
Run({"FT.SEARCH", "i1", "*", "FILTER", "age", "25", "28", "FILTER", "height", "180", "182"});
3231+
EXPECT_THAT(res, AreDocIds("id:3"));
3232+
3233+
// Filter on TWO NUMERIC indexes where second filtering produce empty result
3234+
res =
3235+
Run({"FT.SEARCH", "i1", "*", "FILTER", "age", "25", "28", "FILTER", "height", "200", "300"});
3236+
EXPECT_THAT(res, AreDocIds());
3237+
3238+
// Filter on index which doesn't exists
3239+
res = Run({"FT.SEARCH", "i2", "*", "FILTER", "height", "180", "190"});
3240+
EXPECT_THAT(res, ErrArg("Invalid field: height"));
3241+
3242+
// Two filters on same field
3243+
res = Run({"FT.SEARCH", "i1", "J*", "FILTER", "age", "25", "30", "FILTER", "age", "28", "32"});
3244+
EXPECT_THAT(res, AreDocIds("id:1"));
3245+
3246+
Run({"FLUSHALL"});
3247+
}
3248+
32063249
} // namespace dfly

0 commit comments

Comments
 (0)