From db407142ce40b2be164b5f7392eb381428a6bd5c Mon Sep 17 00:00:00 2001 From: Teon Banek Date: Thu, 8 Feb 2018 12:57:12 +0100 Subject: [PATCH] Add wShortest expansion to openCypher Reviewers: florijan, msantl Reviewed By: msantl Subscribers: pullbot Differential Revision: https://phabricator.memgraph.io/D1181 --- src/query/frontend/ast/ast.hpp | 103 +++++++++++------- .../frontend/ast/cypher_main_visitor.cpp | 77 ++++++++----- .../frontend/ast/cypher_main_visitor.hpp | 2 +- .../frontend/opencypher/grammar/Cypher.g4 | 6 +- .../frontend/semantic/symbol_generator.cpp | 24 ++-- src/query/plan/preprocess.cpp | 27 +++-- src/query/plan/rule_based_planner.hpp | 9 +- tests/unit/cypher_main_visitor.cpp | 100 +++++++++++++++-- tests/unit/distributed_data_exchange.cpp | 2 +- tests/unit/query_common.hpp | 4 +- tests/unit/query_planner.cpp | 12 +- tests/unit/query_semantic.cpp | 89 +++++++++++---- tests/unit/query_variable_start_planner.cpp | 6 +- 13 files changed, 334 insertions(+), 127 deletions(-) diff --git a/src/query/frontend/ast/ast.hpp b/src/query/frontend/ast/ast.hpp index dc748f576..e20312c95 100644 --- a/src/query/frontend/ast/ast.hpp +++ b/src/query/frontend/ast/ast.hpp @@ -40,14 +40,14 @@ namespace query { #define CLONE_BINARY_EXPRESSION \ auto Clone(AstTreeStorage &storage) const->std::remove_const< \ - std::remove_pointer::type>::type * override { \ + std::remove_pointer::type>::type *override { \ return storage.Create< \ std::remove_cv::type>::type>( \ expression1_->Clone(storage), expression2_->Clone(storage)); \ } #define CLONE_UNARY_EXPRESSION \ auto Clone(AstTreeStorage &storage) const->std::remove_const< \ - std::remove_pointer::type>::type * override { \ + std::remove_pointer::type>::type *override { \ return storage.Create< \ std::remove_cv::type>::type>( \ expression_->Clone(storage)); \ @@ -61,7 +61,6 @@ namespace query { class Tree; - // It would be better to call this AstTree, but we already have a class Tree, // which could be renamed to Node or AstTreeNode, but we also have a class // called NodeAtom... @@ -85,7 +84,7 @@ class AstTreeStorage { Query *query() const; /// Id for using get_helper in boost archives. - static void * const kHelperId; + static void *const kHelperId; /// Load an Ast Node into this storage. template @@ -1364,8 +1363,8 @@ class Aggregation : public BinaryOperator { Op op_; protected: - /** Aggregation's first expression is the value being aggregated. The second - * expression is the key used only in COLLECT_MAP. */ + /// Aggregation's first expression is the value being aggregated. The second + /// expression is the key used only in COLLECT_MAP. Aggregation(int uid, Expression *expression1, Expression *expression2, Op op) : BinaryOperator(uid, expression1, expression2), op_(op) { // COUNT without expression denotes COUNT(*) in cypher. @@ -1738,9 +1737,24 @@ class EdgeAtom : public PatternAtom { } public: - enum class Type { SINGLE, DEPTH_FIRST, BREADTH_FIRST }; + enum class Type { + SINGLE, + DEPTH_FIRST, + BREADTH_FIRST, + WEIGHTED_SHORTEST_PATH + }; enum class Direction { IN, OUT, BOTH }; + /// Lambda for use in filtering or weight calculation during variable expands. + struct Lambda { + /// Argument identifier for the edge currently being traversed. + Identifier *inner_edge = nullptr; + /// Argument identifier for the destination node of the edge. + Identifier *inner_node = nullptr; + /// Evaluates the result of the lambda. + Expression *expression = nullptr; + }; + DEFVISITABLE(TreeVisitor); bool Accept(HierarchicalTreeVisitor &visitor) override { if (visitor.PreVisit(*this)) { @@ -1770,9 +1784,13 @@ class EdgeAtom : public PatternAtom { } edge_atom->lower_bound_ = CloneOpt(lower_bound_, storage); edge_atom->upper_bound_ = CloneOpt(upper_bound_, storage); - edge_atom->inner_edge_ = CloneOpt(inner_edge_, storage); - edge_atom->inner_node_ = CloneOpt(inner_node_, storage); - edge_atom->filter_expression_ = CloneOpt(filter_expression_, storage); + auto clone_lambda = [&storage](const auto &lambda) { + return Lambda{CloneOpt(lambda.inner_edge, storage), + CloneOpt(lambda.inner_node, storage), + CloneOpt(lambda.expression, storage)}; + }; + edge_atom->filter_lambda_ = clone_lambda(filter_lambda_); + edge_atom->weight_lambda_ = clone_lambda(weight_lambda_); return edge_atom; } @@ -1780,8 +1798,9 @@ class EdgeAtom : public PatternAtom { switch (type_) { case Type::DEPTH_FIRST: case Type::BREADTH_FIRST: + case Type::WEIGHTED_SHORTEST_PATH: return true; - default: + case Type::SINGLE: return false; } } @@ -1792,14 +1811,20 @@ class EdgeAtom : public PatternAtom { std::unordered_map, Expression *> properties_; - // Used in variable length and BFS expansions. Bounds can be nullptr. Inner - // element symbols can only be null in SINGLE expansion. Filter can be - // nullptr. + // Members used only in variable length expansions. + + /// Evaluates to lower bound in variable length expands. Expression *lower_bound_ = nullptr; + /// Evaluated to upper bound in variable length expands. Expression *upper_bound_ = nullptr; - Identifier *inner_edge_ = nullptr; - Identifier *inner_node_ = nullptr; - Expression *filter_expression_ = nullptr; + /// Filter lambda for variable length expands. + /// Can have an empty expression, but identifiers must be valid, because an + /// optimization pass may inline other expressions into this lambda. + Lambda filter_lambda_; + /// Used in weighted shortest path. + /// It must have valid expressions and identifiers. In all other expand types, + /// it is empty. + Lambda weight_lambda_; protected: using PatternAtom::PatternAtom; @@ -1834,9 +1859,13 @@ class EdgeAtom : public PatternAtom { } SavePointer(ar, lower_bound_); SavePointer(ar, upper_bound_); - SavePointer(ar, inner_edge_); - SavePointer(ar, inner_node_); - SavePointer(ar, filter_expression_); + auto save_lambda = [&ar](const auto &lambda) { + SavePointer(ar, lambda.inner_edge); + SavePointer(ar, lambda.inner_node); + SavePointer(ar, lambda.expression); + }; + save_lambda(filter_lambda_); + save_lambda(weight_lambda_); } template @@ -1858,9 +1887,13 @@ class EdgeAtom : public PatternAtom { } LoadPointer(ar, lower_bound_); LoadPointer(ar, upper_bound_); - LoadPointer(ar, inner_edge_); - LoadPointer(ar, inner_node_); - LoadPointer(ar, filter_expression_); + auto load_lambda = [&ar](auto &lambda) { + LoadPointer(ar, lambda.inner_edge); + LoadPointer(ar, lambda.inner_node); + LoadPointer(ar, lambda.expression); + }; + load_lambda(filter_lambda_); + load_lambda(weight_lambda_); } template @@ -2014,10 +2047,8 @@ class CypherUnion : public Tree { SingleQuery *single_query_ = nullptr; bool distinct_ = false; - /** - * @brief Holds symbols that are created during symbol generation phase. - * These symbols are used when UNION/UNION ALL combines single query results. - */ + /// Holds symbols that are created during symbol generation phase. + /// These symbols are used when UNION/UNION ALL combines single query results. std::vector union_symbols_; protected: @@ -2225,24 +2256,22 @@ class Match : public Clause { const unsigned int); }; -/** @brief Defines the order for sorting values (ascending or descending). */ +/// Defines the order for sorting values (ascending or descending). enum class Ordering { ASC, DESC }; -/** - * @brief Contents common to @c Return and @c With clauses. - */ +/// Contents common to @c Return and @c With clauses. struct ReturnBody { - /** @brief True if distinct results should be produced. */ + /// True if distinct results should be produced. bool distinct = false; - /** @brief True if asterisk was found in return body */ + /// True if asterisk was found in return body bool all_identifiers = false; - /** @brief Expressions which are used to produce results. */ + /// Expressions which are used to produce results. std::vector named_expressions; - /** @brief Expressions used for ordering the results. */ + /// Expressions used for ordering the results. std::vector> order_by; - /** @brief Optional expression on how many results to skip. */ + /// Optional expression on how many results to skip. Expression *skip = nullptr; - /** @brief Optional expression on how much to limit the results. */ + /// Optional expression on how much to limit the results. Expression *limit = nullptr; }; diff --git a/src/query/frontend/ast/cypher_main_visitor.cpp b/src/query/frontend/ast/cypher_main_visitor.cpp index a5bb5286c..b4e30be37 100644 --- a/src/query/frontend/ast/cypher_main_visitor.cpp +++ b/src/query/frontend/ast/cypher_main_visitor.cpp @@ -459,18 +459,11 @@ antlrcpp::Any CypherMainVisitor::visitRelationshipPattern( auto relationshipDetail = ctx->relationshipDetail(); auto *variableExpansion = relationshipDetail ? relationshipDetail->variableExpansion() : nullptr; - bool is_bfs = false; + edge->type_ = EdgeAtom::Type::SINGLE; if (variableExpansion) - std::tie(is_bfs, edge->lower_bound_, edge->upper_bound_) = + std::tie(edge->type_, edge->lower_bound_, edge->upper_bound_) = variableExpansion->accept(this) - .as>(); - - if (!variableExpansion) - edge->type_ = EdgeAtom::Type::SINGLE; - else if (is_bfs) - edge->type_ = EdgeAtom::Type::BREADTH_FIRST; - else - edge->type_ = EdgeAtom::Type::DEPTH_FIRST; + .as>(); if (ctx->leftArrowHead() && !ctx->rightArrowHead()) { edge->direction_ = EdgeAtom::Direction::IN; @@ -504,25 +497,47 @@ antlrcpp::Any CypherMainVisitor::visitRelationshipPattern( auto relationshipLambdas = relationshipDetail->relationshipLambda(); if (variableExpansion) { + auto visit_lambda = [this](auto *lambda) { + EdgeAtom::Lambda edge_lambda; + std::string traversed_edge_variable = + lambda->traversed_edge->accept(this); + edge_lambda.inner_edge = + storage_.Create(traversed_edge_variable); + std::string traversed_node_variable = + lambda->traversed_node->accept(this); + edge_lambda.inner_node = + storage_.Create(traversed_node_variable); + edge_lambda.expression = lambda->expression()->accept(this); + return edge_lambda; + }; switch (relationshipLambdas.size()) { case 0: - // In variable length and BFS expansion inner variables are mandatory. - anonymous_identifiers.push_back(&edge->inner_edge_); - anonymous_identifiers.push_back(&edge->inner_node_); + if (edge->type_ == EdgeAtom::Type::WEIGHTED_SHORTEST_PATH) + throw SemanticException( + "Lambda for calculating weights is mandatory"); + // In variable expansion inner variables are mandatory. + anonymous_identifiers.push_back(&edge->filter_lambda_.inner_edge); + anonymous_identifiers.push_back(&edge->filter_lambda_.inner_node); break; - case 1: { - auto *lambda = relationshipLambdas[0]; - std::string traversed_edge_variable = - lambda->traversed_edge->accept(this); - edge->inner_edge_ = - storage_.Create(traversed_edge_variable); - std::string traversed_node_variable = - lambda->traversed_node->accept(this); - edge->inner_node_ = - storage_.Create(traversed_node_variable); - edge->filter_expression_ = lambda->expression()->accept(this); + case 1: + if (edge->type_ == EdgeAtom::Type::WEIGHTED_SHORTEST_PATH) { + // For wShortest, the first (and required) lambda is used for weight + // calculation. + edge->weight_lambda_ = visit_lambda(relationshipLambdas[0]); + // Add mandatory inner variables for filter lambda. + anonymous_identifiers.push_back(&edge->filter_lambda_.inner_edge); + anonymous_identifiers.push_back(&edge->filter_lambda_.inner_node); + } else { + // Other variable expands only have the filter lambda. + edge->filter_lambda_ = visit_lambda(relationshipLambdas[0]); + } + break; + case 2: + if (edge->type_ != EdgeAtom::Type::WEIGHTED_SHORTEST_PATH) + throw SemanticException("Only one relationship lambda allowed"); + edge->weight_lambda_ = visit_lambda(relationshipLambdas[0]); + edge->filter_lambda_ = visit_lambda(relationshipLambdas[1]); break; - }; default: throw SemanticException("Only one relationship lambda allowed"); } @@ -576,7 +591,11 @@ antlrcpp::Any CypherMainVisitor::visitVariableExpansion( DCHECK(ctx->expression().size() <= 2U) << "Expected 0, 1 or 2 bounds in range literal."; - bool is_bfs = !ctx->getTokens(CypherParser::BFS).empty(); + EdgeAtom::Type edge_type = EdgeAtom::Type::DEPTH_FIRST; + if (!ctx->getTokens(CypherParser::BFS).empty()) + edge_type = EdgeAtom::Type::BREADTH_FIRST; + else if (!ctx->getTokens(CypherParser::WSHORTEST).empty()) + edge_type = EdgeAtom::Type::WEIGHTED_SHORTEST_PATH; Expression *lower = nullptr; Expression *upper = nullptr; @@ -587,7 +606,7 @@ antlrcpp::Any CypherMainVisitor::visitVariableExpansion( Expression *bound = ctx->expression()[0]->accept(this); if (!dots_tokens.size()) { // Case -[*bound]- - lower = bound; + if (edge_type != EdgeAtom::Type::WEIGHTED_SHORTEST_PATH) lower = bound; upper = bound; } else if (dots_tokens[0]->getSourceInterval().startsAfter( ctx->expression()[0]->getSourceInterval())) { @@ -602,8 +621,10 @@ antlrcpp::Any CypherMainVisitor::visitVariableExpansion( lower = ctx->expression()[0]->accept(this); upper = ctx->expression()[1]->accept(this); } + if (lower && edge_type == EdgeAtom::Type::WEIGHTED_SHORTEST_PATH) + throw SemanticException("Lower bound is not allowed in wShortest."); - return std::make_tuple(is_bfs, lower, upper); + return std::make_tuple(edge_type, lower, upper); } antlrcpp::Any CypherMainVisitor::visitExpression( diff --git a/src/query/frontend/ast/cypher_main_visitor.hpp b/src/query/frontend/ast/cypher_main_visitor.hpp index 90e8d70ba..2404b9efc 100644 --- a/src/query/frontend/ast/cypher_main_visitor.hpp +++ b/src/query/frontend/ast/cypher_main_visitor.hpp @@ -296,7 +296,7 @@ class CypherMainVisitor : public antlropencypher::CypherBaseVisitor { CypherParser::RelationshipTypesContext *ctx) override; /** - * @return std::tuple. + * @return std::tuple. */ antlrcpp::Any visitVariableExpansion( CypherParser::VariableExpansionContext *ctx) override; diff --git a/src/query/frontend/opencypher/grammar/Cypher.g4 b/src/query/frontend/opencypher/grammar/Cypher.g4 index 63b5cb3df..8737f4ecf 100644 --- a/src/query/frontend/opencypher/grammar/Cypher.g4 +++ b/src/query/frontend/opencypher/grammar/Cypher.g4 @@ -124,12 +124,12 @@ relationshipPattern : ( leftArrowHead SP? dash SP? ( relationshipDetail )? SP? d ; relationshipDetail : '[' SP? ( variable SP? )? ( relationshipTypes SP? )? ( variableExpansion SP? )? properties SP? ']' - | '[' SP? ( variable SP? )? ( relationshipTypes SP? )? ( variableExpansion SP? )? relationshipLambda SP? ']' + | '[' SP? ( variable SP? )? ( relationshipTypes SP? )? ( variableExpansion SP? )? relationshipLambda SP? (relationshipLambda SP?)? ']' | '[' SP? ( variable SP? )? ( relationshipTypes SP? )? ( variableExpansion SP? )? ( (properties SP?) | (relationshipLambda SP?) )* ']'; relationshipLambda: '(' SP? traversed_edge=variable SP? ',' SP? traversed_node=variable SP? '|' SP? expression SP? ')'; -variableExpansion : '*' SP? (BFS)? SP? ( expression SP? )? ( '..' ( SP? expression )? )? ; +variableExpansion : '*' SP? (BFS | WSHORTEST)? SP? ( expression SP? )? ( '..' ( SP? expression )? )? ; properties : mapLiteral | parameter @@ -482,6 +482,8 @@ INDEX : ( 'I' | 'i') ( 'N' | 'n' ) ( 'D' | 'd' ) ( 'E' | 'e' ) ( 'X' | 'x' ) ; BFS : ( 'B' | 'b' ) ( 'F' | 'f' ) ( 'S' | 's' ) ; +WSHORTEST : ( 'W' | 'w' ) ( 'S' | 's' ) ( 'H' | 'h' ) ( 'O' | 'o' ) ( 'R' | 'r' ) ( 'T' | 't' ) ( 'E' | 'e' ) ( 'S' | 's' ) ( 'T' | 't' ) ; + UnescapedSymbolicName : IdentifierStart ( IdentifierPart )* ; /** diff --git a/src/query/frontend/semantic/symbol_generator.cpp b/src/query/frontend/semantic/symbol_generator.cpp index 3da15d266..5413aedfe 100644 --- a/src/query/frontend/semantic/symbol_generator.cpp +++ b/src/query/frontend/semantic/symbol_generator.cpp @@ -419,18 +419,24 @@ bool SymbolGenerator::PreVisit(EdgeAtom &edge_atom) { } scope_.in_edge_range = false; scope_.in_pattern = false; - if (edge_atom.filter_expression_) { - VisitWithIdentifiers(*edge_atom.filter_expression_, - {edge_atom.inner_edge_, edge_atom.inner_node_}); + if (edge_atom.filter_lambda_.expression) { + VisitWithIdentifiers(*edge_atom.filter_lambda_.expression, + {edge_atom.filter_lambda_.inner_edge, + edge_atom.filter_lambda_.inner_node}); } else { // Create inner symbols, but don't bind them in scope, since they are to // be used in the missing filter expression. - symbol_table_[*edge_atom.inner_edge_] = symbol_table_.CreateSymbol( - edge_atom.inner_edge_->name_, edge_atom.inner_edge_->user_declared_, - Symbol::Type::Edge); - symbol_table_[*edge_atom.inner_node_] = symbol_table_.CreateSymbol( - edge_atom.inner_node_->name_, edge_atom.inner_node_->user_declared_, - Symbol::Type::Vertex); + const auto *inner_edge = edge_atom.filter_lambda_.inner_edge; + symbol_table_[*inner_edge] = symbol_table_.CreateSymbol( + inner_edge->name_, inner_edge->user_declared_, Symbol::Type::Edge); + const auto *inner_node = edge_atom.filter_lambda_.inner_node; + symbol_table_[*inner_node] = symbol_table_.CreateSymbol( + inner_node->name_, inner_node->user_declared_, Symbol::Type::Vertex); + } + if (edge_atom.weight_lambda_.expression) { + VisitWithIdentifiers(*edge_atom.weight_lambda_.expression, + {edge_atom.weight_lambda_.inner_edge, + edge_atom.weight_lambda_.inner_node}); } scope_.in_pattern = true; } diff --git a/src/query/plan/preprocess.cpp b/src/query/plan/preprocess.cpp index 35b54efe9..52cc5c84d 100644 --- a/src/query/plan/preprocess.cpp +++ b/src/query/plan/preprocess.cpp @@ -44,13 +44,22 @@ std::vector NormalizePatterns( auto collect_expansion = [&](auto *prev_node, auto *edge, auto *current_node) { UsedSymbolsCollector collector(symbol_table); - // Remove symbols which are bound by variable expansions. if (edge->IsVariable()) { if (edge->lower_bound_) edge->lower_bound_->Accept(collector); if (edge->upper_bound_) edge->upper_bound_->Accept(collector); - collector.symbols_.erase(symbol_table.at(*edge->inner_edge_)); - collector.symbols_.erase(symbol_table.at(*edge->inner_node_)); - if (edge->filter_expression_) edge->filter_expression_->Accept(collector); + if (edge->filter_lambda_.expression) + edge->filter_lambda_.expression->Accept(collector); + // Remove symbols which are bound by lambda arguments. + collector.symbols_.erase( + symbol_table.at(*edge->filter_lambda_.inner_edge)); + collector.symbols_.erase( + symbol_table.at(*edge->filter_lambda_.inner_node)); + if (edge->type_ == EdgeAtom::Type::WEIGHTED_SHORTEST_PATH) { + collector.symbols_.erase( + symbol_table.at(*edge->weight_lambda_.inner_edge)); + collector.symbols_.erase( + symbol_table.at(*edge->weight_lambda_.inner_node)); + } } expansions.emplace_back(Expansion{prev_node, edge, edge->direction_, false, collector.symbols_, current_node}); @@ -223,11 +232,13 @@ void Filters::CollectPatternFilters(Pattern &pattern, SymbolTable &symbol_table, { collector.symbols_.clear(); prop_pair.second->Accept(collector); - collector.symbols_.emplace(symbol_table.at(*atom->inner_node_)); - collector.symbols_.emplace(symbol_table.at(*atom->inner_edge_)); + collector.symbols_.emplace( + symbol_table.at(*atom->filter_lambda_.inner_node)); + collector.symbols_.emplace( + symbol_table.at(*atom->filter_lambda_.inner_edge)); // First handle the inline property filter. - auto *property_lookup = - storage.Create(atom->inner_edge_, prop_pair.first); + auto *property_lookup = storage.Create( + atom->filter_lambda_.inner_edge, prop_pair.first); auto *prop_equal = storage.Create(property_lookup, prop_pair.second); // Currently, variable expand has no gains if we set PropertyFilter. diff --git a/src/query/plan/rule_based_planner.hpp b/src/query/plan/rule_based_planner.hpp index 6fae68f64..199ea7380 100644 --- a/src/query/plan/rule_based_planner.hpp +++ b/src/query/plan/rule_based_planner.hpp @@ -352,8 +352,10 @@ class RuleBasedPlanner { DCHECK(!utils::Contains(bound_symbols, edge_symbol)) << "Existing edges are not supported"; if (edge->IsVariable()) { - Symbol inner_edge_symbol = symbol_table.at(*edge->inner_edge_); - Symbol inner_node_symbol = symbol_table.at(*edge->inner_node_); + Symbol inner_edge_symbol = + symbol_table.at(*edge->filter_lambda_.inner_edge); + Symbol inner_node_symbol = + symbol_table.at(*edge->filter_lambda_.inner_node); { // Bind the inner edge and node symbols so they're available for // inline filtering in ExpandVariable. @@ -369,7 +371,7 @@ class RuleBasedPlanner { // lambda filtering uses bound symbols. auto *filter_expr = impl::BoolJoin( storage, impl::ExtractFilters(bound_symbols, filters, storage), - edge->filter_expression_); + edge->filter_lambda_.expression); // At this point it's possible we have leftover filters for inline // filtering (they use the inner symbols. If they were not collected, // we have to remove them manually because no other filter-extraction @@ -386,6 +388,7 @@ class RuleBasedPlanner { bound_symbols.erase(inner_edge_symbol); bound_symbols.erase(inner_node_symbol); + // TODO: Pass weight lambda. last_op = std::make_unique( node_symbol, edge_symbol, edge->type_, expansion.direction, edge->edge_types_, expansion.is_flipped, edge->lower_bound_, diff --git a/tests/unit/cypher_main_visitor.cpp b/tests/unit/cypher_main_visitor.cpp index 7cc9aa31c..ce237e40e 100644 --- a/tests/unit/cypher_main_visitor.cpp +++ b/tests/unit/cypher_main_visitor.cpp @@ -1604,12 +1604,12 @@ TYPED_TEST(CypherMainVisitorTest, MatchBfsReturn) { UnorderedElementsAre(ast_generator.db_accessor_.EdgeType("type1"), ast_generator.db_accessor_.EdgeType("type2"))); EXPECT_EQ(bfs->identifier_->name_, "r"); - EXPECT_EQ(bfs->inner_edge_->name_, "e"); - EXPECT_TRUE(bfs->inner_edge_->user_declared_); - EXPECT_EQ(bfs->inner_node_->name_, "n"); - EXPECT_TRUE(bfs->inner_node_->user_declared_); + EXPECT_EQ(bfs->filter_lambda_.inner_edge->name_, "e"); + EXPECT_TRUE(bfs->filter_lambda_.inner_edge->user_declared_); + EXPECT_EQ(bfs->filter_lambda_.inner_node->name_, "n"); + EXPECT_TRUE(bfs->filter_lambda_.inner_node->user_declared_); CheckLiteral(ast_generator.context_, bfs->upper_bound_, 10); - auto *eq = dynamic_cast(bfs->filter_expression_); + auto *eq = dynamic_cast(bfs->filter_lambda_.expression); ASSERT_TRUE(eq); } @@ -1626,8 +1626,94 @@ TYPED_TEST(CypherMainVisitorTest, MatchVariableLambdaSymbols) { auto *var_expand = dynamic_cast(match->patterns_[0]->atoms_[1]); ASSERT_TRUE(var_expand); ASSERT_TRUE(var_expand->IsVariable()); - EXPECT_FALSE(var_expand->inner_edge_->user_declared_); - EXPECT_FALSE(var_expand->inner_node_->user_declared_); + EXPECT_FALSE(var_expand->filter_lambda_.inner_edge->user_declared_); + EXPECT_FALSE(var_expand->filter_lambda_.inner_node->user_declared_); +} + +TYPED_TEST(CypherMainVisitorTest, MatchWShortestReturn) { + TypeParam ast_generator( + "MATCH ()-[r:type1|type2 *wShortest 10 (we, wn | 42) (e, n | true)]->() " + "RETURN r"); + auto *query = ast_generator.query_; + ASSERT_TRUE(query->single_query_); + auto *single_query = query->single_query_; + ASSERT_EQ(single_query->clauses_.size(), 2U); + auto *match = dynamic_cast(single_query->clauses_[0]); + ASSERT_TRUE(match); + ASSERT_EQ(match->patterns_.size(), 1U); + ASSERT_EQ(match->patterns_[0]->atoms_.size(), 3U); + auto *shortest = dynamic_cast(match->patterns_[0]->atoms_[1]); + ASSERT_TRUE(shortest); + EXPECT_TRUE(shortest->IsVariable()); + EXPECT_EQ(shortest->type_, EdgeAtom::Type::WEIGHTED_SHORTEST_PATH); + EXPECT_EQ(shortest->direction_, EdgeAtom::Direction::OUT); + EXPECT_THAT( + shortest->edge_types_, + UnorderedElementsAre(ast_generator.db_accessor_.EdgeType("type1"), + ast_generator.db_accessor_.EdgeType("type2"))); + CheckLiteral(ast_generator.context_, shortest->upper_bound_, 10); + EXPECT_FALSE(shortest->lower_bound_); + EXPECT_EQ(shortest->identifier_->name_, "r"); + EXPECT_EQ(shortest->filter_lambda_.inner_edge->name_, "e"); + EXPECT_TRUE(shortest->filter_lambda_.inner_edge->user_declared_); + EXPECT_EQ(shortest->filter_lambda_.inner_node->name_, "n"); + EXPECT_TRUE(shortest->filter_lambda_.inner_node->user_declared_); + CheckLiteral(ast_generator.context_, shortest->filter_lambda_.expression, + true); + EXPECT_EQ(shortest->weight_lambda_.inner_edge->name_, "we"); + EXPECT_TRUE(shortest->weight_lambda_.inner_edge->user_declared_); + EXPECT_EQ(shortest->weight_lambda_.inner_node->name_, "wn"); + EXPECT_TRUE(shortest->weight_lambda_.inner_node->user_declared_); + CheckLiteral(ast_generator.context_, shortest->weight_lambda_.expression, 42); +} + +TYPED_TEST(CypherMainVisitorTest, MatchWShortestNoFilterReturn) { + TypeParam ast_generator( + "MATCH ()-[r:type1|type2 *wShortest 10 (we, wn | 42)]->() " + "RETURN r"); + auto *query = ast_generator.query_; + ASSERT_TRUE(query->single_query_); + auto *single_query = query->single_query_; + ASSERT_EQ(single_query->clauses_.size(), 2U); + auto *match = dynamic_cast(single_query->clauses_[0]); + ASSERT_TRUE(match); + ASSERT_EQ(match->patterns_.size(), 1U); + ASSERT_EQ(match->patterns_[0]->atoms_.size(), 3U); + auto *shortest = dynamic_cast(match->patterns_[0]->atoms_[1]); + ASSERT_TRUE(shortest); + EXPECT_TRUE(shortest->IsVariable()); + EXPECT_EQ(shortest->type_, EdgeAtom::Type::WEIGHTED_SHORTEST_PATH); + EXPECT_EQ(shortest->direction_, EdgeAtom::Direction::OUT); + EXPECT_THAT( + shortest->edge_types_, + UnorderedElementsAre(ast_generator.db_accessor_.EdgeType("type1"), + ast_generator.db_accessor_.EdgeType("type2"))); + CheckLiteral(ast_generator.context_, shortest->upper_bound_, 10); + EXPECT_FALSE(shortest->lower_bound_); + EXPECT_EQ(shortest->identifier_->name_, "r"); + EXPECT_FALSE(shortest->filter_lambda_.expression); + EXPECT_FALSE(shortest->filter_lambda_.inner_edge->user_declared_); + EXPECT_FALSE(shortest->filter_lambda_.inner_node->user_declared_); + EXPECT_EQ(shortest->weight_lambda_.inner_edge->name_, "we"); + EXPECT_TRUE(shortest->weight_lambda_.inner_edge->user_declared_); + EXPECT_EQ(shortest->weight_lambda_.inner_node->name_, "wn"); + EXPECT_TRUE(shortest->weight_lambda_.inner_node->user_declared_); + CheckLiteral(ast_generator.context_, shortest->weight_lambda_.expression, 42); +} + + +TYPED_TEST(CypherMainVisitorTest, SemanticExceptionOnWShortestLowerBound) { + ASSERT_THROW( + TypeParam("MATCH ()-[r *wShortest 10.. (e, n | 42)]-() RETURN r"), + SemanticException); + ASSERT_THROW( + TypeParam("MATCH ()-[r *wShortest 10..20 (e, n | 42)]-() RETURN r"), + SemanticException); +} + +TYPED_TEST(CypherMainVisitorTest, SemanticExceptionOnWShortestWithoutLambda) { + ASSERT_THROW(TypeParam("MATCH ()-[r *wShortest]-() RETURN r"), + SemanticException); } TYPED_TEST(CypherMainVisitorTest, SemanticExceptionOnUnionTypeMix) { diff --git a/tests/unit/distributed_data_exchange.cpp b/tests/unit/distributed_data_exchange.cpp index e9d29e08b..dd8689adb 100644 --- a/tests/unit/distributed_data_exchange.cpp +++ b/tests/unit/distributed_data_exchange.cpp @@ -100,7 +100,7 @@ TEST_F(DistributedGraphDbTest, RemoteExpansion) { GraphDbAccessor dba{master()}; std::vector visited; - auto expand = [&visited, &dba](auto &v) { + auto expand = [](auto &v) { for (auto e : v.out()) return e.to(); for (auto e : v.in()) return e.from(); CHECK(false) << "No edge in vertex"; diff --git a/tests/unit/query_common.hpp b/tests/unit/query_common.hpp index d4cafade0..87254d262 100644 --- a/tests/unit/query_common.hpp +++ b/tests/unit/query_common.hpp @@ -160,10 +160,10 @@ auto GetEdgeVariable(AstTreeStorage &storage, const std::string &name, auto r_val = storage.Create(storage.Create(name), EdgeAtom::Type::DEPTH_FIRST, dir, edge_types); - r_val->inner_edge_ = + r_val->filter_lambda_.inner_edge = inner_edge ? inner_edge : storage.Create(utils::RandomString(20)); - r_val->inner_node_ = + r_val->filter_lambda_.inner_node = inner_node ? inner_node : storage.Create(utils::RandomString(20)); return r_val; diff --git a/tests/unit/query_planner.cpp b/tests/unit/query_planner.cpp index 10ab2e7b2..3f8f2c058 100644 --- a/tests/unit/query_planner.cpp +++ b/tests/unit/query_planner.cpp @@ -1851,9 +1851,9 @@ TYPED_TEST(TestPlanner, MatchBreadthFirst) { auto *bfs = storage.Create( IDENT("r"), query::EdgeAtom::Type::BREADTH_FIRST, Direction::OUT, std::vector{edge_type}); - bfs->inner_edge_ = IDENT("r"); - bfs->inner_node_ = IDENT("n"); - bfs->filter_expression_ = IDENT("n"); + bfs->filter_lambda_.inner_edge = IDENT("r"); + bfs->filter_lambda_.inner_node = IDENT("n"); + bfs->filter_lambda_.expression = IDENT("n"); bfs->upper_bound_ = LITERAL(10); QUERY(SINGLE_QUERY(MATCH(PATTERN(NODE("n"), bfs, NODE("m"))), RETURN("r"))); CheckPlan(storage, ExpectScanAll(), ExpectExpandBreadthFirst(), @@ -1929,9 +1929,9 @@ TYPED_TEST(TestPlanner, ReturnAsteriskOmitsLambdaSymbols) { database::SingleNode db; AstTreeStorage storage; auto edge = EDGE_VARIABLE("r", Direction::BOTH); - edge->inner_edge_ = IDENT("ie"); - edge->inner_node_ = IDENT("in"); - edge->filter_expression_ = LITERAL(true); + edge->filter_lambda_.inner_edge = IDENT("ie"); + edge->filter_lambda_.inner_node = IDENT("in"); + edge->filter_lambda_.expression = LITERAL(true); auto ret = storage.Create(); ret->body_.all_identifiers = true; QUERY(SINGLE_QUERY(MATCH(PATTERN(NODE("n"), edge, NODE("m"))), ret)); diff --git a/tests/unit/query_semantic.cpp b/tests/unit/query_semantic.cpp index 97f5871eb..b0581bc53 100644 --- a/tests/unit/query_semantic.cpp +++ b/tests/unit/query_semantic.cpp @@ -834,9 +834,9 @@ TEST_F(TestSymbolGenerator, MatchBfsReturn) { auto *bfs = storage.Create( IDENT("r"), EdgeAtom::Type::BREADTH_FIRST, EdgeAtom::Direction::OUT, std::vector{}); - bfs->inner_edge_ = IDENT("r"); - bfs->inner_node_ = IDENT("n"); - bfs->filter_expression_ = r_prop; + bfs->filter_lambda_.inner_edge = IDENT("r"); + bfs->filter_lambda_.inner_node = IDENT("n"); + bfs->filter_lambda_.expression = r_prop; bfs->upper_bound_ = n_prop; auto *ret_r = IDENT("r"); auto *query = QUERY(SINGLE_QUERY(MATCH(PATTERN(node_n, bfs, NODE("m"))), @@ -845,13 +845,14 @@ TEST_F(TestSymbolGenerator, MatchBfsReturn) { // Symbols for pattern, `n`, `[r]`, `r|`, `n|`, `m` and `AS r`. EXPECT_EQ(symbol_table.max_position(), 7); EXPECT_EQ(symbol_table.at(*ret_r), symbol_table.at(*bfs->identifier_)); - EXPECT_NE(symbol_table.at(*ret_r), symbol_table.at(*bfs->inner_edge_)); - EXPECT_TRUE(symbol_table.at(*bfs->inner_edge_).user_declared()); - EXPECT_EQ(symbol_table.at(*bfs->inner_edge_), + EXPECT_NE(symbol_table.at(*ret_r), + symbol_table.at(*bfs->filter_lambda_.inner_edge)); + EXPECT_TRUE(symbol_table.at(*bfs->filter_lambda_.inner_edge).user_declared()); + EXPECT_EQ(symbol_table.at(*bfs->filter_lambda_.inner_edge), symbol_table.at(*r_prop->expression_)); EXPECT_NE(symbol_table.at(*node_n->identifier_), - symbol_table.at(*bfs->inner_node_)); - EXPECT_TRUE(symbol_table.at(*bfs->inner_node_).user_declared()); + symbol_table.at(*bfs->filter_lambda_.inner_node)); + EXPECT_TRUE(symbol_table.at(*bfs->filter_lambda_.inner_node).user_declared()); EXPECT_EQ(symbol_table.at(*node_n->identifier_), symbol_table.at(*n_prop->expression_)); } @@ -860,9 +861,9 @@ TEST_F(TestSymbolGenerator, MatchBfsUsesEdgeSymbolError) { // Test MATCH (n) -[r *bfs..10 (e, n | r)]-> (m) RETURN r auto *bfs = storage.Create( IDENT("r"), EdgeAtom::Type::BREADTH_FIRST, EdgeAtom::Direction::OUT); - bfs->inner_edge_ = IDENT("e"); - bfs->inner_node_ = IDENT("n"); - bfs->filter_expression_ = IDENT("r"); + bfs->filter_lambda_.inner_edge = IDENT("e"); + bfs->filter_lambda_.inner_node = IDENT("n"); + bfs->filter_lambda_.expression = IDENT("r"); bfs->upper_bound_ = LITERAL(10); auto *query = QUERY( SINGLE_QUERY(MATCH(PATTERN(NODE("n"), bfs, NODE("m"))), RETURN("r"))); @@ -874,24 +875,24 @@ TEST_F(TestSymbolGenerator, MatchBfsUsesPreviousOuterSymbol) { auto *node_a = NODE("a"); auto *bfs = storage.Create( IDENT("r"), EdgeAtom::Type::BREADTH_FIRST, EdgeAtom::Direction::OUT); - bfs->inner_edge_ = IDENT("e"); - bfs->inner_node_ = IDENT("n"); - bfs->filter_expression_ = IDENT("a"); + bfs->filter_lambda_.inner_edge = IDENT("e"); + bfs->filter_lambda_.inner_node = IDENT("n"); + bfs->filter_lambda_.expression = IDENT("a"); bfs->upper_bound_ = LITERAL(10); auto *query = QUERY(SINGLE_QUERY(MATCH(PATTERN(node_a, bfs, NODE("m"))), RETURN("r"))); query->Accept(symbol_generator); EXPECT_EQ(symbol_table.at(*node_a->identifier_), - symbol_table.at(*bfs->filter_expression_)); + symbol_table.at(*bfs->filter_lambda_.expression)); } TEST_F(TestSymbolGenerator, MatchBfsUsesLaterSymbolError) { // Test MATCH (n) -[r *bfs..10 (e, n | m)]-> (m) RETURN r auto *bfs = storage.Create( IDENT("r"), EdgeAtom::Type::BREADTH_FIRST, EdgeAtom::Direction::OUT); - bfs->inner_edge_ = IDENT("e"); - bfs->inner_node_ = IDENT("n"); - bfs->filter_expression_ = IDENT("m"); + bfs->filter_lambda_.inner_edge = IDENT("e"); + bfs->filter_lambda_.inner_node = IDENT("n"); + bfs->filter_lambda_.expression = IDENT("m"); bfs->upper_bound_ = LITERAL(10); auto *query = QUERY( SINGLE_QUERY(MATCH(PATTERN(NODE("n"), bfs, NODE("m"))), RETURN("r"))); @@ -905,8 +906,10 @@ TEST_F(TestSymbolGenerator, MatchVariableLambdaSymbols) { auto edge = storage.Create( storage.Create("anon_r", false), EdgeAtom::Type::DEPTH_FIRST, EdgeAtom::Direction::BOTH); - edge->inner_edge_ = storage.Create("anon_inner_e", false); - edge->inner_node_ = storage.Create("anon_inner_n", false); + edge->filter_lambda_.inner_edge = + storage.Create("anon_inner_e", false); + edge->filter_lambda_.inner_node = + storage.Create("anon_inner_n", false); auto end_node = storage.Create(storage.Create("anon_end", false)); auto query = QUERY(SINGLE_QUERY(MATCH(PATTERN(node, edge, end_node)), @@ -926,6 +929,52 @@ TEST_F(TestSymbolGenerator, MatchVariableLambdaSymbols) { } } +TEST_F(TestSymbolGenerator, MatchWShortestReturn) { + // Test MATCH (n) -[r *wShortest (r, n | r.weight) (r, n | r.filter)]-> (m) + // RETURN r AS r + auto weight = dba.Property("weight"); + auto filter = dba.Property("filter"); + auto *node_n = NODE("n"); + auto *r_weight = PROPERTY_LOOKUP("r", weight); + auto *r_filter = PROPERTY_LOOKUP("r", filter); + auto *shortest = storage.Create( + IDENT("r"), EdgeAtom::Type::WEIGHTED_SHORTEST_PATH, + EdgeAtom::Direction::OUT, std::vector{}); + { + shortest->weight_lambda_.inner_edge = IDENT("r"); + shortest->weight_lambda_.inner_node = IDENT("n"); + shortest->weight_lambda_.expression = r_weight; + } + { + shortest->filter_lambda_.inner_edge = IDENT("r"); + shortest->filter_lambda_.inner_node = IDENT("n"); + shortest->filter_lambda_.expression = r_filter; + } + auto *ret_r = IDENT("r"); + auto *query = QUERY(SINGLE_QUERY(MATCH(PATTERN(node_n, shortest, NODE("m"))), + RETURN(ret_r, AS("r")))); + query->Accept(symbol_generator); + // Symbols for pattern, `n`, `[r]`, (`r|`, `n|`)x2, `m` and `AS r`. + EXPECT_EQ(symbol_table.max_position(), 9); + EXPECT_EQ(symbol_table.at(*ret_r), symbol_table.at(*shortest->identifier_)); + EXPECT_NE(symbol_table.at(*ret_r), + symbol_table.at(*shortest->weight_lambda_.inner_edge)); + EXPECT_NE(symbol_table.at(*ret_r), + symbol_table.at(*shortest->filter_lambda_.inner_edge)); + EXPECT_TRUE( + symbol_table.at(*shortest->filter_lambda_.inner_edge).user_declared()); + EXPECT_EQ(symbol_table.at(*shortest->weight_lambda_.inner_edge), + symbol_table.at(*r_weight->expression_)); + EXPECT_NE(symbol_table.at(*shortest->weight_lambda_.inner_edge), + symbol_table.at(*shortest->filter_lambda_.inner_edge)); + EXPECT_NE(symbol_table.at(*shortest->weight_lambda_.inner_node), + symbol_table.at(*shortest->filter_lambda_.inner_node)); + EXPECT_EQ(symbol_table.at(*shortest->filter_lambda_.inner_edge), + symbol_table.at(*r_filter->expression_)); + EXPECT_TRUE( + symbol_table.at(*shortest->filter_lambda_.inner_node).user_declared()); +} + TEST_F(TestSymbolGenerator, MatchUnionSymbols) { // RETURN 5 as X UNION RETURN 6 AS x auto query = QUERY(SINGLE_QUERY(RETURN(LITERAL(5), AS("X"))), diff --git a/tests/unit/query_variable_start_planner.cpp b/tests/unit/query_variable_start_planner.cpp index cfc13cc6e..73ff66638 100644 --- a/tests/unit/query_variable_start_planner.cpp +++ b/tests/unit/query_variable_start_planner.cpp @@ -307,9 +307,9 @@ TEST(TestVariableStartPlanner, MatchBfs) { auto *bfs = storage.Create( IDENT("r"), EdgeAtom::Type::BREADTH_FIRST, Direction::OUT, std::vector{}); - bfs->inner_edge_ = IDENT("r"); - bfs->inner_node_ = IDENT("n"); - bfs->filter_expression_ = NEQ(PROPERTY_LOOKUP("n", id), LITERAL(3)); + bfs->filter_lambda_.inner_edge = IDENT("r"); + bfs->filter_lambda_.inner_node = IDENT("n"); + bfs->filter_lambda_.expression = NEQ(PROPERTY_LOOKUP("n", id), LITERAL(3)); bfs->upper_bound_ = LITERAL(10); QUERY(SINGLE_QUERY(MATCH(PATTERN(NODE("n"), bfs, NODE("m"))), RETURN("r"))); // We expect to get a single column with the following rows: