Implement InListOperator

Reviewers: buda

Reviewed By: buda

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D354

src/query/frontend/ast/ast.hpp

@@ -300,6 +300,23 @@ class GreaterEqualOperator : public BinaryOperator {
   using BinaryOperator::BinaryOperator;
 };
 
+class InListOperator : public BinaryOperator {
+  friend class AstTreeStorage;
+
+ public:
+  void Accept(TreeVisitorBase &visitor) override {
+    if (visitor.PreVisit(*this)) {
+      visitor.Visit(*this);
+      expression1_->Accept(visitor);
+      expression2_->Accept(visitor);
+      visitor.PostVisit(*this);
+    }
+  }
+
+ protected:
+  using BinaryOperator::BinaryOperator;
+};
+
 class ListIndexingOperator : public BinaryOperator {
   friend class AstTreeStorage;
 

src/query/frontend/ast/ast_visitor.hpp

@@ -40,6 +40,7 @@ class LessOperator;
 class GreaterOperator;
 class LessEqualOperator;
 class GreaterEqualOperator;
+class InListOperator;
 class ListIndexingOperator;
 class ListSlicingOperator;
 class Delete;
@@ -57,11 +58,11 @@ using TreeVisitorBase = ::utils::Visitor<
     AdditionOperator, SubtractionOperator, MultiplicationOperator,
     DivisionOperator, ModOperator, NotEqualOperator, EqualOperator,
     LessOperator, GreaterOperator, LessEqualOperator, GreaterEqualOperator,
-    ListIndexingOperator, ListSlicingOperator, UnaryPlusOperator,
-    UnaryMinusOperator, IsNullOperator, Identifier, PrimitiveLiteral,
-    ListLiteral, PropertyLookup, LabelsTest, EdgeTypeTest, Aggregation,
-    Function, Create, Match, Return, With, Pattern, NodeAtom, EdgeAtom, Delete,
-    Where, SetProperty, SetProperties, SetLabels, RemoveProperty, RemoveLabels,
-    Merge, Unwind>;
+    InListOperator, ListIndexingOperator, ListSlicingOperator,
+    UnaryPlusOperator, UnaryMinusOperator, IsNullOperator, Identifier,
+    PrimitiveLiteral, ListLiteral, PropertyLookup, LabelsTest, EdgeTypeTest,
+    Aggregation, Function, Create, Match, Return, With, Pattern, NodeAtom,
+    EdgeAtom, Delete, Where, SetProperty, SetProperties, SetLabels,
+    RemoveProperty, RemoveLabels, Merge, Unwind>;
 
 }  // namespace query

src/query/frontend/ast/cypher_main_visitor.cpp

@@ -590,30 +590,32 @@ antlrcpp::Any CypherMainVisitor::visitExpression4(
                              {kUnaryPlusTokenId, kUnaryMinusTokenId});
 }
 
