fix: fixes

Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
dragonflydb · May 3, 2023 · 0f68aab · 0f68aab
1 parent c1248f5
commit 0f68aab
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 22 deletions.
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -30,7 +30,7 @@ function(gen_bison name)
   ADD_CUSTOM_COMMAND(
            OUTPUT ${full_path_cc}
            COMMAND mkdir -p ${gen_dir}
-           COMMAND bison --language=c++ -o ${gen_dir}/${name}.cc ${name}.y
+           COMMAND bison --language=c++ -o ${gen_dir}/${name}.cc ${name}.y -Wconflicts-sr -Wcounterexamples
            DEPENDS ${_in}
            WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
            COMMENT "Generating parser from ${name}.y" VERBATIM)

diff --git a/src/core/search/ast_expr.cc b/src/core/search/ast_expr.cc
@@ -27,12 +27,12 @@ string AstNegateNode::Debug() const {
 }
 
 bool AstLogicalNode::Check(string_view input) const {
-  return disjunction_ ? (l_->Check(input) || r_->Check(input))
-                      : (l_->Check(input) && r_->Check(input));
+  return op_ == kOr ? (l_->Check(input) || r_->Check(input))
+                    : (l_->Check(input) && r_->Check(input));
 }
 
 string AstLogicalNode::Debug() const {
-  string op = disjunction_ ? "or" : "and";
+  string op = op_ == kOr ? "or" : "and";
   return op + "{" + l_->Debug() + "," + r_->Debug() + "}";
 }
 

diff --git a/src/core/search/ast_expr.h b/src/core/search/ast_expr.h
@@ -30,12 +30,11 @@ using NodePtr = std::shared_ptr<AstNode>;
 using AstExpr = NodePtr;
 
 template <typename T, typename... Ts> AstExpr MakeExpr(Ts... ts) {
-  auto ptr = std::make_shared<T>(std::forward<Ts>(ts)...);
-  return AstExpr{std::reinterpret_pointer_cast<AstNode>(move(ptr))};
+  return std::make_shared<T>(std::forward<Ts>(ts)...);
 }
 
 // AST term node, matches only if input contains term.
