From 871b81656b1723f9ac59d37cd6ad1a92edfd351a Mon Sep 17 00:00:00 2001 From: Mislav Bradac Date: Sun, 7 May 2017 13:48:34 +0200 Subject: [PATCH] Implement InListOperator Reviewers: buda Reviewed By: buda Subscribers: pullbot Differential Revision: https://phabricator.memgraph.io/D354 --- src/query/frontend/ast/ast.hpp | 17 +++++++ src/query/frontend/ast/ast_visitor.hpp | 13 +++--- .../frontend/ast/cypher_main_visitor.cpp | 44 ++++++++++--------- .../frontend/ast/cypher_main_visitor.hpp | 10 ++++- .../frontend/opencypher/grammar/Cypher.g4 | 4 +- src/query/interpret/eval.hpp | 32 ++++++++++++++ tests/unit/cypher_main_visitor.cpp | 15 +++++++ tests/unit/query_expression_evaluator.cpp | 41 +++++++++++++++++ 8 files changed, 147 insertions(+), 29 deletions(-) diff --git a/src/query/frontend/ast/ast.hpp b/src/query/frontend/ast/ast.hpp index b7105fdd7..4977ef5a6 100644 --- a/src/query/frontend/ast/ast.hpp +++ b/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; diff --git a/src/query/frontend/ast/ast_visitor.hpp b/src/query/frontend/ast/ast_visitor.hpp index 65d1beacb..09a257d6d 100644 --- a/src/query/frontend/ast/ast_visitor.hpp +++ b/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 diff --git a/src/query/frontend/ast/cypher_main_visitor.cpp b/src/query/frontend/ast/cypher_main_visitor.cpp index 92e775b71..a24b85b99 100644 --- a/src/query/frontend/ast/cypher_main_visitor.cpp +++ b/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( - storage_.Create(expression)); + Expression *expression = ctx->expression3b()->accept(this); + + for (auto *op : ctx->stringAndNullOperators()) { + if (op->IS() && op->NOT() && op->CYPHERNULL()) { + expression = static_cast(storage_.Create( + storage_.Create(expression))); + } else if (op->IS() && op->CYPHERNULL()) { + expression = static_cast( + storage_.Create(expression)); + } else if (op->IN()) { + expression = static_cast(storage_.Create( + 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(storage_.Create( - storage_.Create(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(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( diff --git a/src/query/frontend/ast/cypher_main_visitor.hpp b/src/query/frontend/ast/cypher_main_visitor.hpp index 52fffa5fe..f7a6dcde5 100644 --- a/src/query/frontend/ast/cypher_main_visitor.hpp +++ b/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. * diff --git a/src/query/frontend/opencypher/grammar/Cypher.g4 b/src/query/frontend/opencypher/grammar/Cypher.g4 index f39629c6c..2f2f14c16 100644 --- a/src/query/frontend/opencypher/grammar/Cypher.g4 +++ b/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 )* ; diff --git a/src/query/interpret/eval.hpp b/src/query/interpret/eval.hpp index 0aa8cb1a3..fca166976 100644 --- a/src/query/interpret/eval.hpp +++ b/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>(); + 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()) { + 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(); diff --git a/tests/unit/cypher_main_visitor.cpp b/tests/unit/cypher_main_visitor.cpp index 0cdd34169..29f51b044 100644 --- a/tests/unit/cypher_main_visitor.cpp +++ b/tests/unit/cypher_main_visitor.cpp @@ -417,6 +417,21 @@ TEST(CypherMainVisitorTest, ListSlicingOperator) { EXPECT_EQ(upper_bound->value_.Value(), 2); } +TEST(CypherMainVisitorTest, InListOperator) { + AstGenerator ast_generator("RETURN 5 IN [1,2]"); + auto *query = ast_generator.query_; + auto *return_clause = dynamic_cast(query->clauses_[0]); + auto *in_list_operator = dynamic_cast( + return_clause->body_.named_expressions[0]->expression_); + ASSERT_TRUE(in_list_operator); + auto *literal = + dynamic_cast(in_list_operator->expression1_); + ASSERT_TRUE(literal); + ASSERT_EQ(literal->value_.Value(), 5); + auto *list = dynamic_cast(in_list_operator->expression2_); + ASSERT_TRUE(list); +} + TEST(CypherMainVisitorTest, IsNull) { AstGenerator ast_generator("RETURN 2 iS NulL"); auto *query = ast_generator.query_; diff --git a/tests/unit/query_expression_evaluator.cpp b/tests/unit/query_expression_evaluator.cpp index b1e7debd8..c4e93bdb9 100644 --- a/tests/unit/query_expression_evaluator.cpp +++ b/tests/unit/query_expression_evaluator.cpp @@ -245,6 +245,47 @@ TEST(ExpressionEvaluator, GreaterEqualOperator) { ASSERT_EQ(eval.eval.PopBack().Value(), true); } +TEST(ExpressionEvaluator, InListOperator) { + AstTreeStorage storage; + NoContextExpressionEvaluator eval; + auto *list_literal = storage.Create(std::vector{ + storage.Create(1), storage.Create(2), + storage.Create("a")}); + { + // Element exists in list. + auto *op = storage.Create( + storage.Create(2), list_literal); + op->Accept(eval.eval); + EXPECT_EQ(eval.eval.PopBack().Value(), true); + } + { + // Element doesn't exist in list. + auto *op = storage.Create( + storage.Create("x"), list_literal); + op->Accept(eval.eval); + EXPECT_EQ(eval.eval.PopBack().Value(), false); + } + { + auto *list_literal = storage.Create(std::vector{ + storage.Create(TypedValue::Null), + storage.Create(2), + storage.Create("a")}); + // Element doesn't exist in list with null element. + auto *op = storage.Create( + storage.Create("x"), list_literal); + op->Accept(eval.eval); + EXPECT_TRUE(eval.eval.PopBack().IsNull()); + } + { + // Null list. + auto *op = storage.Create( + storage.Create("x"), + storage.Create(TypedValue::Null)); + op->Accept(eval.eval); + EXPECT_TRUE(eval.eval.PopBack().IsNull()); + } +} + TEST(ExpressionEvaluator, ListIndexingOperator) { AstTreeStorage storage; NoContextExpressionEvaluator eval;