From e96a99a868c91ce9f2bb9cc3a71ec8b934caadc6 Mon Sep 17 00:00:00 2001 From: Stepan Bagritsevich <43710058+BagritsevichStepan@users.noreply.github.com> Date: Mon, 21 Oct 2024 18:35:23 +0200 Subject: [PATCH] fix(search_family): Temporary remove the error when a field name does not have the '@' sign at the beginning in the FT.AGGREGATE command (#3956) --- src/server/search/search_family.cc | 18 +++++++++++++----- src/server/search/search_family_test.cc | 16 ++++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/server/search/search_family.cc b/src/server/search/search_family.cc index 2a646c6878fd..b79cd9d547d2 100644 --- a/src/server/search/search_family.cc +++ b/src/server/search/search_family.cc @@ -238,12 +238,15 @@ std::string_view ParseField(CmdArgParser* parser) { return field; } -std::optional ParseFieldWithAtSign(CmdArgParser* parser) { +std::string_view ParseFieldWithAtSign(CmdArgParser* parser) { std::string_view field = parser->Next(); if (field.front() != '@') { - return std::nullopt; // if we expect @, but it's not there, return nullopt + // Temporary warning until we can throw an error + LOG(WARNING) << "bad arguments: Field name '" << field << "' should start with '@'. '@" << field + << "' is expected"; + } else { + field.remove_prefix(1); // remove leading @ } - field.remove_prefix(1); // remove leading @ return field; } @@ -273,12 +276,17 @@ optional ParseAggregatorParamsOrReply(CmdArgParser parser, vector fields(parser.Next()); for (string_view& field : fields) { auto parsed_field = ParseFieldWithAtSign(&parser); + + /* + TODO: Throw an error if the field has no '@' sign at the beginning + if (!parsed_field) { cntx->SendError(absl::StrCat("bad arguments for GROUPBY: Unknown property '", field, "'. Did you mean '@", field, "`?")); return nullopt; - } - field = parsed_field.value(); + } */ + + field = parsed_field; } vector reducers; diff --git a/src/server/search/search_family_test.cc b/src/server/search/search_family_test.cc index 87ac9d4e85f5..f6883096267c 100644 --- a/src/server/search/search_family_test.cc +++ b/src/server/search/search_family_test.cc @@ -894,6 +894,22 @@ TEST_F(SearchFamilyTest, JsonAggregateGroupBy) { EXPECT_THAT(resp, IsUnordArrayWithSize(IsMap("avg_price", "20"))); } +TEST_F(SearchFamilyTest, JsonAggregateGroupByWithoutAtSign) { + Run({"HSET", "h1", "group", "first", "value", "1"}); + Run({"HSET", "h2", "group", "second", "value", "2"}); + Run({"HSET", "h3", "group", "first", "value", "3"}); + + auto resp = + Run({"FT.CREATE", "index", "ON", "HASH", "SCHEMA", "group", "TAG", "value", "NUMERIC"}); + EXPECT_EQ(resp, "OK"); + + // TODO: Throw an error when no '@' is provided in the GROUPBY option + resp = Run({"FT.AGGREGATE", "index", "*", "GROUPBY", "1", "group", "REDUCE", "COUNT", "0", "AS", + "count"}); + EXPECT_THAT(resp, IsUnordArrayWithSize(IsMap("count", "2", "group", "first"), + IsMap("group", "second", "count", "1"))); +} + TEST_F(SearchFamilyTest, AggregateGroupByReduceSort) { for (size_t i = 0; i < 101; i++) { // 51 even, 50 odd Run({"hset", absl::StrCat("k", i), "even", (i % 2 == 0) ? "true" : "false", "value",