fix(search-parser): fix AND, OR, NOT parsing issue (#1838)

Signed-off-by: Borys <[email protected]>
dragonflydb · Sep 10, 2023 · fcebb6a · fcebb6a
1 parent ba8adf7
commit fcebb6a
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 20 deletions.
diff --git a/src/core/search/parser.y b/src/core/search/parser.y
@@ -66,12 +66,14 @@ using namespace std;
 %token <std::string> TERM "term" PARAM "param" FIELD "field"
 
 %precedence TERM
-%left OR_OP AND_OP
+%left OR_OP
+%left AND_OP
 %right NOT_OP
 %precedence LPAREN RPAREN
 
 %token <int64_t> INT64 "int64"
-%nterm <AstExpr> final_query filter search_expr field_cond field_cond_expr tag_list
+%nterm <AstExpr> final_query filter search_expr search_unary_expr search_or_expr search_and_expr
+%nterm <AstExpr> field_cond field_cond_expr field_unary_expr field_or_expr field_and_expr tag_list
 
 %printer { yyo << $$; } <*>;
 
@@ -84,20 +86,29 @@ final_query:
       { driver->Set(AstKnnNode(move($1), $5, $6, BytesToFtVector($7))); }
 
 filter:
-  search_expr { $$ = move($1); }
-  | STAR { $$ = AstStarNode(); }
+  search_expr               { $$ = move($1); }
+  | STAR                    { $$ = AstStarNode(); }
 
 search_expr:
- LPAREN search_expr RPAREN              { $$ = move($2); }
- | search_expr search_expr %prec AND_OP { $$ = AstLogicalNode(move($1), move($2), AstLogicalNode::AND); }
- | search_expr OR_OP search_expr        { $$ = AstLogicalNode(move($1), move($3), AstLogicalNode::OR); }
- | NOT_OP search_expr                   { $$ = AstNegateNode(move($2)); }
- | TERM                                 { $$ = AstTermNode(move($1)); }
- | INT64                                { $$ = AstTermNode(to_string($1)); }
- | FIELD COLON field_cond               { $$ = AstFieldNode(move($1), move($3)); }
-
-// field_cond doesn't implicitly turn into field_cond_expr that can contain multi-term and/or expressions,
-// as those can only be contained inside parentheses
+  search_unary_expr         { $$ = move($1); }
+  | search_and_expr         { $$ = move($1); }
+  | search_or_expr          { $$ = move($1); }
+
+search_and_expr:
+  search_unary_expr search_unary_expr %prec AND_OP { $$ = AstLogicalNode(move($1), move($2), AstLogicalNode::AND); }
+  | search_and_expr search_unary_expr %prec AND_OP { $$ = AstLogicalNode(move($1), move($2), AstLogicalNode::AND); }
+
+search_or_expr:
+  search_expr OR_OP search_and_expr                { $$ = AstLogicalNode(move($1), move($3), AstLogicalNode::OR); }
+  | search_expr OR_OP search_unary_expr            { $$ = AstLogicalNode(move($1), move($3), AstLogicalNode::OR); }
+
+search_unary_expr:
+  LPAREN search_expr RPAREN           { $$ = move($2); }
+  | NOT_OP search_unary_expr          { $$ = AstNegateNode(move($2)); }
+  | TERM                              { $$ = AstTermNode(move($1)); }
+  | INT64                             { $$ = AstTermNode(to_string($1)); }
+  | FIELD COLON field_cond            { $$ = AstFieldNode(move($1), move($3)); }
+
 field_cond:
   TERM                                  { $$ = AstTermNode(move($1)); }
   | INT64                               { $$ = AstTermNode(to_string($1)); }
@@ -107,12 +118,23 @@ field_cond:
   | LCURLBR tag_list RCURLBR            { $$ = move($2); }
 
 field_cond_expr:
-  LPAREN field_cond_expr RPAREN                   { $$ = move($2); }
-  | field_cond_expr field_cond_expr %prec AND_OP  { $$ = AstLogicalNode(move($1), move($2), AstLogicalNode::AND); }
-  | field_cond_expr OR_OP field_cond_expr         { $$ = AstLogicalNode(move($1), move($3), AstLogicalNode::OR); }
-  | NOT_OP field_cond_expr                        { $$ = AstNegateNode(move($2)); };
-  | TERM                                          { $$ = AstTermNode(move($1)); }
-  | INT64                                         { $$ = AstTermNode(to_string($1)); }
+  field_unary_expr                       { $$ = move($1); }
+  | field_and_expr                       { $$ = move($1); }
+  | field_or_expr                        { $$ = move($1); }
+
+field_and_expr:
+  field_unary_expr field_unary_expr %prec AND_OP  { $$ = AstLogicalNode(move($1), move($2), AstLogicalNode::AND); }
+  | field_and_expr field_unary_expr %prec AND_OP  { $$ = AstLogicalNode(move($1), move($2), AstLogicalNode::AND); }
+
+field_or_expr:
+  field_cond_expr OR_OP field_unary_expr          { $$ = AstLogicalNode(move($1), move($3), AstLogicalNode::OR); }
+  | field_cond_expr OR_OP field_and_expr          { $$ = AstLogicalNode(move($1), move($3), AstLogicalNode::OR); }
+
+field_unary_expr:
+  LPAREN field_cond_expr RPAREN                  { $$ = move($2); }
+  | NOT_OP field_unary_expr                      { $$ = AstNegateNode(move($2)); };
+  | TERM                                         { $$ = AstTermNode(move($1)); }
+  | INT64                                        { $$ = AstTermNode(to_string($1)); }
 
 tag_list:
   TERM                       { $$ = AstTagsNode(move($1)); }

diff --git a/src/core/search/search_test.cc b/src/core/search/search_test.cc
@@ -223,6 +223,15 @@ TEST_F(SearchTest, CheckNotPriority) {
 
     EXPECT_TRUE(Check()) << GetError();
   }
+
+  for (auto expr : {"-bar far|-foo tam"}) {
+    PrepareQuery(expr);
+
+    ExpectAll("far baz", "far foo", "bar tam");
+    ExpectNone("bar far", "foo tam", "bar foo", "far bar foo");
+
+    EXPECT_TRUE(Check()) << GetError();
+  }
 }
 
 TEST_F(SearchTest, CheckParenthesisPriority) {
@@ -294,6 +303,14 @@ TEST_F(SearchTest, CheckExprInField) {
     ExpectNone(Map{{"f1", "a b d"}, {"f2", "c"}}, Map{{"f1", "a b w"}, {"f2", "a"}},
                Map{{"f1", "a w"}, {"f2", "c"}});
 
+    EXPECT_TRUE(Check()) << GetError();
+  }
+  {
+    PrepareQuery("@f1:(-a c|-b d)");
+
+    ExpectAll(Map{{"f1", "c"}}, Map{{"f1", "d"}});
+    ExpectNone(Map{{"f1", "a"}}, Map{{"f1", "b"}});
+
     EXPECT_TRUE(Check()) << GetError();
   }
 }