-class AstTermNode : AstNode {
+class AstTermNode : public AstNode {
  public:
   AstTermNode(std::string term) : term_{move(term)} {
   }
@@ -47,11 +46,11 @@ class AstTermNode : AstNode {
 };
 
 // Ast negation node, matches only if its sub node didn't match.
-class AstNegateNode : AstNode {
+class AstNegateNode : public AstNode {
  public:
   AstNegateNode(NodePtr node) : node_{node} {
   }
-  virtual bool Check(std::string_view needle) const;
+  virtual bool Check(std::string_view input) const;
   virtual std::string Debug() const;
 
  private:
@@ -60,16 +59,21 @@ class AstNegateNode : AstNode {
 
 // Ast logical operation node, matches only if sub nodes match
 // in respect to logical operation (and/or).
-class AstLogicalNode : AstNode {
+class AstLogicalNode : public AstNode {
  public:
-  AstLogicalNode(NodePtr l, NodePtr r, bool disjunction) : l_{l}, r_{r}, disjunction_{disjunction} {
+  enum Op {
+    kAnd,
+    kOr,
+  };
+
+  AstLogicalNode(NodePtr l, NodePtr r, Op op) : l_{l}, r_{r}, op_{op} {
   }
-  virtual bool Check(std::string_view needle) const;
+  virtual bool Check(std::string_view input) const;
   virtual std::string Debug() const;
 
  private:
   NodePtr l_, r_;
-  bool disjunction_;
+  Op op_;
 };
 
 }  // namespace search

diff --git a/src/core/search/lexer.lex b/src/core/search/lexer.lex
@@ -58,7 +58,7 @@ term_char [_]|\w
 "=>"           return Parser::make_ARROW (loc());
 "["            return Parser::make_LBRACKET (loc());
 "]"            return Parser::make_RBRACKET (loc());
-"|"            return Parser::make_VBAR (loc());
+"|"            return Parser::make_OR_OP (loc());
 
 -?[0-9]+       return make_INT64(matched_view(), loc());
 

diff --git a/src/core/search/parser.y b/src/core/search/parser.y
@@ -48,15 +48,20 @@ using namespace std;
   COLON    ":"
   LBRACKET "["
   RBRACKET "]"
-  VBAR     "|"
+  OR_OP    "|"
 ;
 
-%precedence NOT_OP
+%token AND_OP
 
 // Needed 0 at the end to satisfy bison 3.5.1
 %token YYEOF 0
 %token <std::string> TERM "term" PARAM "param" FIELD "field"
 
+%precedence TERM
+%right NOT_OP
+%left OR_OP AND_OP
+%precedence LPAREN RPAREN
+
 %token <int64_t> INT64 "int64"
 %nterm <AstExpr> final_query filter search_expr field_filter field_cond range_value term_list opt_neg_term
 
@@ -69,12 +74,11 @@ final_query:
 
 filter:
   search_expr { $$ = $1; }
-  | filter search_expr { $$ = MakeExpr<AstLogicalNode>($1, $2, false);};
-  | filter VBAR search_expr { $$ = MakeExpr<AstLogicalNode>($1, $3, true); }
-
 
 search_expr:
  LPAREN search_expr RPAREN { $$ = $2; }
+ | search_expr search_expr %prec AND_OP { $$ = MakeExpr<AstLogicalNode>($1, $2, AstLogicalNode::kAnd); };
+ | search_expr OR_OP search_expr { $$ = MakeExpr<AstLogicalNode>($1, $3, AstLogicalNode::kOr); }
  | NOT_OP search_expr { $$ = MakeExpr<AstNegateNode>($2); };
  | TERM { $$ = MakeExpr<AstTermNode>($1); }
  | field_filter;

diff --git a/src/core/search/search_parser_test.cc b/src/core/search/search_parser_test.cc
@@ -131,22 +131,45 @@ TEST_F(SearchParserTest, MatchNotTerm) {
   EXPECT_TRUE(Check("definitelyright"));
 }
 
-TEST_F(SearchParserTest, MatchConjunctionTerm) {
+TEST_F(SearchParserTest, MatchConjunction) {
   ParseExpr("foo bar");
 
   EXPECT_TRUE(Check("foo bar"));
   EXPECT_TRUE(Check("bar foo"));
   EXPECT_TRUE(Check("foo bar and more"));
   EXPECT_TRUE(Check("more bar and foo"));
 
+  EXPECT_FALSE(Check("wrong"));
   EXPECT_FALSE(Check("foo"));
   EXPECT_FALSE(Check("bar"));
   EXPECT_FALSE(Check("foob"));
   EXPECT_FALSE(Check("foo and more stuff"));
   EXPECT_FALSE(Check("but not bar"));
 }
 
-TEST_F(SearchParserTest, MatchDisjunctionTerm) {
+TEST_F(SearchParserTest, MatchConjunctionNot) {
+  ParseExpr("foo -bar");
+
+  EXPECT_TRUE(Check("foo"));
+  EXPECT_TRUE(Check("foo rab"));
+
+  EXPECT_FALSE(Check("wrong"));
+  EXPECT_FALSE(Check("bar"));
+  EXPECT_FALSE(Check("foo bar"));
+}
+
+TEST_F(SearchParserTest, MatchNotConjunction) {
+  ParseExpr("-bar foo");
+
+  EXPECT_TRUE(Check("foo"));
+  EXPECT_TRUE(Check("foo rab"));
+
+  EXPECT_FALSE(Check("wrong"));
+  EXPECT_FALSE(Check("bar"));
+  EXPECT_FALSE(Check("foo bar"));
+}
+
+TEST_F(SearchParserTest, MatchDisjunction) {
   ParseExpr("foo | bar");
 
   EXPECT_TRUE(Check("foo bar"));
@@ -156,7 +179,21 @@ TEST_F(SearchParserTest, MatchDisjunctionTerm) {
   EXPECT_TRUE(Check("or only bar"));
 
   EXPECT_FALSE(Check("wrong"));
-  EXPECT_FALSE(Check("no mentions"));
+  EXPECT_FALSE(Check("far"));
+}
+
+TEST_F(SearchParserTest, MatchParenthesis) {
+  ParseExpr("( foo | oof ) ( bar | rab )");
+
+  EXPECT_TRUE(Check("foo bar"));
+  EXPECT_TRUE(Check("oof rab"));
+  EXPECT_TRUE(Check("foo rab"));
+  EXPECT_TRUE(Check("oof bar"));
+  EXPECT_TRUE(Check("foo oof bar rab"));
+
+  EXPECT_FALSE(Check("wrong"));
+  EXPECT_FALSE(Check("bar rab"));
+  EXPECT_FALSE(Check("foo oof"));
 }
 
 }  // namespace search