mirrors/dragonfly
1
0
Fork 0
mirror of https://github.com/dragonflydb/dragonfly.git synced 2025-05-10 18:05:44 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

fix(search_family): Add options test for the FT.SEARCH command (#4478)

Browse source 
Signed-off-by: Stepan Bagritsevich <stefan@dragonflydb.io>
This commit is contained in:
Stepan Bagritsevich 2025-01-30 10:43:01 +01:00 • committed by GitHub
parent fc94b2c265
commit 83569aca28
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 74 additions and 13 deletions
Download patch file Download diff file Expand all files Collapse all files

3
src/server/search/doc_index.h
View file

@ -148,6 +148,7 @@ struct SearchParams {
Only one of load_fields and return_fields should be set.
*/
std::optional<SearchFieldsList> load_fields;
bool no_content = false;
std::optional<search::SortOption> sort_option;
search::QueryParams query_params;
@ -157,7 +158,7 @@ struct SearchParams {
}
bool IdsOnly() const {
return return_fields && return_fields->empty();
return no_content || (return_fields && return_fields->empty());
}
bool ShouldReturnField(std::string_view alias) const;

9
src/server/search/search_family.cc
View file

@ -221,7 +221,7 @@ void ParseLoadFields(CmdArgParser* parser, std::optional<SearchFieldsList>* load
load_fields->emplace();
}
while (num_fields--) {
while (parser->HasNext() && num_fields--) {
string_view str = parser->Next();
if (absl::StartsWith(str, "@"sv)) {
@ -274,7 +274,7 @@ optional<SearchParams> ParseSearchParamsOrReply(CmdArgParser* parser, SinkReplyB
* AS a */
size_t num_fields = parser->Next<size_t>();
params.return_fields.emplace();
while (params.return_fields->size() < num_fields) {
while (parser->HasNext() && params.return_fields->size() < num_fields) {
StringOrView name = StringOrView::FromString(parser->Next<std::string>());
if (parser->Check("AS")) {
@ -285,8 +285,7 @@ optional<SearchParams> ParseSearchParamsOrReply(CmdArgParser* parser, SinkReplyB
}
}
} else if (parser->Check("NOCONTENT")) { // NOCONTENT
params.load_fields.emplace();
params.return_fields.emplace();
params.no_content = true;
} else if (parser->Check("PARAMS")) { // [PARAMS num(ignored) name(ignored) knn_vector]
params.query_params = ParseQueryParams(parser);
} else if (parser->Check("SORTBY")) {
@ -360,7 +359,7 @@ optional<AggregateParams> ParseAggregatorParamsOrReply(CmdArgParser parser,
std::vector<std::string> fields;
fields.reserve(num_fields);
while (num_fields > 0 && parser.HasNext()) {
while (parser.HasNext() && num_fields > 0) {
auto parsed_field = ParseFieldWithAtSign(&parser);
/*

75
src/server/search/search_family_test.cc
View file

@ -6,6 +6,7 @@
#include "base/gtest.h"
#include "base/logging.h"
#include "facade/error.h"
#include "facade/facade_test.h"
#include "server/command_registry.h"
#include "server/test_utils.h"
@ -13,6 +14,7 @@
using namespace testing;
using namespace std;
using namespace util;
using namespace facade;
namespace dfly {
@ -628,10 +630,10 @@ TEST_F(SearchFamilyTest, TestReturn) {
// Check no fields are returned
resp = Run({"ft.search", "i1", "@justA:0", "return", "0"});
EXPECT_THAT(resp, RespArray(ElementsAre(IntArg(1), "k0")));
EXPECT_THAT(resp, IsArray(IntArg(1), "k0"));
resp = Run({"ft.search", "i1", "@justA:0", "nocontent"});
EXPECT_THAT(resp, RespArray(ElementsAre(IntArg(1), "k0")));
EXPECT_THAT(resp, IsArray(IntArg(1), "k0"));
// Check only one field is returned (and with original identifier)
resp = Run({"ft.search", "i1", "@justA:0", "return", "1", "longA"});
@ -856,7 +858,7 @@ TEST_F(SearchFamilyTest, FtProfileErrorReply) {
EXPECT_THAT(resp, ErrArg("no `SEARCH` or `AGGREGATE` provided"));
resp = Run({"ft.profile", "i1", "search", "not_query", "(a | b) c d"});
EXPECT_THAT(resp, ErrArg("syntax error"));
EXPECT_THAT(resp, ErrArg(kSyntaxErr));
resp = Run({"ft.profile", "non_existent_key", "search", "query", "(a | b) c d"});
EXPECT_THAT(resp, ErrArg("non_existent_key: no such index"));
@ -1803,15 +1805,15 @@ TEST_F(SearchFamilyTest, AggregateSortByParsingErrors) {
// Test SORTBY with negative argument count
resp = Run({"FT.AGGREGATE", "index", "*", "SORTBY", "-3", "@name", "@number", "DESC"});
EXPECT_THAT(resp, ErrArg("value is not an integer or out of range"));
EXPECT_THAT(resp, ErrArg(kInvalidIntErr));
// Test MAX with invalid value
resp = Run({"FT.AGGREGATE", "index", "*", "SORTBY", "1", "@name", "MAX", "-10"});
EXPECT_THAT(resp, ErrArg("value is not an integer or out of range"));
EXPECT_THAT(resp, ErrArg(kInvalidIntErr));
// Test MAX without a value
resp = Run({"FT.AGGREGATE", "index", "*", "SORTBY", "1", "@name", "MAX"});
EXPECT_THAT(resp, ErrArg("syntax error"));
EXPECT_THAT(resp, ErrArg(kSyntaxErr));
// Test SORTBY with a non-existing field
/* Temporary unsupported
@ -1820,7 +1822,66 @@ TEST_F(SearchFamilyTest, AggregateSortByParsingErrors) {
// Test SORTBY with an invalid value
resp = Run({"FT.AGGREGATE", "index", "*", "SORTBY", "notvalue", "@name"});
EXPECT_THAT(resp, ErrArg("value is not an integer or out of range"));
EXPECT_THAT(resp, ErrArg(kInvalidIntErr));
}
TEST_F(SearchFamilyTest, InvalidSearchOptions) {
Run({"JSON.SET", "j1", ".", R"({"field1":"first","field2":"second"})"});
Run({"FT.CREATE", "idx", "ON", "JSON", "SCHEMA", "$.field1", "AS", "field1", "TEXT", "$.field2",
"AS", "field2", "TEXT"});
/* Test with an empty query and LOAD. TODO: Add separate test for query syntax
auto resp = Run({"FT.SEARCH", "idx", "", "LOAD", "1", "@field1"});
EXPECT_THAT(resp, IsMapWithSize()); */
// Test with LIMIT missing arguments
auto resp = Run({"FT.SEARCH", "idx", "*", "LIMIT", "0"});
EXPECT_THAT(resp, ErrArg(kSyntaxErr));
// Test with LIMIT exceeding the maximum allowed value
resp = Run({"FT.SEARCH", "idx", "*", "LIMIT", "0", "100000000000000000000"});
EXPECT_THAT(resp, ErrArg(kInvalidIntErr));
// Test with LIMIT and negative arguments
resp = Run({"FT.SEARCH", "idx", "*", "LIMIT", "-1", "10"});
EXPECT_THAT(resp, ErrArg(kInvalidIntErr));
// Test with LIMIT and invalid argument types
resp = Run({"FT.SEARCH", "idx", "*", "LIMIT", "start", "count"});
EXPECT_THAT(resp, ErrArg(kInvalidIntErr));
// Test with invalid LOAD arguments
resp = Run({"FT.SEARCH", "idx", "*", "LOAD", "@field1", "@field2"});
EXPECT_THAT(resp, ErrArg(kInvalidIntErr));
// Test with duplicate fields in LOAD
resp = Run({"FT.SEARCH", "idx", "*", "LOAD", "4", "@field1", "@field1", "@field2", "@field2"});
EXPECT_THAT(resp, IsMapWithSize("j1", IsMap("field1", "\"first\"", "field2", "\"second\"", "$",
R"({"field1":"first","field2":"second"})")));
// Test with LOAD exceeding maximum allowed count
resp = Run({"FT.SEARCH", "idx", "*", "LOAD", "100000000000000000000", "@field1", "@field2"});
EXPECT_THAT(resp, ErrArg(kInvalidIntErr));
// Test with invalid RETURN syntax (missing count)
resp = Run({"FT.SEARCH", "idx", "*", "RETURN", "@field1", "@field2"});
EXPECT_THAT(resp, ErrArg(kInvalidIntErr));
// Test with RETURN having duplicate fields
resp = Run({"FT.SEARCH", "idx", "*", "RETURN", "4", "field1", "field1", "field2", "field2"});
EXPECT_THAT(resp, IsMapWithSize("j1", IsMap("field1", "\"first\"", "field2", "\"second\"")));
// Test with RETURN exceeding maximum allowed count
resp = Run({"FT.SEARCH", "idx", "*", "RETURN", "100000000000000000000", "@field1", "@field2"});
EXPECT_THAT(resp, ErrArg(kInvalidIntErr));
// Test with NOCONTENT and LOAD
resp = Run({"FT.SEARCH", "idx", "*", "NOCONTENT", "LOAD", "2", "@field1", "@field2"});
EXPECT_THAT(resp, IsArray(IntArg(1), "j1"));
// Test with NOCONTENT and RETURN
resp = Run({"FT.SEARCH", "idx", "*", "NOCONTENT", "RETURN", "2", "@field1", "@field2"});
EXPECT_THAT(resp, IsArray(IntArg(1), "j1"));
}
} // namespace dfly