From 4bfae46150984119bd41cb74e4bdb9f17d3c8b96 Mon Sep 17 00:00:00 2001 From: Teon Banek Date: Mon, 27 Mar 2017 12:10:50 +0200 Subject: [PATCH] Add basic planning of WHERE Summary: Add testing unbound variable in WHERE Remove some duplication in planner tests Reviewers: florijan, mislav.bradac Reviewed By: florijan Subscribers: pullbot Differential Revision: https://phabricator.memgraph.io/D186 --- src/query/frontend/ast/ast.hpp | 34 ++++---- src/query/frontend/logical/planner.cpp | 8 +- tests/unit/query_common.hpp | 8 +- tests/unit/query_planner.cpp | 115 +++++++++---------------- tests/unit/query_semantic.cpp | 12 +++ 5 files changed, 82 insertions(+), 95 deletions(-) diff --git a/src/query/frontend/ast/ast.hpp b/src/query/frontend/ast/ast.hpp index c94241eaf..c7c308bad 100644 --- a/src/query/frontend/ast/ast.hpp +++ b/src/query/frontend/ast/ast.hpp @@ -492,6 +492,22 @@ class Create : public Clause { } }; +class Where : public Tree { + friend class AstTreeStorage; + + public: + void Accept(TreeVisitorBase &visitor) override { + visitor.Visit(*this); + expression_->Accept(visitor); + visitor.PostVisit(*this); + } + Expression *expression_ = nullptr; + + protected: + Where(int uid) : Tree(uid) {} + Where(int uid, Expression *expression) : Tree(uid), expression_(expression) {} +}; + class Match : public Clause { friend class AstTreeStorage; @@ -501,6 +517,9 @@ class Match : public Clause { for (auto &pattern : patterns_) { pattern->Accept(visitor); } + if (where_) { + where_->Accept(visitor); + } visitor.PostVisit(*this); } std::vector patterns_; @@ -545,21 +564,6 @@ class Delete : public Clause { Delete(int uid) : Clause(uid) {} }; -class Where : public Tree { - friend class AstTreeStorage; - - public: - void Accept(TreeVisitorBase &visitor) override { - visitor.Visit(*this); - expression_->Accept(visitor); - visitor.PostVisit(*this); - } - Expression *expression_ = nullptr; - - protected: - Where(int uid) : Tree(uid) {} -}; - class SetProperty : public Clause { friend class AstTreeStorage; diff --git a/src/query/frontend/logical/planner.cpp b/src/query/frontend/logical/planner.cpp index fc8599a2a..bc37c2716 100644 --- a/src/query/frontend/logical/planner.cpp +++ b/src/query/frontend/logical/planner.cpp @@ -152,7 +152,13 @@ auto GenMatch(Match &match, LogicalOperator *input_op, // TODO: Support matching multiple patterns. throw NotYetImplemented(); } - return ReducePattern(*match.patterns_[0], base, collect); + auto last_op = + ReducePattern(*match.patterns_[0], base, collect); + if (match.where_) { + last_op = new Filter(std::shared_ptr(last_op), + match.where_->expression_); + } + return last_op; } auto GenReturn(Return &ret, LogicalOperator *input_op) { diff --git a/tests/unit/query_common.hpp b/tests/unit/query_common.hpp index be350b52c..54421ddb2 100644 --- a/tests/unit/query_common.hpp +++ b/tests/unit/query_common.hpp @@ -96,8 +96,7 @@ auto GetReturn(AstTreeStorage &storage, /// /// All the following macros implicitly pass `storage` variable to functions. -/// You -/// need to have `AstTreeStorage storage;` somewhere in scope to use them. +/// You need to have `AstTreeStorage storage;` somewhere in scope to use them. /// Refer to function documentation to see what the macro does. /// /// Example usage: @@ -112,11 +111,14 @@ auto GetReturn(AstTreeStorage &storage, #define PATTERN(...) query::test_common::GetPattern(storage, {__VA_ARGS__}) #define MATCH(...) \ query::test_common::GetWithPatterns(storage, {__VA_ARGS__}) +#define WHERE(expr) storage.Create((expr)) #define CREATE(...) \ query::test_common::GetWithPatterns(storage, {__VA_ARGS__}) #define IDENT(name) storage.Create((name)) #define LITERAL(val) storage.Create((val)) -#define PROPERTY_LOOKUP(...) query::test_common::GetPropertyLookup(storage, __VA_ARGS__) +#define PROPERTY_LOOKUP(...) \ + query::test_common::GetPropertyLookup(storage, __VA_ARGS__) #define NEXPR(name, expr) storage.Create((name), (expr)) #define RETURN(...) query::test_common::GetReturn(storage, {__VA_ARGS__}) #define QUERY(...) query::test_common::GetQuery(storage, {__VA_ARGS__}) +#define LESS(expr1, expr2) storage.Create((expr1), (expr2)) diff --git a/tests/unit/query_planner.cpp b/tests/unit/query_planner.cpp index e277d970d..01e55b375 100644 --- a/tests/unit/query_planner.cpp +++ b/tests/unit/query_planner.cpp @@ -32,6 +32,7 @@ class PlanChecker : public LogicalOperatorVisitor { void Visit(Expand &op) override { AssertType(op); } void Visit(NodeFilter &op) override { AssertType(op); } void Visit(EdgeFilter &op) override { AssertType(op); } + void Visit(Filter &op) override { AssertType(op); } void Visit(Produce &op) override { AssertType(op); } private: @@ -43,19 +44,20 @@ class PlanChecker : public LogicalOperatorVisitor { std::list types_; }; +auto CheckPlan(query::Query &query, std::list expected_types) { + SymbolTable symbol_table; + SymbolGenerator symbol_generator(symbol_table); + query.Accept(symbol_generator); + auto plan = MakeLogicalPlan(query, symbol_table); + PlanChecker plan_checker(expected_types); + plan->Accept(plan_checker); +} + TEST(TestLogicalPlanner, MatchNodeReturn) { // Test MATCH (n) RETURN n AS n AstTreeStorage storage; auto query = QUERY(MATCH(PATTERN(NODE("n"))), RETURN(NEXPR("n", IDENT("n")))); - SymbolTable symbol_table; - SymbolGenerator symbol_generator(symbol_table); - query->Accept(symbol_generator); - auto plan = MakeLogicalPlan(*query, symbol_table); - std::list expected_types; - expected_types.emplace_back(typeid(ScanAll).hash_code()); - expected_types.emplace_back(typeid(Produce).hash_code()); - PlanChecker plan_checker(expected_types); - plan->Accept(plan_checker); + CheckPlan(*query, {typeid(ScanAll).hash_code(), typeid(Produce).hash_code()}); } TEST(TestLogicalPlanner, CreateNodeReturn) { @@ -63,15 +65,8 @@ TEST(TestLogicalPlanner, CreateNodeReturn) { AstTreeStorage storage; auto query = QUERY(CREATE(PATTERN(NODE("n"))), RETURN(NEXPR("n", IDENT("n")))); - SymbolTable symbol_table; - SymbolGenerator symbol_generator(symbol_table); - query->Accept(symbol_generator); - auto plan = MakeLogicalPlan(*query, symbol_table); - std::list expected_types; - expected_types.emplace_back(typeid(CreateNode).hash_code()); - expected_types.emplace_back(typeid(Produce).hash_code()); - PlanChecker plan_checker(expected_types); - plan->Accept(plan_checker); + CheckPlan(*query, + {typeid(CreateNode).hash_code(), typeid(Produce).hash_code()}); } TEST(TestLogicalPlanner, CreateExpand) { @@ -80,30 +75,16 @@ TEST(TestLogicalPlanner, CreateExpand) { std::string relationship("relationship"); auto query = QUERY(CREATE(PATTERN( NODE("n"), EDGE("r", &relationship, Direction::RIGHT), NODE("m")))); - SymbolTable symbol_table; - SymbolGenerator symbol_generator(symbol_table); - query->Accept(symbol_generator); - auto plan = MakeLogicalPlan(*query, symbol_table); - std::list expected_types; - expected_types.emplace_back(typeid(CreateNode).hash_code()); - expected_types.emplace_back(typeid(CreateExpand).hash_code()); - PlanChecker plan_checker(expected_types); - plan->Accept(plan_checker); + CheckPlan(*query, + {typeid(CreateNode).hash_code(), typeid(CreateExpand).hash_code()}); } TEST(TestLogicalPlanner, CreateMultipleNode) { // Test CREATE (n), (m) AstTreeStorage storage; auto query = QUERY(CREATE(PATTERN(NODE("n")), PATTERN(NODE("m")))); - SymbolTable symbol_table; - SymbolGenerator symbol_generator(symbol_table); - query->Accept(symbol_generator); - auto plan = MakeLogicalPlan(*query, symbol_table); - std::list expected_types; - expected_types.emplace_back(typeid(CreateNode).hash_code()); - expected_types.emplace_back(typeid(CreateNode).hash_code()); - PlanChecker plan_checker(expected_types); - plan->Accept(plan_checker); + CheckPlan(*query, + {typeid(CreateNode).hash_code(), typeid(CreateNode).hash_code()}); } TEST(TestLogicalPlanner, CreateNodeExpandNode) { @@ -113,16 +94,9 @@ TEST(TestLogicalPlanner, CreateNodeExpandNode) { auto query = QUERY(CREATE( PATTERN(NODE("n"), EDGE("r", &relationship, Direction::RIGHT), NODE("m")), PATTERN(NODE("l")))); - SymbolTable symbol_table; - SymbolGenerator symbol_generator(symbol_table); - query->Accept(symbol_generator); - auto plan = MakeLogicalPlan(*query, symbol_table); - std::list expected_types; - expected_types.emplace_back(typeid(CreateNode).hash_code()); - expected_types.emplace_back(typeid(CreateExpand).hash_code()); - expected_types.emplace_back(typeid(CreateNode).hash_code()); - PlanChecker plan_checker(expected_types); - plan->Accept(plan_checker); + CheckPlan(*query, + {typeid(CreateNode).hash_code(), typeid(CreateExpand).hash_code(), + typeid(CreateNode).hash_code()}); } TEST(TestLogicalPlanner, MatchCreateExpand) { @@ -133,15 +107,8 @@ TEST(TestLogicalPlanner, MatchCreateExpand) { MATCH(PATTERN(NODE("n"))), CREATE(PATTERN(NODE("n"), EDGE("r", &relationship, Direction::RIGHT), NODE("m")))); - SymbolTable symbol_table; - SymbolGenerator symbol_generator(symbol_table); - query->Accept(symbol_generator); - auto plan = MakeLogicalPlan(*query, symbol_table); - std::list expected_types; - expected_types.emplace_back(typeid(ScanAll).hash_code()); - expected_types.emplace_back(typeid(CreateExpand).hash_code()); - PlanChecker plan_checker(expected_types); - plan->Accept(plan_checker); + CheckPlan(*query, + {typeid(ScanAll).hash_code(), typeid(CreateExpand).hash_code()}); } TEST(TestLogicalPlanner, MatchLabeledNodes) { @@ -150,16 +117,9 @@ TEST(TestLogicalPlanner, MatchLabeledNodes) { std::string label("label"); auto query = QUERY(MATCH(PATTERN(NODE("n", &label))), RETURN(NEXPR("n", IDENT("n")))); - SymbolTable symbol_table; - SymbolGenerator symbol_generator(symbol_table); - query->Accept(symbol_generator); - auto plan = MakeLogicalPlan(*query, symbol_table); - std::list expected_types; - expected_types.emplace_back(typeid(ScanAll).hash_code()); - expected_types.emplace_back(typeid(NodeFilter).hash_code()); - expected_types.emplace_back(typeid(Produce).hash_code()); - PlanChecker plan_checker(expected_types); - plan->Accept(plan_checker); + CheckPlan(*query, + {typeid(ScanAll).hash_code(), typeid(NodeFilter).hash_code(), + typeid(Produce).hash_code()}); } TEST(TestLogicalPlanner, MatchPathReturn) { @@ -169,17 +129,20 @@ TEST(TestLogicalPlanner, MatchPathReturn) { auto query = QUERY(MATCH(PATTERN(NODE("n"), EDGE("r", &relationship), NODE("m"))), RETURN(NEXPR("n", IDENT("n")))); - SymbolTable symbol_table; - SymbolGenerator symbol_generator(symbol_table); - query->Accept(symbol_generator); - auto plan = MakeLogicalPlan(*query, symbol_table); - std::list expected_types; - expected_types.emplace_back(typeid(ScanAll).hash_code()); - expected_types.emplace_back(typeid(Expand).hash_code()); - expected_types.emplace_back(typeid(EdgeFilter).hash_code()); - expected_types.emplace_back(typeid(Produce).hash_code()); - PlanChecker plan_checker(expected_types); - plan->Accept(plan_checker); + CheckPlan(*query, + {typeid(ScanAll).hash_code(), typeid(Expand).hash_code(), + typeid(EdgeFilter).hash_code(), typeid(Produce).hash_code()}); +} + +TEST(TestLogicalPlanner, MatchWhereReturn) { + // Test MATCH (n) WHERE n.property < 42 RETURN n AS n + AstTreeStorage storage; + std::string property("property"); + auto match = MATCH(PATTERN(NODE("n"))); + match->where_ = WHERE(LESS(PROPERTY_LOOKUP("n", &property), LITERAL(42))); + auto query = QUERY(match, RETURN(NEXPR("n", IDENT("n")))); + CheckPlan(*query, {typeid(ScanAll).hash_code(), typeid(Filter).hash_code(), + typeid(Produce).hash_code()}); } } diff --git a/tests/unit/query_semantic.cpp b/tests/unit/query_semantic.cpp index 528be6395..fdd79e665 100644 --- a/tests/unit/query_semantic.cpp +++ b/tests/unit/query_semantic.cpp @@ -225,4 +225,16 @@ TEST(TestSymbolGenerator, CreateBidirectionalEdge) { EXPECT_THROW(query->Accept(symbol_generator), SemanticException); } +TEST(TestSymbolGenerator, MatchWhereUnbound) { + // Test MATCH (n) WHERE missing < 42 RETURN n AS n + AstTreeStorage storage; + std::string property("property"); + auto match = MATCH(PATTERN(NODE("n"))); + match->where_ = WHERE(LESS(IDENT("missing"), LITERAL(42))); + auto query = QUERY(match, RETURN(NEXPR("n", IDENT("n")))); + SymbolTable symbol_table; + SymbolGenerator symbol_generator(symbol_table); + EXPECT_THROW(query->Accept(symbol_generator), UnboundVariableError); +} + }