Skip to content

Commit

Permalink
refactor: remove double conversion from str to number to str in search (
Browse files Browse the repository at this point in the history 
#3591)

fixes #3581
  • Loading branch information
@BorysTheDev
BorysTheDev authored Aug 29, 2024
1 parent 0ee52c9 commit 79a80a0
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 43 deletions.
22 changes: 2 additions & 20 deletions src/core/search/lexer.lex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@
using dfly::search::Parser;
using namespace std;

Parser::symbol_type make_DOUBLE(string_view, const Parser::location_type& loc);
Parser::symbol_type make_UINT32(string_view, const Parser::location_type& loc);
Parser::symbol_type make_StringLit(string_view src, const Parser::location_type& loc);
Parser::symbol_type make_TagVal(string_view src, const Parser::location_type& loc);
%}
Expand Down Expand Up @@ -68,8 +66,8 @@ tag_val_char {term_char}|\\[,.<>{}\[\]\\\"\':;!@#$%^&*()\-+=~\/ ]
"AS" return Parser::make_AS (loc());
"EF_RUNTIME" return Parser::make_EF_RUNTIME (loc());

[0-9]{1,9} return make_UINT32(matched_view(), loc());
[+-]?(([0-9]*[.])?[0-9]+|inf) return make_DOUBLE(matched_view(), loc());
[0-9]{1,9} return Parser::make_UINT32(str(), loc());
[+-]?(([0-9]*[.])?[0-9]+|inf) return Parser::make_DOUBLE(str(), loc());

{dq}([^"]|{esc_seq})*{dq} return make_StringLit(matched_view(1, 1), loc());
{sq}([^']|{esc_seq})*{sq} return make_StringLit(matched_view(1, 1), loc());
Expand All @@ -83,22 +81,6 @@ tag_val_char {term_char}|\\[,.<>{}\[\]\\\"\':;!@#$%^&*()\-+=~\/ ]
<<EOF>> return Parser::make_YYEOF(loc());
%%

Parser::symbol_type make_UINT32 (string_view str, const Parser::location_type& loc) {
uint32_t val = 0;
if (!absl::SimpleAtoi(str, &val))
throw Parser::syntax_error (loc, "not an unsigned integer or out of range: " + string(str));

return Parser::make_UINT32(val, loc);
}

Parser::symbol_type make_DOUBLE (string_view str, const Parser::location_type& loc) {
double val = 0;
if (!absl::SimpleAtod(str, &val))
throw Parser::syntax_error (loc, "not a double or out of range: " + string(str));

return Parser::make_DOUBLE(val, loc);
}

Parser::symbol_type make_StringLit(string_view src, const Parser::location_type& loc) {
string res;
if (!absl::CUnescape(src, &res))
Expand Down
37 changes: 26 additions & 11 deletions src/core/search/parser.y
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@

using namespace std;

uint32_t toUint32(string_view src);
double toDouble(string_view src);

}

%parse-param { QueryDriver *driver }
Expand Down Expand Up @@ -74,8 +77,8 @@ using namespace std;
%right NOT_OP
%precedence LPAREN RPAREN

%token <double> DOUBLE "double"
%token <uint32_t> UINT32 "uint32"
%token <std::string> DOUBLE "double"
%token <std::string> UINT32 "uint32"
%nterm <double> generic_number
%nterm <bool> opt_lparen
%nterm <AstExpr> final_query filter search_expr search_unary_expr search_or_expr search_and_expr numeric_filter_expr
Expand All @@ -98,15 +101,15 @@ final_query:

knn_query:
LBRACKET KNN UINT32 FIELD TERM opt_knn_alias opt_ef_runtime RBRACKET
{ $$ = AstKnnNode($3, $4, BytesToFtVector($5), $6, $7); }
{ $$ = AstKnnNode(toUint32($3), $4, BytesToFtVector($5), $6, $7); }

opt_knn_alias:
AS TERM { $$ = std::move($2); }
| { $$ = std::string{}; }

opt_ef_runtime:
/* empty */ { $$ = std::nullopt; }
| EF_RUNTIME UINT32 { $$ = $2; }
| EF_RUNTIME UINT32 { $$ = toUint32($2); }

filter:
search_expr { $$ = std::move($1); }
Expand All @@ -129,12 +132,12 @@ search_unary_expr:
LPAREN search_expr RPAREN { $$ = std::move($2); }
| NOT_OP search_unary_expr { $$ = AstNegateNode(std::move($2)); }
| TERM { $$ = AstTermNode(std::move($1)); }
| UINT32 { $$ = AstTermNode(to_string($1)); }
| UINT32 { $$ = AstTermNode(std::move($1)); }
| FIELD COLON field_cond { $$ = AstFieldNode(std::move($1), std::move($3)); }

field_cond:
TERM { $$ = AstTermNode(std::move($1)); }
| UINT32 { $$ = AstTermNode(to_string($1)); }
| UINT32 { $$ = AstTermNode(std::move($1)); }
| NOT_OP field_cond { $$ = AstNegateNode(std::move($2)); }
| LPAREN field_cond_expr RPAREN { $$ = std::move($2); }
| LBRACKET numeric_filter_expr RBRACKET { $$ = std::move($2); }
Expand All @@ -144,8 +147,8 @@ numeric_filter_expr:
opt_lparen generic_number opt_lparen generic_number { $$ = AstRangeNode($2, $1, $4, $3); }

generic_number:
DOUBLE { $$ = $1; }
| UINT32 { $$ = $1; }
DOUBLE { $$ = toDouble($1); }
| UINT32 { $$ = toUint32($1); }

opt_lparen:
/* empty */ { $$ = false; }
Expand All @@ -168,16 +171,16 @@ field_unary_expr:
LPAREN field_cond_expr RPAREN { $$ = std::move($2); }
| NOT_OP field_unary_expr { $$ = AstNegateNode(std::move($2)); };
| TERM { $$ = AstTermNode(std::move($1)); }
| UINT32 { $$ = AstTermNode(to_string($1)); }
| UINT32 { $$ = AstTermNode(std::move($1)); }

tag_list:
tag_list_element { $$ = AstTagsNode(std::move($1)); }
| tag_list OR_OP tag_list_element { $$ = AstTagsNode(std::move($1), std::move($3)); }

tag_list_element:
TERM { $$ = std::move($1); }
| UINT32 { $$ = to_string($1); }
| DOUBLE { $$ = to_string($1); }
| UINT32 { $$ = std::move($1); }
| DOUBLE { $$ = std::move($1); }
| TAG_VAL { $$ = std::move($1); }


Expand All @@ -188,3 +191,15 @@ dfly::search::Parser::error(const location_type& l, const string& m)
{
driver->Error(l, m);
}

std::uint32_t toUint32(string_view str) {
uint32_t val = 0;
absl::SimpleAtoi(str, &val); // no need to check the result because str is parsed by regex
return val;
}

double toDouble(string_view str) {
double val = 0;
absl::SimpleAtod(str, &val); // no need to check the result because str is parsed by regex
return val;
}
5 changes: 2 additions & 3 deletions src/core/search/scanner.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ class Scanner : public Lexer {
}

Parser::symbol_type ParseParam(std::string_view name, const Parser::location_type& loc) {
if (!name.empty())
name.remove_prefix(1);
name.remove_prefix(1); // drop $ symbol

std::string_view str = (*params_)[name];
if (str.empty())
Expand All @@ -50,7 +49,7 @@ class Scanner : public Lexer {
if (!absl::SimpleAtoi(str, &val))
return Parser::make_TERM(std::string{str}, loc);

return Parser::make_UINT32(val, loc);
return Parser::make_UINT32(std::string{str}, loc);
}

private:
Expand Down
16 changes: 7 additions & 9 deletions src/core/search/search_parser_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ TEST_F(SearchParserTest, Scanner) {

NEXT_TOK(TOK_LPAREN);
NEXT_EQ(TOK_TERM, string, "5a");
NEXT_EQ(TOK_UINT32, uint32_t, 6);
NEXT_EQ(TOK_UINT32, string, "6");
NEXT_TOK(TOK_RPAREN);

SetInput(R"( "hello\"world" )");
Expand Down Expand Up @@ -155,10 +155,8 @@ TEST_F(SearchParserTest, Scanner) {
NEXT_EQ(TOK_TERM, string, "почтальон");
NEXT_EQ(TOK_TERM, string, "Печкин");

double d;
ASSERT_TRUE(absl::SimpleAtod("33.3", &d));
SetInput("33.3");
NEXT_EQ(TOK_DOUBLE, double, d);
NEXT_EQ(TOK_DOUBLE, string, "33.3");
}

TEST_F(SearchParserTest, Parse) {
Expand All @@ -184,7 +182,7 @@ TEST_F(SearchParserTest, ParseParams) {

SetInput("$name $k");
NEXT_EQ(TOK_TERM, string, "alex");
NEXT_EQ(TOK_UINT32, uint32_t, 10);
NEXT_EQ(TOK_UINT32, string, "10");
}

TEST_F(SearchParserTest, Quotes) {
Expand All @@ -197,8 +195,8 @@ TEST_F(SearchParserTest, Quotes) {

TEST_F(SearchParserTest, Numeric) {
SetInput("11 123123123123 '22'");
NEXT_EQ(TOK_UINT32, uint32_t, 11);
NEXT_EQ(TOK_DOUBLE, double, 123123123123.0);
NEXT_EQ(TOK_UINT32, string, "11");
NEXT_EQ(TOK_DOUBLE, string, "123123123123");
NEXT_EQ(TOK_TERM, string, "22");
}

Expand All @@ -216,15 +214,15 @@ TEST_F(SearchParserTest, KNNfull) {
NEXT_TOK(TOK_LBRACKET);

NEXT_TOK(TOK_KNN);
NEXT_EQ(TOK_UINT32, uint32_t, 1);
NEXT_EQ(TOK_UINT32, string, "1");
NEXT_TOK(TOK_FIELD);
NEXT_TOK(TOK_TERM);

NEXT_TOK(TOK_AS);
NEXT_EQ(TOK_TERM, string, "vec_sort");

NEXT_TOK(TOK_EF_RUNTIME);
NEXT_EQ(TOK_UINT32, uint32_t, 15);
NEXT_EQ(TOK_UINT32, string, "15");

NEXT_TOK(TOK_RBRACKET);
}
Expand Down

0 comments on commit 79a80a0

Please sign in to comment.