-// IS NULL, IS NOT NULL, ...
+// IS NULL, IS NOT NULL, STARTS WITH, ..
 antlrcpp::Any CypherMainVisitor::visitExpression3a(
     CypherParser::Expression3aContext *ctx) {
-  // TODO: implement this.
-  // This is a hack. Unfortunately, grammar for expression3 contains a lot of
-  // different expressions. We should break that production in parts so that
-  // we can easily implement its visitor.
-  Expression *expression = ctx->expression3b()[0].accept(this);
-  if (ctx->children.size() - ctx->SP().size() == 3U && ctx->IS().size() == 1U &&
-      ctx->CYPHERNULL().size() == 1U) {
-    return static_cast<Expression *>(
-        storage_.Create<IsNullOperator>(expression));
+  Expression *expression = ctx->expression3b()->accept(this);
+
+  for (auto *op : ctx->stringAndNullOperators()) {
+    if (op->IS() && op->NOT() && op->CYPHERNULL()) {
+      expression = static_cast<Expression *>(storage_.Create<NotOperator>(
+          storage_.Create<IsNullOperator>(expression)));
+    } else if (op->IS() && op->CYPHERNULL()) {
+      expression = static_cast<Expression *>(
+          storage_.Create<IsNullOperator>(expression));
+    } else if (op->IN()) {
+      expression = static_cast<Expression *>(storage_.Create<InListOperator>(
+          expression, op->expression2a()->accept(this)));
+    } else {
+      throw utils::NotYetImplemented();
+    }
   }
-  if (ctx->children.size() - ctx->SP().size() == 4U && ctx->IS().size() == 1U &&
-      ctx->NOT().size() == 1U && ctx->CYPHERNULL().size() == 1U) {
-    return static_cast<Expression *>(storage_.Create<NotOperator>(
-        storage_.Create<IsNullOperator>(expression)));
-  }
-  if (ctx->children.size() > 1U) {
-    throw utils::NotYetImplemented();
-  }
-  // If there is only one child we don't need to generate any code in this since
-  // that child is expression2.
-  return static_cast<Expression *>(visitChildren(ctx));
+  return expression;
+}
+
+antlrcpp::Any CypherMainVisitor::visitStringAndNullOperators(
+    CypherParser::StringAndNullOperatorsContext *) {
+  debug_assert(false, "Should never be called. See documentation in hpp.");
+  return 0;
 }
 
 antlrcpp::Any CypherMainVisitor::visitExpression3b(

src/query/frontend/ast/cypher_main_visitor.hpp

@@ -361,13 +361,21 @@ class CypherMainVisitor : public antlropencypher::CypherBaseVisitor {
       CypherParser::Expression4Context *ctx) override;
 
   /**
-   * IS NULL, IS NOT NULL, ...
+   * IS NULL, IS NOT NULL, STARTS WITH, END WITH, =~, ...
    *
    * @return Expression*
    */
   antlrcpp::Any visitExpression3a(
       CypherParser::Expression3aContext *ctx) override;
 
+  /**
+   * Does nothing, everything is done in visitExpression3a.
+   *
+   * @return Expression*
+   */
+  antlrcpp::Any visitStringAndNullOperators(
+      CypherParser::StringAndNullOperatorsContext *ctx) override;
+
   /**
    * List indexing and slicing.
    *

src/query/frontend/opencypher/grammar/Cypher.g4

@@ -153,7 +153,9 @@ expression5 : expression4 ( SP? '^' SP? expression4 )* ;
 
 expression4 : ( ( '+' | '-' ) SP? )* expression3a ;
 
-expression3a : expression3b ( ( ( ( SP? '=~' ) | ( SP IN ) | ( SP STARTS SP WITH ) | ( SP ENDS SP WITH ) | ( SP CONTAINS ) ) SP? expression2a ) | ( SP IS SP CYPHERNULL ) | ( SP IS SP NOT SP CYPHERNULL ) )* ;
+expression3a : expression3b ( stringAndNullOperators )* ;
+
+stringAndNullOperators : ( ( ( ( SP? '=~' ) | ( SP IN ) | ( SP STARTS SP WITH ) | ( SP ENDS SP WITH ) | ( SP CONTAINS ) ) SP? expression2a ) | ( SP IS SP CYPHERNULL ) | ( SP IS SP NOT SP CYPHERNULL ) ) ;
 
 expression3b : expression2a ( SP? listIndexingOrSlicing )* ;
 

src/query/interpret/eval.hpp

@@ -89,6 +89,38 @@ class ExpressionEvaluator : public TreeVisitorBase {
 #undef BINARY_OPERATOR_VISITOR
 #undef UNARY_OPERATOR_VISITOR
 
+  void PostVisit(InListOperator &) override {
+    auto _list = PopBack();
+    auto literal = PopBack();
+    if (_list.IsNull()) {
+      result_stack_.emplace_back(TypedValue::Null);
+      return;
+    }
+    // Exceptions have higher priority than returning null.
+    // We need to convert list to its type before checking if literal is null,
+    // because conversion will throw exception if list conversion fails.
+    auto list = _list.Value<std::vector<TypedValue>>();
+    if (literal.IsNull()) {
+      result_stack_.emplace_back(TypedValue::Null);
+      return;
+    }
+    auto has_null = false;
+    for (const auto &element : list) {
+      auto result = literal == element;
+      if (result.IsNull()) {
+        has_null = true;
+      } else if (result.Value<bool>()) {
+        result_stack_.emplace_back(true);
+        return;
+      }
+    }
+    if (has_null) {
+      result_stack_.emplace_back(TypedValue::Null);
+    } else {
+      result_stack_.emplace_back(false);
+    }
+  }
+
   void PostVisit(ListIndexingOperator &) override {
     // TODO: implement this for maps
     auto _index = PopBack();

tests/unit/cypher_main_visitor.cpp

@@ -417,6 +417,21 @@ TEST(CypherMainVisitorTest, ListSlicingOperator) {
   EXPECT_EQ(upper_bound->value_.Value<int64_t>(), 2);
 }
 
+TEST(CypherMainVisitorTest, InListOperator) {
+  AstGenerator ast_generator("RETURN 5 IN [1,2]");
+  auto *query = ast_generator.query_;
+  auto *return_clause = dynamic_cast<Return *>(query->clauses_[0]);
+  auto *in_list_operator = dynamic_cast<InListOperator *>(
+      return_clause->body_.named_expressions[0]->expression_);
+  ASSERT_TRUE(in_list_operator);
+  auto *literal =
+      dynamic_cast<PrimitiveLiteral *>(in_list_operator->expression1_);
+  ASSERT_TRUE(literal);
+  ASSERT_EQ(literal->value_.Value<int64_t>(), 5);
+  auto *list = dynamic_cast<ListLiteral *>(in_list_operator->expression2_);
+  ASSERT_TRUE(list);
+}
+
 TEST(CypherMainVisitorTest, IsNull) {
   AstGenerator ast_generator("RETURN 2 iS NulL");
   auto *query = ast_generator.query_;

tests/unit/query_expression_evaluator.cpp

@@ -245,6 +245,47 @@ TEST(ExpressionEvaluator, GreaterEqualOperator) {
   ASSERT_EQ(eval.eval.PopBack().Value<bool>(), true);
 }
 
+TEST(ExpressionEvaluator, InListOperator) {
+  AstTreeStorage storage;
+  NoContextExpressionEvaluator eval;
+  auto *list_literal = storage.Create<ListLiteral>(std::vector<Expression *>{
+      storage.Create<PrimitiveLiteral>(1), storage.Create<PrimitiveLiteral>(2),
+      storage.Create<PrimitiveLiteral>("a")});
+  {
+    // Element exists in list.
+    auto *op = storage.Create<InListOperator>(
+        storage.Create<PrimitiveLiteral>(2), list_literal);
+    op->Accept(eval.eval);
+    EXPECT_EQ(eval.eval.PopBack().Value<bool>(), true);
+  }
+  {
+    // Element doesn't exist in list.
+    auto *op = storage.Create<InListOperator>(
+        storage.Create<PrimitiveLiteral>("x"), list_literal);
+    op->Accept(eval.eval);
+    EXPECT_EQ(eval.eval.PopBack().Value<bool>(), false);
+  }
+  {
+    auto *list_literal = storage.Create<ListLiteral>(std::vector<Expression *>{
+        storage.Create<PrimitiveLiteral>(TypedValue::Null),
+        storage.Create<PrimitiveLiteral>(2),
+        storage.Create<PrimitiveLiteral>("a")});
+    // Element doesn't exist in list with null element.
+    auto *op = storage.Create<InListOperator>(
+        storage.Create<PrimitiveLiteral>("x"), list_literal);
+    op->Accept(eval.eval);
+    EXPECT_TRUE(eval.eval.PopBack().IsNull());
+  }
+  {
+    // Null list.
+    auto *op = storage.Create<InListOperator>(
+        storage.Create<PrimitiveLiteral>("x"),
+        storage.Create<PrimitiveLiteral>(TypedValue::Null));
+    op->Accept(eval.eval);
+    EXPECT_TRUE(eval.eval.PopBack().IsNull());
+  }
+}
+
 TEST(ExpressionEvaluator, ListIndexingOperator) {
   AstTreeStorage storage;
   NoContextExpressionEvaluator eval;