Skip to content

Commit be4d4d4

Browse files
BorysTheDevBorysTheDev
authored
feat: add FT.CONFIG command (#5855)
* feat: add FT.CONFIG command
1 parent 79420c7 commit be4d4d4

File tree

6 files changed

+200
-18
lines changed

6 files changed

+200
-18
lines changed

‎src/server/config_registry.cc‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,18 +43,23 @@ auto ConfigRegistry::Set(string_view config_name, string_view value) -> SetResul
4343
return success ? SetResult::OK : SetResult::INVALID;
4444
}
4545

46-
optional<string> ConfigRegistry::Get(string_view config_name) {
46+
absl::CommandLineFlag* ConfigRegistry::GetFlag(std::string_view config_name) {
4747
string name = NormalizeConfigName(config_name);
4848

4949
{
5050
util::fb2::LockGuard lk(mu_);
5151
if (!registry_.contains(name))
52-
return nullopt;
52+
return nullptr;
5353
}
5454

5555
absl::CommandLineFlag* flag = absl::FindCommandLineFlag(name);
5656
CHECK(flag);
57-
return flag->CurrentValue();
57+
return flag;
58+
}
59+
60+
optional<string> ConfigRegistry::Get(string_view config_name) {
61+
absl::CommandLineFlag* flag = GetFlag(config_name);
62+
return flag ? flag->CurrentValue() : optional<string>();
5863
}
5964

6065
void ConfigRegistry::Reset() {

‎src/server/config_registry.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ class ConfigRegistry {
5353

5454
std::optional<std::string> Get(std::string_view config_name) ABSL_LOCKS_EXCLUDED(mu_);
5555

56+
absl::CommandLineFlag* GetFlag(std::string_view config_name) ABSL_LOCKS_EXCLUDED(mu_);
57+
5658
void Reset();
5759

5860
std::vector<std::string> List(std::string_view glob) const ABSL_LOCKS_EXCLUDED(mu_);

‎src/server/main_service.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -970,6 +970,7 @@ void Service::Init(util::AcceptServer* acceptor, std::vector<facade::Listener*>
970970
config_registry.RegisterMutable("timeout");
971971
config_registry.RegisterMutable("send_timeout");
972972
config_registry.RegisterMutable("managed_service_info");
973+
config_registry.RegisterMutable("MAXSEARCHRESULTS");
973974

974975
config_registry.RegisterMutable(
975976
"notify_keyspace_events", [pool = &pp_](const absl::CommandLineFlag& flag) {

‎src/server/search/search_family.cc‎

Lines changed: 119 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "facade/reply_builder.h"
2626
#include "server/acl/acl_commands_def.h"
2727
#include "server/command_registry.h"
28+
#include "server/config_registry.h"
2829
#include "server/conn_context.h"
2930
#include "server/container_utils.h"
3031
#include "server/engine_shard_set.h"
@@ -35,12 +36,15 @@
3536

3637
ABSL_FLAG(bool, search_reject_legacy_field, true, "FT.AGGREGATE: Reject legacy field names.");
3738

39+
ABSL_FLAG(size_t, MAXSEARCHRESULTS, 1000000, "Maximum number of results from ft.search command");
3840
namespace dfly {
3941

4042
using namespace std;
4143
using namespace facade;
4244

4345
namespace {
46+
// we use it to find which flags are belong to search
47+
const std::string kCurrentFile = std::filesystem::path(__FILE__).filename().string();
4448

4549
using nonstd::make_unexpected;
4650

@@ -385,11 +389,16 @@ search::QueryParams ParseQueryParams(CmdArgParser* parser) {
385389
ParseResult<SearchParams> ParseSearchParams(CmdArgParser* parser) {
386390
SearchParams params;
387391

392+
const size_t max_results = absl::GetFlag(FLAGS_MAXSEARCHRESULTS);
393+
388394
while (parser->HasNext()) {
389395
// [LIMIT offset total]
390396
if (parser->Check("LIMIT")) {
391397
params.limit_offset = parser->Next<size_t>();
392398
params.limit_total = parser->Next<size_t>();
399+
if (params.limit_total > max_results) {
400+
return CreateSyntaxError(absl::StrFormat("LIMIT exceeds maximum of %d", max_results));
401+
}
393402
} else if (parser->Check("LOAD")) {
394403
if (params.return_fields) {
395404
return CreateSyntaxError("LOAD cannot be applied after RETURN"sv);
@@ -418,6 +427,8 @@ ParseResult<SearchParams> ParseSearchParams(CmdArgParser* parser) {
418427
}
419428
}
420429

430+
params.limit_total = std::min(params.limit_total, max_results);
431+
421432
return params;
422433
}
423434

@@ -1649,6 +1660,97 @@ void SearchFamily::FtSynDump(CmdArgList args, const CommandContext& cmd_cntx) {
16491660
}
16501661
}
16511662

1663+
void FtConfigHelp(CmdArgParser* parser, RedisReplyBuilder* rb) {
1664+
string_view param = parser->Next();
1665+
1666+
vector<string> names = config_registry.List(param);
1667+
vector<absl::CommandLineFlag*> res;
1668+
1669+
for (const auto& name : names) {
1670+
auto* flag = config_registry.GetFlag(name);
1671+
DCHECK(flag);
1672+
if (flag && flag->Filename().find(kCurrentFile) != std::string::npos) {
1673+
res.push_back(flag);
1674+
}
1675+
}
1676+
1677+
rb->StartArray(res.size());
1678+
for (const auto& flag : res) {
1679+
rb->StartArray(5);
1680+
rb->SendBulkString(flag->Name());
1681+
rb->SendBulkString("Description"sv);
1682+
rb->SendBulkString(flag->Help());
1683+
rb->SendBulkString("Value"sv);
1684+
rb->SendBulkString(flag->CurrentValue());
1685+
}
1686+
}
1687+
1688+
void FtConfigGet(CmdArgParser* parser, RedisReplyBuilder* rb) {
1689+
string_view param = parser->Next();
1690+
vector<string> names = config_registry.List(param);
1691+
1692+
vector<string> res;
1693+
1694+
for (const auto& name : names) {
1695+
auto* flag = config_registry.GetFlag(name);
1696+
DCHECK(flag);
1697+
if (flag && flag->Filename().find(kCurrentFile) != std::string::npos) {
1698+
res.push_back(name);
1699+
res.push_back(flag->CurrentValue());
1700+
}
1701+
}
1702+
return rb->SendBulkStrArr(res, RedisReplyBuilder::MAP);
1703+
}
1704+
1705+
void FtConfigSet(CmdArgParser* parser, RedisReplyBuilder* rb) {
1706+
auto [param, value] = parser->Next<string_view, string_view>();
1707+
1708+
if (!parser->Finalize()) {
1709+
rb->SendError(parser->TakeError().MakeReply());
1710+
return;
1711+
}
1712+
1713+
vector<string> names = config_registry.List(param);
1714+
if (names.size() != 1 ||
1715+
config_registry.GetFlag(names[0])->Filename().find(kCurrentFile) == std::string::npos) {
1716+
return rb->SendError("Invalid option name");
1717+
}
1718+
1719+
ConfigRegistry::SetResult result = config_registry.Set(param, value);
1720+
1721+
const char kErrPrefix[] = "FT.CONFIG SET failed (possibly related to argument '";
1722+
switch (result) {
1723+
case ConfigRegistry::SetResult::OK:
1724+
return rb->SendOk();
1725+
case ConfigRegistry::SetResult::UNKNOWN:
1726+
return rb->SendError(
1727+
absl::StrCat("Unknown option or number of arguments for CONFIG SET - '", param, "'"),
1728+
kConfigErrType);
1729+
1730+
case ConfigRegistry::SetResult::READONLY:
1731+
return rb->SendError(absl::StrCat(kErrPrefix, param, "') - can't set immutable config"),
1732+
kConfigErrType);
1733+
1734+
case ConfigRegistry::SetResult::INVALID:
1735+
return rb->SendError(absl::StrCat(kErrPrefix, param, "') - argument can not be set"),
1736+
kConfigErrType);
1737+
}
1738+
ABSL_UNREACHABLE();
1739+
}
1740+
1741+
void SearchFamily::FtConfig(CmdArgList args, const CommandContext& cmd_cntx) {
1742+
CmdArgParser parser{args};
1743+
auto* rb = static_cast<RedisReplyBuilder*>(cmd_cntx.rb);
1744+
1745+
auto func = parser.MapNext("GET", &FtConfigGet, "SET", &FtConfigSet, "HELP", &FtConfigHelp);
1746+
1747+
if (auto err = parser.TakeError(); err) {
1748+
rb->SendError("Unknown subcommand");
1749+
return;
1750+
}
1751+
func(&parser, rb);
1752+
}
1753+
16521754
void SearchFamily::FtSynUpdate(CmdArgList args, const CommandContext& cmd_cntx) {
16531755
facade::CmdArgParser parser{args};
16541756
auto [index_name, group_id] = parser.Next<string_view, string>();
@@ -1711,21 +1813,23 @@ void SearchFamily::Register(CommandRegistry* registry) {
17111813
CO::NO_KEY_TRANSACTIONAL | CO::NO_KEY_TX_SPAN_ALL | CO::NO_AUTOJOURNAL | CO::IDEMPOTENT;
17121814

17131815
registry->StartFamily();
1714-
*registry << CI{"FT.CREATE", CO::WRITE | CO::GLOBAL_TRANS, -2, 0, 0, acl::FT_SEARCH}.HFUNC(
1715-
FtCreate)
1716-
<< CI{"FT.ALTER", CO::WRITE | CO::GLOBAL_TRANS, -3, 0, 0, acl::FT_SEARCH}.HFUNC(FtAlter)
1717-
<< CI{"FT.DROPINDEX", CO::WRITE | CO::GLOBAL_TRANS, -2, 0, 0, acl::FT_SEARCH}.HFUNC(
1718-
FtDropIndex)
1719-
<< CI{"FT.INFO", kReadOnlyMask, 2, 0, 0, acl::FT_SEARCH}.HFUNC(FtInfo)
1720-
// Underscore same as in RediSearch because it's "temporary" (long time already)
1721-
<< CI{"FT._LIST", kReadOnlyMask, 1, 0, 0, acl::FT_SEARCH}.HFUNC(FtList)
1722-
<< CI{"FT.SEARCH", kReadOnlyMask, -3, 0, 0, acl::FT_SEARCH}.HFUNC(FtSearch)
1723-
<< CI{"FT.AGGREGATE", kReadOnlyMask, -3, 0, 0, acl::FT_SEARCH}.HFUNC(FtAggregate)
1724-
<< CI{"FT.PROFILE", kReadOnlyMask, -4, 0, 0, acl::FT_SEARCH}.HFUNC(FtProfile)
1725-
<< CI{"FT.TAGVALS", kReadOnlyMask, 3, 0, 0, acl::FT_SEARCH}.HFUNC(FtTagVals)
1726-
<< CI{"FT.SYNDUMP", kReadOnlyMask, 2, 0, 0, acl::FT_SEARCH}.HFUNC(FtSynDump)
1727-
<< CI{"FT.SYNUPDATE", CO::WRITE | CO::GLOBAL_TRANS, -4, 0, 0, acl::FT_SEARCH}.HFUNC(
1728-
FtSynUpdate);
1816+
*registry
1817+
<< CI{"FT.CREATE", CO::WRITE | CO::GLOBAL_TRANS, -2, 0, 0, acl::FT_SEARCH}.HFUNC(FtCreate)
1818+
<< CI{"FT.ALTER", CO::WRITE | CO::GLOBAL_TRANS, -3, 0, 0, acl::FT_SEARCH}.HFUNC(FtAlter)
1819+
<< CI{"FT.DROPINDEX", CO::WRITE | CO::GLOBAL_TRANS, -2, 0, 0, acl::FT_SEARCH}.HFUNC(
1820+
FtDropIndex)
1821+
<< CI{"FT.INFO", kReadOnlyMask, 2, 0, 0, acl::FT_SEARCH}.HFUNC(FtInfo)
1822+
<< CI{"FT.CONFIG", CO::ADMIN | CO::LOADING | CO::DANGEROUS, -3, 0, 0, acl::FT_SEARCH}.HFUNC(
1823+
FtConfig)
1824+
// Underscore same as in RediSearch because it's "temporary" (long time already)
1825+
<< CI{"FT._LIST", kReadOnlyMask, 1, 0, 0, acl::FT_SEARCH}.HFUNC(FtList)
1826+
<< CI{"FT.SEARCH", kReadOnlyMask, -3, 0, 0, acl::FT_SEARCH}.HFUNC(FtSearch)
1827+
<< CI{"FT.AGGREGATE", kReadOnlyMask, -3, 0, 0, acl::FT_SEARCH}.HFUNC(FtAggregate)
1828+
<< CI{"FT.PROFILE", kReadOnlyMask, -4, 0, 0, acl::FT_SEARCH}.HFUNC(FtProfile)
1829+
<< CI{"FT.TAGVALS", kReadOnlyMask, 3, 0, 0, acl::FT_SEARCH}.HFUNC(FtTagVals)
1830+
<< CI{"FT.SYNDUMP", kReadOnlyMask, 2, 0, 0, acl::FT_SEARCH}.HFUNC(FtSynDump)
1831+
<< CI{"FT.SYNUPDATE", CO::WRITE | CO::GLOBAL_TRANS, -4, 0, 0, acl::FT_SEARCH}.HFUNC(
1832+
FtSynUpdate);
17291833
}
17301834

17311835
} // namespace dfly

‎src/server/search/search_family.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ class SearchFamily {
3131
static void FtTagVals(CmdArgList args, const CommandContext& cmd_cntx);
3232
static void FtSynDump(CmdArgList args, const CommandContext& cmd_cntx);
3333
static void FtSynUpdate(CmdArgList args, const CommandContext& cmd_cntx);
34+
static void FtConfig(CmdArgList args, const CommandContext& cmd_cntx);
3435

3536
public:
3637
static void Register(CommandRegistry* registry);

‎src/server/search/search_family_test.cc‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3246,4 +3246,73 @@ TEST_F(SearchFamilyTest, NumericFilter) {
32463246
Run({"FLUSHALL"});
32473247
}
32483248

3249+
TEST_F(SearchFamilyTest, MAXSEARCHRESULTS) {
3250+
EXPECT_EQ(Run({"HSET", "s1", "phrase", "hello world"}), 1);
3251+
EXPECT_EQ(Run({"HSET", "s2", "phrase", "hello simple world"}), 1);
3252+
EXPECT_EQ(Run({"HSET", "s3", "phrase", "hello somewhat less simple world"}), 1);
3253+
EXPECT_EQ(Run({"FT.CREATE", "memes", "SCHEMA", "phrase", "TEXT"}), "OK");
3254+
3255+
auto resp = Run({"FT.CONFIG", "GET", "MAXSEARCHRESULTS"});
3256+
EXPECT_THAT(resp, IsArray("MAXSEARCHRESULTS", "1000000"));
3257+
3258+
resp = Run({"FT.SEARCH", "memes", "@phrase:(hello world)", "NOCONTENT"});
3259+
EXPECT_THAT(resp, RespElementsAre(IntArg(3), _, _, _));
3260+
3261+
resp = Run({"FT.CONFIG", "SET", "MAXSEARCHRESULTS", "1"});
3262+
EXPECT_EQ(resp, "OK");
3263+
3264+
resp = Run({"FT.SEARCH", "memes", "@phrase:(hello world)", "NOCONTENT"});
3265+
EXPECT_THAT(resp, RespElementsAre(IntArg(3), _));
3266+
3267+
resp = Run({"FT.SEARCH", "memes", "@phrase:(hello world)", "NOCONTENT", "LIMIT", "0", "1"});
3268+
EXPECT_THAT(resp, RespElementsAre(IntArg(3), _));
3269+
3270+
resp = Run({"FT.SEARCH", "memes", "@phrase:(hello world)", "NOCONTENT", "LIMIT", "0", "3"});
3271+
EXPECT_THAT(resp, ErrArg("LIMIT exceeds maximum of 1"));
3272+
3273+
resp = Run({"FT.CONFIG", "GET", "MAXSEARCHRESULTS"});
3274+
EXPECT_THAT(resp, IsArray("MAXSEARCHRESULTS", "1"));
3275+
3276+
resp = Run({"FT.CONFIG", "HELP", "MAXSEARCHRESULTS"});
3277+
EXPECT_THAT(resp, IsArray("MAXSEARCHRESULTS", "Description",
3278+
"Maximum number of results from ft.search command", "Value", "1"));
3279+
3280+
resp = Run({"FT.CONFIG", "GET", "*"});
3281+
EXPECT_THAT(resp, IsArray("MAXSEARCHRESULTS", "1"));
3282+
3283+
resp = Run({"FT.CONFIG", "HELP", "*"});
3284+
EXPECT_THAT(resp, IsArray("MAXSEARCHRESULTS", "Description",
3285+
"Maximum number of results from ft.search command", "Value", "1"));
3286+
3287+
// restore normal value for other tests
3288+
Run({"FT.CONFIG", "SET", "MAXSEARCHRESULTS", "1000000"});
3289+
}
3290+
3291+
TEST_F(SearchFamilyTest, InvalidConfigOptions) {
3292+
// Test with an invalid argument
3293+
auto resp = Run({"FT.CONFIG", "INVALIDARG", "INVLIDARG"});
3294+
EXPECT_THAT(resp, ErrArg("Unknown subcommand"));
3295+
3296+
// Test with an invalid argument
3297+
resp = Run({"FT.CONFIG", "GET", "INVALIDARG"});
3298+
EXPECT_THAT(resp, IsArray());
3299+
3300+
// Test with an invalid argument
3301+
resp = Run({"FT.CONFIG", "SET", "INVALIDARG"});
3302+
EXPECT_THAT(resp, ErrArg(kSyntaxErr));
3303+
3304+
// Test with an invalid argument
3305+
resp = Run({"FT.CONFIG", "SET", "INVALIDARG", "5"});
3306+
EXPECT_THAT(resp, ErrArg("Invalid option"));
3307+
3308+
// Test with an invalid value
3309+
resp = Run({"FT.CONFIG", "SET", "MAXSEARCHRESULTS", "not_a_number"});
3310+
EXPECT_THAT(resp, ErrArg("ERR FT.CONFIG SET failed (possibly related to argument "
3311+
"'MAXSEARCHRESULTS') - argument can not be set"));
3312+
3313+
// Test with an invalid argument
3314+
resp = Run({"FT.CONFIG", "HELP", "INVALIDARG"});
3315+
EXPECT_THAT(resp, IsArray());
3316+
}
3317+
32493318
} // namespace dfly

0 commit comments

Comments
 (0)