diff --git a/CHANGELOG.md b/CHANGELOG.md index dfa931945..cc98ebb0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,17 +5,18 @@ ### Major Features and Improvements * CASE construct (without aggregations). -* `rand` function added. +* Named path support added. * Maps can now be stored as vertex/edge properties. -* `collect` aggregation now supports Map collection. * Map indexing supported. +* `rand` function added. * `assert` function added. -* Use \u to specify 4 digit codepoint and \U for 8 digit * `counter` and `counterSet` functions added. * `indexInfo` function added. +* `collect` aggregation now supports Map collection. ### Bug Fixes and Other Changes +* Use \u to specify 4 digit codepoint and \U for 8 digit * Keywords appearing in header (named expressions) keep original case. * Our Bolt protocol implementation is now completely compatible with the protocol version 1 specification. (https://boltprotocol.org/v1/) diff --git a/docs/user_technical/open-cypher.md b/docs/user_technical/open-cypher.md index 0488e3325..03e87a4dd 100644 --- a/docs/user_technical/open-cypher.md +++ b/docs/user_technical/open-cypher.md @@ -51,17 +51,22 @@ There are cases when a user needs to find data which is connected by traversing a path of connections, but the user doesn't know how many connections need to be traversed. openCypher allows for designating patterns with *variable path lengths*. Matching such a path is achieved by using the -`*` (*asterisk*) symbol inside the pattern for a connection. For example, +`*` (*asterisk*) symbol inside the edge element of a pattern. For example, traversing from `node1` to `node2` by following any number of connections in a single direction can be achieved with: - MATCH (node1) -[*]-> (node2) + MATCH (node1) -[r*]-> (node2) RETURN node1, r, node2 If paths are very long, finding them could take a long time. To prevent that, a user can provide the minimum and maximum length of the path. For example, paths of length between 2 and 4 can be obtained with a query like: - MATCH (node1) -[*2..4]-> (node2) + MATCH (node1) -[r*2..4]-> (node2) RETURN node1, r, node2 + +It is possible to name patterns in the query and return the resulting paths. +This is especially useful when matching variable length paths: + + MATCH path = () -[r*2..4]-> () RETURN path More details on how `MATCH` works can be found [here](https://neo4j.com/docs/developer-manual/current/cypher/clauses/match/). @@ -467,7 +472,7 @@ functions. `head` | Returns the first element of a list. `last` | Returns the last element of a list. `properties` | Returns the properties of a node or an edge. - `size` | Returns the number of elements in a list. + `size` | Returns the number of elements in a list or a map. When given a string it returns the number of characters. When given a path it returns the number of expansions (edges) in that path. `toBoolean` | Converts the argument to a boolean. `toFloat` | Converts the argument to a floating point number. `toInteger` | Converts the argument to an integer. diff --git a/docs/user_technical/upcoming-features.md b/docs/user_technical/upcoming-features.md index a71373b4f..e9e6399f7 100644 --- a/docs/user_technical/upcoming-features.md +++ b/docs/user_technical/upcoming-features.md @@ -26,19 +26,6 @@ allow deletion of created indices. Although we have implemented the most common features of the openCypher query language, there are other useful features we are still working on. -#### Named Paths - -It would be useful to store paths that match a pattern into a variable. This -enables the user to display the matched patterns or do some other operations -on the path, like calculating the length of the path. - -The feature would be used by simply assigning the variable to a pattern. For -example: - - MATCH path = (node1) -[connection]-> (node2) - -Path naming is especially useful with the *variable length paths* feature. - #### Functions Memgraph's openCypher implementation supports the most useful functions, but diff --git a/src/query/frontend/ast/ast.hpp b/src/query/frontend/ast/ast.hpp index 3cbbccf7a..b39a0b12d 100644 --- a/src/query/frontend/ast/ast.hpp +++ b/src/query/frontend/ast/ast.hpp @@ -1125,6 +1125,7 @@ class Pattern : public Tree { DEFVISITABLE(TreeVisitor); bool Accept(HierarchicalTreeVisitor &visitor) override { if (visitor.PreVisit(*this)) { + identifier_->Accept(visitor); for (auto &part : atoms_) { if (!part->Accept(visitor)) break; } diff --git a/src/query/frontend/semantic/symbol_generator.cpp b/src/query/frontend/semantic/symbol_generator.cpp index e73f3b810..2cc6f009f 100644 --- a/src/query/frontend/semantic/symbol_generator.cpp +++ b/src/query/frontend/semantic/symbol_generator.cpp @@ -190,7 +190,12 @@ SymbolGenerator::ReturnType SymbolGenerator::Visit(Identifier &ident) { scope_.in_skip ? "SKIP" : "LIMIT"); } Symbol symbol; - if (scope_.in_pattern && scope_.in_pattern_identifier) { + if (scope_.in_pattern && !(scope_.in_node_atom || scope_.visiting_edge)) { + // If we are in the pattern, and outside of a node or an edge, the + // identifier is the pattern name. + symbol = GetOrCreateSymbol(ident.name_, ident.user_declared_, + Symbol::Type::Path); + } else if (scope_.in_pattern && scope_.in_pattern_atom_identifier) { // Patterns can bind new symbols or reference already bound. But there // are the following special cases: // 1) Patterns used to create nodes and edges cannot redeclare already @@ -221,7 +226,7 @@ SymbolGenerator::ReturnType SymbolGenerator::Visit(Identifier &ident) { } } symbol = GetOrCreateSymbol(ident.name_, ident.user_declared_, type); - } else if (scope_.in_pattern && !scope_.in_pattern_identifier && + } else if (scope_.in_pattern && !scope_.in_pattern_atom_identifier && scope_.in_match) { if (scope_.in_edge_range && scope_.visiting_edge->identifier_->name_ == ident.name_) { @@ -329,9 +334,9 @@ bool SymbolGenerator::PreVisit(NodeAtom &node_atom) { for (auto kv : node_atom.properties_) { kv.second->Accept(*this); } - scope_.in_pattern_identifier = true; + scope_.in_pattern_atom_identifier = true; node_atom.identifier_->Accept(*this); - scope_.in_pattern_identifier = false; + scope_.in_pattern_atom_identifier = false; return false; } @@ -374,9 +379,9 @@ bool SymbolGenerator::PreVisit(EdgeAtom &edge_atom) { } scope_.in_edge_range = false; } - scope_.in_pattern_identifier = true; + scope_.in_pattern_atom_identifier = true; edge_atom.identifier_->Accept(*this); - scope_.in_pattern_identifier = false; + scope_.in_pattern_atom_identifier = false; return false; } @@ -428,9 +433,9 @@ bool SymbolGenerator::PreVisit(BreadthFirstAtom &bf_atom) { scope_.in_pattern = true; // XXX: Make BFS symbol be EdgeList. bf_atom.has_range_ = true; - scope_.in_pattern_identifier = true; + scope_.in_pattern_atom_identifier = true; bf_atom.identifier_->Accept(*this); - scope_.in_pattern_identifier = false; + scope_.in_pattern_atom_identifier = false; bf_atom.has_range_ = false; return false; } diff --git a/src/query/frontend/semantic/symbol_generator.hpp b/src/query/frontend/semantic/symbol_generator.hpp index 996e8d183..32c36fa7c 100644 --- a/src/query/frontend/semantic/symbol_generator.hpp +++ b/src/query/frontend/semantic/symbol_generator.hpp @@ -83,9 +83,9 @@ class SymbolGenerator : public HierarchicalTreeVisitor { bool in_order_by{false}; bool in_where{false}; bool in_match{false}; - // True when visiting a pattern identifier, which can be reused or created - // in the pattern itself. - bool in_pattern_identifier{false}; + // True when visiting a pattern atom (node or edge) identifier, which can be + // reused or created in the pattern itself. + bool in_pattern_atom_identifier{false}; // True when visiting range bounds of a variable path. bool in_edge_range{false}; // True if the return/with contains an aggregation in any named expression. diff --git a/src/query/frontend/semantic/symbol_table.hpp b/src/query/frontend/semantic/symbol_table.hpp index 27671a810..610fefdae 100644 --- a/src/query/frontend/semantic/symbol_table.hpp +++ b/src/query/frontend/semantic/symbol_table.hpp @@ -64,6 +64,8 @@ class SymbolTable { int max_position() const { return position_; } + const auto &table() const { return table_; } + private: int position_{0}; std::map table_; diff --git a/src/query/interpret/awesome_memgraph_functions.cpp b/src/query/interpret/awesome_memgraph_functions.cpp index 00a044046..0c5d30552 100644 --- a/src/query/interpret/awesome_memgraph_functions.cpp +++ b/src/query/interpret/awesome_memgraph_functions.cpp @@ -135,6 +135,8 @@ TypedValue Size(const std::vector &args, GraphDbAccessor &) { // to do it. return static_cast( args[0].Value>().size()); + case TypedValue::Type::Path: + return static_cast(args[0].ValuePath().edges().size()); default: throw QueryRuntimeException("size called with incompatible type"); } diff --git a/src/query/path.hpp b/src/query/path.hpp new file mode 100644 index 000000000..ca925e310 --- /dev/null +++ b/src/query/path.hpp @@ -0,0 +1,81 @@ +#pragma once + +#include +#include + +#include "storage/edge_accessor.hpp" +#include "storage/vertex_accessor.hpp" +#include "utils/assert.hpp" + +namespace query { + +/** + * A data structure that holds a graph path. A path consists of at least one + * vertex, followed by zero or more edge + vertex extensions (thus having one + * vertex more then edges). + */ +class Path { + public: + /** Creates the path starting with the given vertex. */ + Path(const VertexAccessor &vertex) { Expand(vertex); } + + /** Creates the path starting with the given vertex and containing all other + * elements. */ + template + Path(const VertexAccessor &vertex, const TOthers &... others) { + Expand(vertex); + Expand(others...); + } + + /** Expands the path with the given vertex. */ + void Expand(const VertexAccessor &vertex) { + debug_assert(vertices_.size() == edges_.size(), + "Illegal path construction order"); + vertices_.emplace_back(vertex); + } + + /** Expands the path with the given edge. */ + void Expand(const EdgeAccessor &edge) { + debug_assert(vertices_.size() - 1 == edges_.size(), + "Illegal path construction order"); + edges_.emplace_back(edge); + } + + /** Expands the path with the given elements. */ + template + void Expand(const TFirst &first, const TOthers &... others) { + Expand(first); + Expand(others...); + } + + auto &vertices() { return vertices_; } + auto &edges() { return edges_; } + const auto &vertices() const { return vertices_; } + const auto &edges() const { return edges_; } + + bool operator==(const Path &other) const { + return vertices_ == other.vertices_ && edges_ == other.edges_; + } + + friend std::ostream &operator<<(std::ostream &os, const Path &path) { + debug_assert(path.vertices_.size() > 0U, + "Attempting to stream out an invalid path"); + os << path.vertices_[0]; + for (int i = 0; i < static_cast(path.edges_.size()); i++) { + bool arrow_to_left = path.vertices_[i] == path.edges_[i].to(); + if (arrow_to_left) os << "<"; + os << "-" << path.edges_[i] << "-"; + if (!arrow_to_left) os << ">"; + os << path.vertices_[i + 1]; + } + + return os; + } + + private: + // Contains all the vertices in the path. + std::vector vertices_; + // Contains all the edges in the path (one less then there are vertices). + std::vector edges_; +}; +} // namespace query diff --git a/src/query/plan/cost_estimator.hpp b/src/query/plan/cost_estimator.hpp index 30e197b7d..f7cbf787c 100644 --- a/src/query/plan/cost_estimator.hpp +++ b/src/query/plan/cost_estimator.hpp @@ -223,11 +223,12 @@ class CostEstimator : public HierarchicalLogicalOperatorVisitor { std::experimental::optional ConstPropertyValue( const Expression *expression) { if (auto *literal = dynamic_cast(expression)) { - if (literal->value_.IsPropertyValue()) return literal->value_; + if (literal->value_.IsPropertyValue()) + return static_cast(literal->value_); } else if (auto *param_lookup = dynamic_cast(expression)) { auto value = parameters.AtTokenPosition(param_lookup->token_position_); - if (value.IsPropertyValue()) return value; + if (value.IsPropertyValue()) return static_cast(value); } return std::experimental::nullopt; } diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index f8c229404..41ca4779d 100644 --- a/src/query/plan/operator.cpp +++ b/src/query/plan/operator.cpp @@ -313,10 +313,10 @@ std::unique_ptr ScanAllByLabelPropertyRange::MakeCursor( context.symbol_table_, db, graph_view_); auto convert = [&evaluator](const auto &bound) -> std::experimental::optional> { - if (!bound) return std::experimental::nullopt; - return std::experimental::make_optional(utils::Bound( - bound.value().value()->Accept(evaluator), bound.value().type())); - }; + if (!bound) return std::experimental::nullopt; + return std::experimental::make_optional(utils::Bound( + bound.value().value()->Accept(evaluator), bound.value().type())); + }; return db.Vertices(label_, property_, convert(lower_bound()), convert(upper_bound()), graph_view_ == GraphView::NEW); }; @@ -577,11 +577,6 @@ bool Expand::ExpandCursor::InitEdges(Frame &frame, Context &context) { out_edges_it_.emplace(out_edges_->begin()); } - // TODO add support for Front and Back expansion (when QueryPlanner - // will need it). For now only Back expansion (left to right) is - // supported - // TODO add support for named paths - return true; } } @@ -1132,6 +1127,88 @@ void ExpandBreadthFirst::Cursor::Reset() { to_visit_current_.clear(); } +class ConstructNamedPathCursor : public Cursor { + public: + ConstructNamedPathCursor(const ConstructNamedPath &self, GraphDbAccessor &db) + : self_(self), input_cursor_(self_.input()->MakeCursor(db)) {} + + bool Pull(Frame &frame, Context &context) override { + if (!input_cursor_->Pull(frame, context)) return false; + + auto symbol_it = self_.path_elements().begin(); + debug_assert(symbol_it != self_.path_elements().end(), + "Named path must contain at least one node"); + + TypedValue start_vertex = frame[*symbol_it++]; + + // In an OPTIONAL MATCH everything could be Null. + if (start_vertex.IsNull()) { + frame[self_.path_symbol()] = TypedValue::Null; + return true; + } + + debug_assert(start_vertex.IsVertex(), + "First named path element must be a vertex"); + query::Path path(start_vertex.ValueVertex()); + + // If the last path element symbol was for an edge list, then + // the next symbol is a vertex and it should not append to the path because + // expansion already did it. + bool last_was_edge_list = false; + + for (; symbol_it != self_.path_elements().end(); symbol_it++) { + TypedValue expansion = frame[*symbol_it]; + // We can have Null (OPTIONAL MATCH), a vertex, an edge, or an edge + // list (variable expand or BFS). + switch (expansion.type()) { + case TypedValue::Type::Null: + frame[self_.path_symbol()] = TypedValue::Null; + return true; + case TypedValue::Type::Vertex: + if (!last_was_edge_list) path.Expand(expansion.ValueVertex()); + last_was_edge_list = false; + break; + case TypedValue::Type::Edge: + path.Expand(expansion.ValueEdge()); + break; + case TypedValue::Type::List: { + last_was_edge_list = true; + // We need to expand all edges in the list and intermediary vertices. + const std::vector &edges = expansion.ValueList(); + for (const auto &edge_value : edges) { + const EdgeAccessor &edge = edge_value.ValueEdge(); + const VertexAccessor from = edge.from(); + if (path.vertices().back() == from) + path.Expand(edge, edge.to()); + else + path.Expand(edge, from); + } + break; + } + default: + permanent_fail("Unsupported type in named path construction"); + + break; + } + } + + frame[self_.path_symbol()] = path; + return true; + } + + void Reset() override { input_cursor_->Reset(); } + + private: + const ConstructNamedPath self_; + const std::unique_ptr input_cursor_; +}; + +ACCEPT_WITH_INPUT(ConstructNamedPath) + +std::unique_ptr ConstructNamedPath::MakeCursor(GraphDbAccessor &db) { + return std::make_unique(*this, db); +} + Filter::Filter(const std::shared_ptr &input, Expression *expression) : input_(input ? input : std::make_shared()), @@ -1147,8 +1224,8 @@ Filter::FilterCursor::FilterCursor(const Filter &self, GraphDbAccessor &db) : self_(self), db_(db), input_cursor_(self_.input_->MakeCursor(db)) {} bool Filter::FilterCursor::Pull(Frame &frame, Context &context) { - // Like all filters, newly set values should not affect filtering of old nodes - // and edges. + // Like all filters, newly set values should not affect filtering of old + // nodes and edges. ExpressionEvaluator evaluator(frame, context.parameters_, context.symbol_table_, db_, GraphView::OLD); while (input_cursor_->Pull(frame, context)) { @@ -1553,8 +1630,8 @@ bool ExpandUniquenessFilter::ExpandUniquenessFilterCursor::Pull( for (const auto &previous_symbol : self_.previous_symbols_) { TypedValue &previous_value = frame[previous_symbol]; // This shouldn't raise a TypedValueException, because the planner makes - // sure these are all of the expected type. In case they are not, an error - // should be raised long before this code is executed. + // sure these are all of the expected type. In case they are not, an + // error should be raised long before this code is executed. // TODO handle possible null due to optional match if (ContainsSame(previous_value, expand_value)) return false; } @@ -1583,18 +1660,20 @@ namespace { * given TypedValue. */ void ReconstructTypedValue(TypedValue &value) { + const static std::string vertex_error_msg = + "Vertex invalid after WITH clause, (most likely deleted by a " + "preceeding DELETE clause)"; + const static std::string edge_error_msg = + "Edge invalid after WITH clause, (most likely deleted by a " + "preceeding DELETE clause)"; switch (value.type()) { case TypedValue::Type::Vertex: if (!value.Value().Reconstruct()) - throw QueryRuntimeException( - "Vertex invalid after WITH clause, (most likely deleted by a " - "preceeding DELETE clause)"); + throw QueryRuntimeException(vertex_error_msg); break; case TypedValue::Type::Edge: if (!value.Value().Reconstruct()) - throw QueryRuntimeException( - "Edge invalid after WITH clause, (most likely deleted by a " - "preceeding DELETE clause)"); + throw QueryRuntimeException(edge_error_msg); break; case TypedValue::Type::List: for (TypedValue &inner_value : value.Value>()) @@ -1605,8 +1684,10 @@ void ReconstructTypedValue(TypedValue &value) { ReconstructTypedValue(kv.second); break; case TypedValue::Type::Path: - // TODO implement path reconstruct? - throw utils::NotYetImplemented("path reconstruction"); + for (auto &vertex : value.ValuePath().vertices()) + if (vertex.Reconstruct()) throw QueryRuntimeException(vertex_error_msg); + for (auto &edge : value.ValuePath().edges()) + if (edge.Reconstruct()) throw QueryRuntimeException(edge_error_msg); default: break; } @@ -2413,8 +2494,8 @@ class CreateIndexCursor : public Cursor { // Report to the end user. did_create_ = false; throw QueryRuntimeException( - "Index building already in progress on this database. Memgraph does " - "not support concurrent index building."); + "Index building already in progress on this database. Memgraph " + "does not support concurrent index building."); } did_create_ = true; return true; diff --git a/src/query/plan/operator.hpp b/src/query/plan/operator.hpp index 9ae4ea10b..53ea1ef3f 100644 --- a/src/query/plan/operator.hpp +++ b/src/query/plan/operator.hpp @@ -18,6 +18,8 @@ #include "query/common.hpp" #include "query/exceptions.hpp" #include "query/frontend/semantic/symbol_table.hpp" +#include "query/path.hpp" +#include "query/typed_value.hpp" #include "utils/bound.hpp" #include "utils/hashing/fnv.hpp" #include "utils/visitor.hpp" @@ -67,6 +69,7 @@ class ScanAllByLabelPropertyValue; class Expand; class ExpandVariable; class ExpandBreadthFirst; +class ConstructNamedPath; class Filter; class Produce; class Delete; @@ -92,8 +95,8 @@ class CreateIndex; using LogicalOperatorCompositeVisitor = ::utils::CompositeVisitor< Once, CreateNode, CreateExpand, ScanAll, ScanAllByLabel, ScanAllByLabelPropertyRange, ScanAllByLabelPropertyValue, Expand, - ExpandVariable, ExpandBreadthFirst, Filter, Produce, Delete, SetProperty, - SetProperties, SetLabels, RemoveProperty, RemoveLabels, + ExpandVariable, ExpandBreadthFirst, ConstructNamedPath, Filter, Produce, + Delete, SetProperty, SetProperties, SetLabels, RemoveProperty, RemoveLabels, ExpandUniquenessFilter, ExpandUniquenessFilter, Accumulate, AdvanceCommand, Aggregate, Skip, Limit, OrderBy, Merge, Optional, Unwind, Distinct>; @@ -731,6 +734,30 @@ class ExpandBreadthFirst : public LogicalOperator { const GraphView graph_view_; }; +/** + * Constructs a named path from it's elements and places it on the frame. + */ +class ConstructNamedPath : public LogicalOperator { + public: + ConstructNamedPath(const std::shared_ptr &input, + Symbol path_symbol, + const std::vector &path_elements) + : input_(input), + path_symbol_(path_symbol), + path_elements_(path_elements) {} + bool Accept(HierarchicalLogicalOperatorVisitor &visitor) override; + std::unique_ptr MakeCursor(GraphDbAccessor &db) override; + + const auto &input() const { return input_; } + const auto &path_symbol() const { return path_symbol_; } + const auto &path_elements() const { return path_elements_; } + + private: + const std::shared_ptr input_; + const Symbol path_symbol_; + const std::vector path_elements_; +}; + /** * @brief Filter whose Pull returns true only when the given expression * evaluates into true. diff --git a/src/query/plan/rule_based_planner.cpp b/src/query/plan/rule_based_planner.cpp index 97e46653a..9de49c5c3 100644 --- a/src/query/plan/rule_based_planner.cpp +++ b/src/query/plan/rule_based_planner.cpp @@ -589,6 +589,13 @@ void AddMatching(const std::vector &patterns, Where *where, } for (auto *pattern : patterns) { matching.filters.CollectPatternFilters(*pattern, symbol_table, storage); + if (pattern->identifier_->user_declared_) { + std::vector path_elements; + for (auto *pattern_atom : pattern->atoms_) + path_elements.emplace_back(symbol_table.at(*pattern_atom->identifier_)); + matching.named_paths.emplace(symbol_table.at(*pattern->identifier_), + std::move(path_elements)); + } } if (where) { matching.filters.CollectWhereFilter(*where, symbol_table); @@ -624,7 +631,6 @@ Expression *ExtractFilters(const std::unordered_set &bound_symbols, } return filter_expr; } - } // namespace namespace impl { @@ -662,6 +668,30 @@ LogicalOperator *GenFilters(LogicalOperator *last_op, return last_op; } +LogicalOperator *GenNamedPaths( + LogicalOperator *last_op, std::unordered_set &bound_symbols, + std::unordered_map> &named_paths) { + auto all_are_bound = [&bound_symbols](const std::vector &syms) { + for (const auto &sym : syms) + if (bound_symbols.find(sym) == bound_symbols.end()) return false; + return true; + }; + for (auto named_path_it = named_paths.begin(); + named_path_it != named_paths.end();) { + if (all_are_bound(named_path_it->second)) { + last_op = new ConstructNamedPath( + std::shared_ptr(last_op), named_path_it->first, + std::move(named_path_it->second)); + bound_symbols.insert(named_path_it->first); + named_path_it = named_paths.erase(named_path_it); + } else { + ++named_path_it; + } + } + + return last_op; +} + LogicalOperator *GenReturn(Return &ret, LogicalOperator *input_op, SymbolTable &symbol_table, bool is_write, const std::unordered_set &bound_symbols, @@ -707,7 +737,20 @@ LogicalOperator *GenCreateForPattern( input_symbol, node_existing); }; - return ReducePattern(pattern, base, collect); + LogicalOperator *last_op = + ReducePattern(pattern, base, collect); + + // If the pattern is named, append the path constructing logical operator. + if (pattern.identifier_->user_declared_) { + std::vector path_elements; + for (const PatternAtom *atom : pattern.atoms_) + path_elements.emplace_back(symbol_table.at(*atom->identifier_)); + last_op = new ConstructNamedPath(std::shared_ptr(last_op), + symbol_table.at(*pattern.identifier_), + path_elements); + } + + return last_op; } // Generate an operator for a clause which writes to the database. If the clause diff --git a/src/query/plan/rule_based_planner.hpp b/src/query/plan/rule_based_planner.hpp index 1d064c59a..6d0023647 100644 --- a/src/query/plan/rule_based_planner.hpp +++ b/src/query/plan/rule_based_planner.hpp @@ -120,6 +120,8 @@ struct Matching { Filters filters; /// Maps node symbols to expansions which bind them. std::unordered_map> node_symbol_to_expansions{}; + /// Maps named path symbols to a vector of Symbols that define its pattern. + std::unordered_map> named_paths{}; /// All node and edge symbols across all expansions (from all matches). std::unordered_set expansion_symbols{}; }; @@ -230,6 +232,14 @@ LogicalOperator *GenFilters(LogicalOperator *last_op, const std::unordered_set &bound_symbols, std::vector &all_filters, AstTreeStorage &storage); +// +/// For all given `named_paths` checks if the all it's symbols have been bound. +/// If so it creates a logical operator for named path generation, binds it's +/// symbol, removes that path from the collection of unhandled ones and returns +/// the new op. Otherwise it returns nullptr. +LogicalOperator *GenNamedPaths( + LogicalOperator *last_op, std::unordered_set &bound_symbols, + std::unordered_map> &named_paths); LogicalOperator *GenReturn(Return &ret, LogicalOperator *input_op, SymbolTable &symbol_table, bool is_write, @@ -474,6 +484,8 @@ class RuleBasedPlanner { const auto &matching = match_context.matching; // Copy all_filters, because we will modify the list as we generate Filters. auto all_filters = matching.filters.all_filters(); + // Copy the named_paths for the same reason. + auto named_paths = matching.named_paths; // Try to generate any filters even before the 1st match operator. This // optimizes the optional match which filters only on symbols bound in // regular match. @@ -496,6 +508,9 @@ class RuleBasedPlanner { match_context.new_symbols.emplace_back(node1_symbol); last_op = impl::GenFilters(last_op, bound_symbols, all_filters, storage); + last_op = impl::GenNamedPaths(last_op, bound_symbols, named_paths); + last_op = + impl::GenFilters(last_op, bound_symbols, all_filters, storage); } // We have an edge, so generate Expand. if (expansion.edge) { @@ -598,6 +613,9 @@ class RuleBasedPlanner { } last_op = impl::GenFilters(last_op, bound_symbols, all_filters, storage); + last_op = impl::GenNamedPaths(last_op, bound_symbols, named_paths); + last_op = + impl::GenFilters(last_op, bound_symbols, all_filters, storage); } } debug_assert(all_filters.empty(), "Expected to generate all filters"); diff --git a/src/query/typed_value.cpp b/src/query/typed_value.cpp index 9ec5a33c2..444c29294 100644 --- a/src/query/typed_value.cpp +++ b/src/query/typed_value.cpp @@ -421,7 +421,7 @@ TypedValue operator==(const TypedValue &a, const TypedValue &b) { return true; } case TypedValue::Type::Path: - throw utils::NotYetImplemented("equality for TypedValue::Type::Path"); + return a.ValuePath() == b.ValuePath(); default: permanent_fail("Unhandled comparison for types"); } @@ -657,8 +657,10 @@ size_t TypedValue::Hash::operator()(const TypedValue &value) const { case TypedValue::Type::Edge: return value.Value().temporary_id(); case TypedValue::Type::Path: - throw utils::NotYetImplemented("hashing for TypedValue::Type::Path"); - break; + return FnvCollection, VertexAccessor>{}( + value.ValuePath().vertices()) ^ + FnvCollection, EdgeAccessor>{}( + value.ValuePath().edges()); } permanent_fail("Unhandled TypedValue.type() in hash function"); } diff --git a/src/query/typed_value.hpp b/src/query/typed_value.hpp index abf336882..de38a2249 100644 --- a/src/query/typed_value.hpp +++ b/src/query/typed_value.hpp @@ -8,6 +8,7 @@ #include #include +#include "query/path.hpp" #include "storage/edge_accessor.hpp" #include "storage/property_value.hpp" #include "storage/vertex_accessor.hpp" @@ -17,8 +18,6 @@ namespace query { -typedef traversal_template::Path Path; - // TODO: Neo4j does overflow checking. Should we also implement it? /** * Encapsulation of a value and it's type encapsulated in a class that has no diff --git a/src/storage/edge_accessor.hpp b/src/storage/edge_accessor.hpp index f01adcc3b..948c52633 100644 --- a/src/storage/edge_accessor.hpp +++ b/src/storage/edge_accessor.hpp @@ -43,3 +43,11 @@ class EdgeAccessor : public RecordAccessor { }; std::ostream &operator<<(std::ostream &, const EdgeAccessor &); + +// hash function for the edge accessor +namespace std { +template <> +struct hash { + size_t operator()(const EdgeAccessor &e) const { return e.temporary_id(); }; +}; +} diff --git a/src/utils/hashing/fnv.hpp b/src/utils/hashing/fnv.hpp index d2df451e5..8338dd906 100644 --- a/src/utils/hashing/fnv.hpp +++ b/src/utils/hashing/fnv.hpp @@ -66,15 +66,20 @@ struct FnvCollection { static const uint64_t fnv_prime = 1099511628211u; }; -template +/** + * Like FNV hashing for a collection, just specialized for two elements to avoid + * iteration overhead. + */ +template , + typename TBHash = std::hash> struct HashCombine { - size_t operator()(const TA& a, const TB& b) const { + size_t operator()(const TA &a, const TB &b) const { constexpr size_t fnv_prime = 1099511628211UL; constexpr size_t fnv_offset = 14695981039346656037UL; size_t ret = fnv_offset; - ret ^= std::hash()(a); + ret ^= TAHash()(a); ret *= fnv_prime; - ret ^= std::hash()(b); + ret ^= TBHash()(b); return ret; } }; diff --git a/tests/manual/query_planner.cpp b/tests/manual/query_planner.cpp index 23539e134..7a82754be 100644 --- a/tests/manual/query_planner.cpp +++ b/tests/manual/query_planner.cpp @@ -428,6 +428,7 @@ class PlanPrinter : public query::plan::HierarchicalLogicalOperatorVisitor { } PRE_VISIT(ExpandBreadthFirst); + PRE_VISIT(ConstructNamedPath); PRE_VISIT(Filter); PRE_VISIT(SetProperty); PRE_VISIT(SetProperties); diff --git a/tests/qa/tck_engine/tests/memgraph_V1/features/match.feature b/tests/qa/tck_engine/tests/memgraph_V1/features/match.feature index 502288597..7bc01763c 100644 --- a/tests/qa/tck_engine/tests/memgraph_V1/features/match.feature +++ b/tests/qa/tck_engine/tests/memgraph_V1/features/match.feature @@ -484,3 +484,18 @@ Feature: Match Then the result should be: | n.a | m.a | | 1 | 2 | + + Scenario: Named path with length function. + Given an empty graph + And having executed: + """ + CREATE (:start)-[:type]->() + """ + When executing query: + """ + MATCH path = (:start) -[*0..1]-> () RETURN size(path) + """ + Then the result should be: + | size(path) | + | 0 | + | 1 | diff --git a/tests/unit/query_common.hpp b/tests/unit/query_common.hpp index e1b10c5e3..55fbd3dc3 100644 --- a/tests/unit/query_common.hpp +++ b/tests/unit/query_common.hpp @@ -23,9 +23,11 @@ #pragma once +#include +#include +#include #include #include -#include #include "database/dbms.hpp" #include "database/graph_db_datatypes.hpp" @@ -168,11 +170,30 @@ auto GetNode(AstTreeStorage &storage, const std::string &name, return node; } +/// Generates a randomly chosen (uniformly) string from a population of 10 ** 20 +std::string random_string() { + std::string str = "rand_str_"; + for (int i = 0; i < 20; i++) str += std::to_string(rand() % 10); + return str; +} + /// /// Create a Pattern with given atoms. /// auto GetPattern(AstTreeStorage &storage, std::vector atoms) { auto pattern = storage.Create(); + pattern->identifier_ = storage.Create(random_string(), false); + pattern->atoms_.insert(pattern->atoms_.begin(), atoms.begin(), atoms.end()); + return pattern; +} + +/// +/// Create a Pattern with given name and atoms. +/// +auto GetPattern(AstTreeStorage &storage, const std::string &name, + std::vector atoms) { + auto pattern = storage.Create(); + pattern->identifier_ = storage.Create(name, true); pattern->atoms_.insert(pattern->atoms_.begin(), atoms.begin(), atoms.end()); return pattern; } @@ -439,6 +460,8 @@ auto GetMerge(AstTreeStorage &storage, Pattern *pattern, OnMatch on_match, #define NODE(...) query::test_common::GetNode(storage, __VA_ARGS__) #define EDGE(...) query::test_common::GetEdge(storage, __VA_ARGS__) #define PATTERN(...) query::test_common::GetPattern(storage, {__VA_ARGS__}) +#define NAMED_PATTERN(name, ...) \ + query::test_common::GetPattern(storage, name, {__VA_ARGS__}) #define OPTIONAL_MATCH(...) \ query::test_common::GetWithPatterns(storage.Create(true), \ {__VA_ARGS__}) diff --git a/tests/unit/query_expression_evaluator.cpp b/tests/unit/query_expression_evaluator.cpp index 123d34ea2..37815f167 100644 --- a/tests/unit/query_expression_evaluator.cpp +++ b/tests/unit/query_expression_evaluator.cpp @@ -833,6 +833,16 @@ TEST(ExpressionEvaluator, FunctionSize) { .Value(), 3); ASSERT_THROW(EvaluateFunction("SIZE", {5}), QueryRuntimeException); + + Dbms dbms; + auto dba = dbms.active(); + auto v0 = dba->InsertVertex(); + query::Path path(v0); + EXPECT_EQ(EvaluateFunction("SIZE", {path}).ValueInt(), 0); + auto v1 = dba->InsertVertex(); + path.Expand(dba->InsertEdge(v0, v1, dba->EdgeType("type"))); + path.Expand(v1); + EXPECT_EQ(EvaluateFunction("SIZE", {path}).ValueInt(), 1); } TEST(ExpressionEvaluator, FunctionStartNode) { diff --git a/tests/unit/query_plan_match_filter_return.cpp b/tests/unit/query_plan_match_filter_return.cpp index aa187f89c..cc59ec451 100644 --- a/tests/unit/query_plan_match_filter_return.cpp +++ b/tests/unit/query_plan_match_filter_return.cpp @@ -25,18 +25,30 @@ using namespace query; using namespace query::plan; -TEST(QueryPlan, MatchReturn) { +class MatchReturnFixture : public testing::Test { + protected: Dbms dbms; - auto dba = dbms.active(); - - // add a few nodes to the database - dba->InsertVertex(); - dba->InsertVertex(); - dba->AdvanceCommand(); - + std::unique_ptr dba = dbms.active(); AstTreeStorage storage; SymbolTable symbol_table; + void AddVertices(int count) { + for (int i = 0; i < count; i++) dba->InsertVertex(); + } + + template + std::vector Results(std::shared_ptr &op) { + std::vector res; + for (const auto &row : CollectProduce(op.get(), symbol_table, *dba)) + res.emplace_back(row[0].Value()); + return res; + } +}; + +TEST_F(MatchReturnFixture, MatchReturn) { + AddVertices(2); + dba->AdvanceCommand(); + auto test_pull_count = [&](GraphView graph_view) { auto scan_all = MakeScanAll(storage, symbol_table, "n", nullptr, graph_view); @@ -57,6 +69,27 @@ TEST(QueryPlan, MatchReturn) { EXPECT_EQ(3, test_pull_count(GraphView::OLD)); } +TEST_F(MatchReturnFixture, MatchReturnPath) { + AddVertices(2); + dba->AdvanceCommand(); + + auto scan_all = MakeScanAll(storage, symbol_table, "n", nullptr); + Symbol path_sym = symbol_table.CreateSymbol("path", true); + auto make_path = std::make_shared( + scan_all.op_, path_sym, std::vector{scan_all.sym_}); + auto output = NEXPR("path", IDENT("path")); + symbol_table[*output->expression_] = path_sym; + symbol_table[*output] = symbol_table.CreateSymbol("named_expression_1", true); + auto produce = MakeProduce(make_path, output); + auto results = Results(produce); + ASSERT_EQ(results.size(), 2); + std::vector expected_paths; + for (const auto &v : dba->Vertices(false)) expected_paths.emplace_back(v); + ASSERT_EQ(expected_paths.size(), 2); + EXPECT_TRUE(std::is_permutation(expected_paths.begin(), expected_paths.end(), + results.begin())); +} + TEST(QueryPlan, MatchReturnCartesian) { Dbms dbms; auto dba = dbms.active(); @@ -218,25 +251,30 @@ TEST(QueryPlan, NodeFilterMultipleLabels) { EXPECT_EQ(results.size(), 2); } -TEST(QueryPlan, Expand) { +class ExpandFixture : public testing::Test { + protected: Dbms dbms; - auto dba = dbms.active(); - - // make a V-graph (v3)<-[r2]-(v1)-[r1]->(v2) - auto v1 = dba->InsertVertex(); - v1.add_label((GraphDbTypes::Label)1); - auto v2 = dba->InsertVertex(); - v2.add_label((GraphDbTypes::Label)2); - auto v3 = dba->InsertVertex(); - v3.add_label((GraphDbTypes::Label)3); - auto edge_type = dba->EdgeType("Edge"); - dba->InsertEdge(v1, v2, edge_type); - dba->InsertEdge(v1, v3, edge_type); - dba->AdvanceCommand(); - + std::unique_ptr dba = dbms.active(); AstTreeStorage storage; SymbolTable symbol_table; + // make a V-graph (v3)<-[r2]-(v1)-[r1]->(v2) + VertexAccessor v1 = dba->InsertVertex(); + VertexAccessor v2 = dba->InsertVertex(); + VertexAccessor v3 = dba->InsertVertex(); + GraphDbTypes::EdgeType edge_type = dba->EdgeType("Edge"); + EdgeAccessor r1 = dba->InsertEdge(v1, v2, edge_type); + EdgeAccessor r2 = dba->InsertEdge(v1, v3, edge_type); + + void SetUp() override { + v1.add_label((GraphDbTypes::Label)1); + v2.add_label((GraphDbTypes::Label)2); + v3.add_label((GraphDbTypes::Label)3); + dba->AdvanceCommand(); + } +}; + +TEST_F(ExpandFixture, Expand) { auto test_expand = [&](EdgeAtom::Direction direction, GraphView graph_view) { auto n = MakeScanAll(storage, symbol_table, "n"); auto r_m = MakeExpand(storage, symbol_table, n.op_, n.sym_, "r", direction, @@ -274,6 +312,29 @@ TEST(QueryPlan, Expand) { EXPECT_EQ(8, test_expand(EdgeAtom::Direction::BOTH, GraphView::OLD)); } +TEST_F(ExpandFixture, ExpandPath) { + auto n = MakeScanAll(storage, symbol_table, "n"); + auto r_m = MakeExpand(storage, symbol_table, n.op_, n.sym_, "r", + EdgeAtom::Direction::OUT, nullptr, false, "m", false); + Symbol path_sym = symbol_table.CreateSymbol("path", true); + auto path = std::make_shared( + r_m.op_, path_sym, + std::vector{n.sym_, r_m.edge_sym_, r_m.node_sym_}); + auto output = NEXPR("m", IDENT("m")); + symbol_table[*output->expression_] = path_sym; + symbol_table[*output] = symbol_table.CreateSymbol("named_expression_1", true); + auto produce = MakeProduce(path, output); + + std::vector expected_paths{{v1, r2, v3}, {v1, r1, v2}}; + auto results = CollectProduce(produce.get(), symbol_table, *dba); + ASSERT_EQ(results.size(), 2); + std::vector results_paths; + for (const auto &result : results) + results_paths.emplace_back(result[0].ValuePath()); + EXPECT_TRUE(std::is_permutation(expected_paths.begin(), expected_paths.end(), + results_paths.begin())); +} + /** * A fixture that sets a graph up and provides some functions. * @@ -288,7 +349,7 @@ TEST(QueryPlan, Expand) { */ class QueryPlanExpandVariable : public testing::Test { protected: - // type returned by the GetResults function, used + // type returned by the GetEdgeListSizes function, used // a lot below in test declaration using map_int = std::unordered_map; @@ -296,6 +357,8 @@ class QueryPlanExpandVariable : public testing::Test { std::unique_ptr dba = dbms.active(); // labels for layers in the double chain std::vector labels; + // for all the edges + GraphDbTypes::EdgeType edge_type = dba->EdgeType("edge_type"); AstTreeStorage storage; SymbolTable symbol_table; @@ -318,7 +381,7 @@ class QueryPlanExpandVariable : public testing::Test { v_to.add_label(label); for (size_t v_from_ind = 0; v_from_ind < layer.size(); v_from_ind++) { auto &v_from = layer[v_from_ind]; - auto edge = dba->InsertEdge(v_from, v_to, dba->EdgeType("edge_type")); + auto edge = dba->InsertEdge(v_from, v_to, edge_type); edge.PropsSet(dba->Property("p"), fmt::format("V{}{}->V{}{}", from_layer_ind, v_from_ind, from_layer_ind + 1, v_to_ind)); @@ -385,19 +448,35 @@ class QueryPlanExpandVariable : public testing::Test { } /** - * Pulls from the given input and analyses the edge-list (result of variable - * length expansion) found in the results under the given symbol. + * Pulls from the given input and returns the results under the given symbol. * - * @return a map {path_lenth -> number_of_results} + * @return a vector of values of the given type. + * @tparam TResult type of the result that is sought. */ + template auto GetResults(std::shared_ptr input_op, Symbol symbol) { - map_int count_per_length; Frame frame(symbol_table.max_position()); auto cursor = input_op->MakeCursor(*dba); Context context(*dba); context.symbol_table_ = symbol_table; - while (cursor->Pull(frame, context)) { - auto length = frame[symbol].Value>().size(); + std::vector results; + while (cursor->Pull(frame, context)) + results.emplace_back(frame[symbol].Value()); + return results; + } + + /** + * Pulls from the given input and analyses the edge-list (result of variable + * length expansion) found in the results under the given symbol. + * + * @return a map {edge_list_length -> number_of_results} + */ + auto GetEdgeListSizes(std::shared_ptr input_op, + Symbol symbol) { + map_int count_per_length; + for (const auto &edge_list : + GetResults>(input_op, symbol)) { + auto length = edge_list.size(); auto found = count_per_length.find(length); if (found == count_per_length.end()) count_per_length[length] = 1; @@ -414,10 +493,11 @@ TEST_F(QueryPlanExpandVariable, OneVariableExpansion) { std::experimental::optional upper, bool reverse) { auto e = Edge("r", direction); - return GetResults(AddMatch(nullptr, "n", layer, direction, - nullptr, lower, upper, e, false, - "m", GraphView::AS_IS, reverse), - e); + return GetEdgeListSizes( + AddMatch(nullptr, "n", layer, direction, nullptr, lower, + upper, e, false, "m", GraphView::AS_IS, + reverse), + e); }; for (int reverse = 0; reverse < 2; ++reverse) { @@ -501,7 +581,7 @@ TEST_F(QueryPlanExpandVariable, EdgeUniquenessSingleAndVariableExpansion) { last_op, last_symbol, symbols); } - return GetResults(last_op, var_length_sym); + return GetEdgeListSizes(last_op, var_length_sym); }; // no uniqueness between variable and single expansion @@ -531,7 +611,7 @@ TEST_F(QueryPlanExpandVariable, EdgeUniquenessTwoVariableExpansions) { last_op, e2, std::vector{e1}); } - return GetResults(last_op, e2); + return GetEdgeListSizes(last_op, e2); }; EXPECT_EQ(test_expand(0, EdgeAtom::Direction::OUT, 2, 2, false), @@ -553,7 +633,7 @@ TEST_F(QueryPlanExpandVariable, ExistingEdges) { auto second = AddMatch(first, "n2", layer, direction, nullptr, lower, upper, e2, same_edge_symbol, "m2"); - return GetResults(second, e2); + return GetEdgeListSizes(second, e2); }; EXPECT_EQ(test_expand(0, EdgeAtom::Direction::OUT, 1, 1, false), @@ -580,15 +660,12 @@ TEST_F(QueryPlanExpandVariable, ExistingEdges) { TEST_F(QueryPlanExpandVariable, GraphState) { auto test_expand = [&](GraphView graph_view, const auto &edge_type) { auto e = Edge("r", EdgeAtom::Direction::OUT); - return GetResults( + return GetEdgeListSizes( AddMatch(nullptr, "n", 0, EdgeAtom::Direction::OUT, edge_type, 2, 2, e, false, "m", graph_view), e); }; - EXPECT_EQ(test_expand(GraphView::OLD, dba->EdgeType("edge_type")), - (map_int{{2, 8}})); - auto new_edge_type = dba->EdgeType("some_type"); // add two vertices branching out from the second layer for (VertexAccessor &vertex : dba->Vertices(true)) @@ -602,18 +679,46 @@ TEST_F(QueryPlanExpandVariable, GraphState) { EXPECT_EQ(test_expand(GraphView::OLD, nullptr), (map_int{{2, 8}})); EXPECT_EQ(test_expand(GraphView::OLD, new_edge_type), (map_int{})); EXPECT_EQ(test_expand(GraphView::NEW, nullptr), (map_int{{2, 12}})); - EXPECT_EQ(test_expand(GraphView::NEW, dba->EdgeType("edge_type")), - (map_int{{2, 8}})); + EXPECT_EQ(test_expand(GraphView::NEW, edge_type), (map_int{{2, 8}})); EXPECT_EQ(test_expand(GraphView::NEW, new_edge_type), (map_int{})); dba->AdvanceCommand(); for (const auto graph_view : {GraphView::OLD, GraphView::NEW}) { EXPECT_EQ(test_expand(graph_view, nullptr), (map_int{{2, 12}})); - EXPECT_EQ(test_expand(graph_view, dba->EdgeType("edge_type")), - (map_int{{2, 8}})); + EXPECT_EQ(test_expand(graph_view, edge_type), (map_int{{2, 8}})); EXPECT_EQ(test_expand(graph_view, new_edge_type), (map_int{})); } } +TEST_F(QueryPlanExpandVariable, NamedPath) { + auto e = Edge("r", EdgeAtom::Direction::OUT); + auto expand = AddMatch( + nullptr, "n", 0, EdgeAtom::Direction::OUT, nullptr, 2, 2, e, false, "m"); + auto find_symbol = [this](const std::string &name) { + for (const auto &pos_sym : symbol_table.table()) + if (pos_sym.second.name() == name) return pos_sym.second; + throw std::runtime_error("Symbol not found"); + }; + + auto path_symbol = + symbol_table.CreateSymbol("path", true, Symbol::Type::Path); + auto create_path = std::make_shared( + expand, path_symbol, + std::vector{find_symbol("n"), e, find_symbol("m")}); + + std::vector expected_paths; + for (const auto &v : dba->Vertices(labels[0], false)) + for (const auto &e1 : v.out()) + for (const auto &e2 : e1.to().out()) + expected_paths.emplace_back(v, e1, e1.to(), e2, e2.to()); + ASSERT_EQ(expected_paths.size(), 8); + + auto results = GetResults(create_path, path_symbol); + ASSERT_EQ(results.size(), 8); + EXPECT_TRUE(std::is_permutation(results.begin(), results.end(), + expected_paths.begin(), + TypedValue::BoolEqual{})); +} + namespace std { template <> struct hash> { diff --git a/tests/unit/query_planner.cpp b/tests/unit/query_planner.cpp index ca1b9bc57..411f37291 100644 --- a/tests/unit/query_planner.cpp +++ b/tests/unit/query_planner.cpp @@ -57,6 +57,7 @@ class PlanChecker : public HierarchicalLogicalOperatorVisitor { PRE_VISIT(ExpandVariable); PRE_VISIT(ExpandBreadthFirst); PRE_VISIT(Filter); + PRE_VISIT(ConstructNamedPath); PRE_VISIT(Produce); PRE_VISIT(SetProperty); PRE_VISIT(SetProperties); @@ -83,7 +84,7 @@ class PlanChecker : public HierarchicalLogicalOperatorVisitor { PRE_VISIT(Unwind); PRE_VISIT(Distinct); - bool Visit(Once &op) override { + bool Visit(Once &) override { // Ignore checking Once, it is implicitly at the end. return true; } @@ -115,7 +116,7 @@ class OpChecker : public BaseOpChecker { ExpectOp(*expected_op, symbol_table); } - virtual void ExpectOp(TOp &op, const SymbolTable &) {} + virtual void ExpectOp(TOp &, const SymbolTable &) {} }; using ExpectCreateNode = OpChecker; @@ -127,6 +128,7 @@ using ExpectExpand = OpChecker; using ExpectExpandVariable = OpChecker; using ExpectExpandBreadthFirst = OpChecker; using ExpectFilter = OpChecker; +using ExpectConstructNamedPath = OpChecker; using ExpectProduce = OpChecker; using ExpectSetProperty = OpChecker; using ExpectSetProperties = OpChecker; @@ -147,7 +149,7 @@ class ExpectAccumulate : public OpChecker { ExpectAccumulate(const std::unordered_set &symbols) : symbols_(symbols) {} - void ExpectOp(Accumulate &op, const SymbolTable &symbol_table) override { + void ExpectOp(Accumulate &op, const SymbolTable &) override { std::unordered_set got_symbols(op.symbols().begin(), op.symbols().end()); EXPECT_EQ(symbols_, got_symbols); @@ -364,6 +366,18 @@ TEST(TestLogicalPlanner, CreateNodeExpandNode) { ExpectCreateNode()); } +TEST(TestLogicalPlanner, CreateNamedPattern) { + // Test CREATE p = (n) -[r :rel]-> (m) + AstTreeStorage storage; + Dbms dbms; + auto dba = dbms.active(); + auto relationship = dba->EdgeType("rel"); + QUERY(CREATE(NAMED_PATTERN( + "p", NODE("n"), EDGE("r", relationship, Direction::OUT), NODE("m")))); + CheckPlan(storage, ExpectCreateNode(), ExpectCreateExpand(), + ExpectConstructNamedPath()); +} + TEST(TestLogicalPlanner, MatchCreateExpand) { // Test MATCH (n) CREATE (n) -[r :rel1]-> (m) AstTreeStorage storage; @@ -398,6 +412,32 @@ TEST(TestLogicalPlanner, MatchPathReturn) { ExpectProduce()); } +TEST(TestLogicalPlanner, MatchNamedPatternReturn) { + // Test MATCH p = (n) -[r :relationship]- (m) RETURN p + AstTreeStorage storage; + Dbms dbms; + auto dba = dbms.active(); + auto relationship = dba->EdgeType("relationship"); + QUERY( + MATCH(NAMED_PATTERN("p", NODE("n"), EDGE("r", relationship), NODE("m"))), + RETURN("n")); + CheckPlan(storage, ExpectScanAll(), ExpectExpand(), ExpectFilter(), + ExpectConstructNamedPath(), ExpectProduce()); +} + +TEST(TestLogicalPlanner, MatchNamedPatternWithPredicateReturn) { + // Test MATCH p = (n) -[r :relationship]- (m) RETURN p + AstTreeStorage storage; + Dbms dbms; + auto dba = dbms.active(); + auto relationship = dba->EdgeType("relationship"); + QUERY( + MATCH(NAMED_PATTERN("p", NODE("n"), EDGE("r", relationship), NODE("m"))), + WHERE(EQ(LITERAL(2), IDENT("p"))), RETURN("n")); + CheckPlan(storage, ExpectScanAll(), ExpectExpand(), ExpectFilter(), + ExpectConstructNamedPath(), ExpectFilter(), ExpectProduce()); +} + TEST(TestLogicalPlanner, MatchWhereReturn) { // Test MATCH (n) WHERE n.property < 42 RETURN n AstTreeStorage storage; diff --git a/tests/unit/query_semantic.cpp b/tests/unit/query_semantic.cpp index d07e201d3..566753119 100644 --- a/tests/unit/query_semantic.cpp +++ b/tests/unit/query_semantic.cpp @@ -21,9 +21,13 @@ TEST(TestSymbolGenerator, MatchNodeReturn) { QUERY(MATCH(PATTERN(NODE("node_atom_1"))), RETURN("node_atom_1")); SymbolGenerator symbol_generator(symbol_table); query_ast->Accept(symbol_generator); - EXPECT_EQ(symbol_table.max_position(), 2); + // symbols for pattern, node_atom_1 and named_expr in return + EXPECT_EQ(symbol_table.max_position(), 3); auto match = dynamic_cast(query_ast->clauses_[0]); auto pattern = match->patterns_[0]; + auto pattern_sym = symbol_table[*pattern->identifier_]; + EXPECT_EQ(pattern_sym.type(), Symbol::Type::Path); + EXPECT_FALSE(pattern_sym.user_declared()); auto node_atom = dynamic_cast(pattern->atoms_[0]); auto node_sym = symbol_table[*node_atom->identifier_]; EXPECT_EQ(node_sym.name(), "node_atom_1"); @@ -37,6 +41,24 @@ TEST(TestSymbolGenerator, MatchNodeReturn) { EXPECT_EQ(node_sym, ret_sym); } +TEST(TestSymbolGenerator, MatchNamedPattern) { + SymbolTable symbol_table; + AstTreeStorage storage; + // MATCH p = (node_atom_1) RETURN node_atom_1 + auto query_ast = QUERY(MATCH(NAMED_PATTERN("p", NODE("node_atom_1"))), + RETURN("p")); + SymbolGenerator symbol_generator(symbol_table); + query_ast->Accept(symbol_generator); + // symbols for p, node_atom_1 and named_expr in return + EXPECT_EQ(symbol_table.max_position(), 3); + auto match = dynamic_cast(query_ast->clauses_[0]); + auto pattern = match->patterns_[0]; + auto pattern_sym = symbol_table[*pattern->identifier_]; + EXPECT_EQ(pattern_sym.type(), Symbol::Type::Path); + EXPECT_EQ(pattern_sym.name(), "p"); + EXPECT_TRUE(pattern_sym.user_declared()); +} + TEST(TestSymbolGenerator, MatchUnboundMultiReturn) { SymbolTable symbol_table; AstTreeStorage storage; @@ -69,7 +91,8 @@ TEST(TestSymbolGenerator, MatchSameEdge) { RETURN("r")); SymbolGenerator symbol_generator(symbol_table); query_ast->Accept(symbol_generator); - EXPECT_EQ(symbol_table.max_position(), 3); + // symbols for pattern, `n`, `r` and named_expr in return + EXPECT_EQ(symbol_table.max_position(), 4); auto match = dynamic_cast(query_ast->clauses_[0]); auto pattern = match->patterns_[0]; std::vector node_symbols; @@ -120,7 +143,8 @@ TEST(TestSymbolGenerator, CreateNodeReturn) { auto query_ast = QUERY(CREATE(PATTERN(NODE("n"))), RETURN("n")); SymbolGenerator symbol_generator(symbol_table); query_ast->Accept(symbol_generator); - EXPECT_EQ(symbol_table.max_position(), 2); + // symbols for pattern, `n` and named_expr + EXPECT_EQ(symbol_table.max_position(), 3); auto create = dynamic_cast(query_ast->clauses_[0]); auto pattern = create->patterns_[0]; auto node_atom = dynamic_cast(pattern->atoms_[0]); @@ -252,7 +276,8 @@ TEST(TestSymbolGenerator, CreateDelete) { SymbolTable symbol_table; SymbolGenerator symbol_generator(symbol_table); query->Accept(symbol_generator); - EXPECT_EQ(symbol_table.max_position(), 1); + // symbols for pattern and `n` + EXPECT_EQ(symbol_table.max_position(), 2); auto node_symbol = symbol_table.at(*node->identifier_); auto ident_symbol = symbol_table.at(*ident); EXPECT_EQ(node_symbol.type(), Symbol::Type::Vertex); @@ -281,7 +306,8 @@ TEST(TestSymbolGenerator, MatchWithReturn) { SymbolTable symbol_table; SymbolGenerator symbol_generator(symbol_table); query->Accept(symbol_generator); - EXPECT_EQ(symbol_table.max_position(), 3); + // symbols for pattern, `old`, `n` and named_expr in return + EXPECT_EQ(symbol_table.max_position(), 4); auto node_symbol = symbol_table.at(*node->identifier_); auto old = symbol_table.at(*old_ident); EXPECT_EQ(node_symbol, old); @@ -318,7 +344,8 @@ TEST(TestSymbolGenerator, MatchWithWhere) { SymbolTable symbol_table; SymbolGenerator symbol_generator(symbol_table); query->Accept(symbol_generator); - EXPECT_EQ(symbol_table.max_position(), 2); + // symbols for pattern, `old` and `n` + EXPECT_EQ(symbol_table.max_position(), 3); auto node_symbol = symbol_table.at(*node->identifier_); auto old = symbol_table.at(*old_ident); EXPECT_EQ(node_symbol, old); @@ -360,7 +387,8 @@ TEST(TestSymbolGenerator, CreateMultiExpand) { SymbolTable symbol_table; SymbolGenerator symbol_generator(symbol_table); query->Accept(symbol_generator); - EXPECT_EQ(symbol_table.max_position(), 5); + // symbols for pattern * 2, `n`, `r`, `m`, `p`, `l` + EXPECT_EQ(symbol_table.max_position(), 7); auto n1 = symbol_table.at(*node_n1->identifier_); auto n2 = symbol_table.at(*node_n2->identifier_); EXPECT_EQ(n1, n2); @@ -424,8 +452,8 @@ TEST(TestSymbolGenerator, MatchReturnSum) { SymbolTable symbol_table; SymbolGenerator symbol_generator(symbol_table); query->Accept(symbol_generator); - // 3 symbols for: 'n', 'sum' and 'result'. - EXPECT_EQ(symbol_table.max_position(), 3); + // 3 symbols for: pattern, 'n', 'sum' and 'result'. + EXPECT_EQ(symbol_table.max_position(), 4); auto node_symbol = symbol_table.at(*node->identifier_); auto sum_symbol = symbol_table.at(*sum); EXPECT_NE(node_symbol, sum_symbol); @@ -476,7 +504,8 @@ TEST(TestSymbolGenerator, MatchPropCreateNodeProp) { SymbolTable symbol_table; SymbolGenerator symbol_generator(symbol_table); query->Accept(symbol_generator); - EXPECT_EQ(symbol_table.max_position(), 2); + // symbols: pattern * 2, `node_n`, `node_m` + EXPECT_EQ(symbol_table.max_position(), 4); auto n = symbol_table.at(*node_n->identifier_); EXPECT_EQ(n, symbol_table.at(*n_prop->expression_)); auto m = symbol_table.at(*node_m->identifier_); @@ -497,7 +526,8 @@ TEST(TestSymbolGenerator, CreateNodeEdge) { SymbolTable symbol_table; SymbolGenerator symbol_generator(symbol_table); query->Accept(symbol_generator); - EXPECT_EQ(symbol_table.max_position(), 2); + // symbols: pattern * 2, `n`, `r` + EXPECT_EQ(symbol_table.max_position(), 4); auto n = symbol_table.at(*node_1->identifier_); EXPECT_EQ(n, symbol_table.at(*node_2->identifier_)); EXPECT_EQ(n, symbol_table.at(*node_3->identifier_)); @@ -519,7 +549,8 @@ TEST(TestSymbolGenerator, MatchWithCreate) { SymbolTable symbol_table; SymbolGenerator symbol_generator(symbol_table); query->Accept(symbol_generator); - EXPECT_EQ(symbol_table.max_position(), 3); + // symbols: pattern * 2, `n`, `m`, `r` + EXPECT_EQ(symbol_table.max_position(), 5); auto n = symbol_table.at(*node_1->identifier_); EXPECT_EQ(n.type(), Symbol::Type::Vertex); auto m = symbol_table.at(*node_2->identifier_); @@ -612,8 +643,8 @@ TEST(TestSymbolGenerator, AggregationOrderBy) { SymbolTable symbol_table; SymbolGenerator symbol_generator(symbol_table); query->Accept(symbol_generator); - // Symbols for `old`, `count(old)` and `new` - EXPECT_EQ(symbol_table.max_position(), 3); + // Symbols for pattern, `old`, `count(old)` and `new` + EXPECT_EQ(symbol_table.max_position(), 4); auto old = symbol_table.at(*node->identifier_); EXPECT_EQ(old, symbol_table.at(*ident_old)); auto new_sym = symbol_table.at(*as_new); @@ -633,8 +664,8 @@ TEST(TestSymbolGenerator, OrderByOldVariable) { SymbolTable symbol_table; SymbolGenerator symbol_generator(symbol_table); query->Accept(symbol_generator); - // Symbols for `old` and `new` - EXPECT_EQ(symbol_table.max_position(), 2); + // Symbols for pattern, `old` and `new` + EXPECT_EQ(symbol_table.max_position(), 3); auto old = symbol_table.at(*node->identifier_); EXPECT_EQ(old, symbol_table.at(*ident_old)); EXPECT_EQ(old, symbol_table.at(*by_old)); @@ -699,8 +730,8 @@ TEST(TestSymbolGenerator, MergeOnMatchOnCreate) { SymbolTable symbol_table; SymbolGenerator symbol_generator(symbol_table); query->Accept(symbol_generator); - // Symbols for: `n`, `r`, `m` and `AS r`. - EXPECT_EQ(symbol_table.max_position(), 4); + // Symbols for: pattern * 2, `n`, `r`, `m` and `AS r`. + EXPECT_EQ(symbol_table.max_position(), 6); auto n = symbol_table.at(*match_n->identifier_); EXPECT_EQ(n, symbol_table.at(*merge_n->identifier_)); EXPECT_EQ(n, symbol_table.at(*n_prop->expression_)); @@ -770,8 +801,8 @@ TEST(TestSymbolGenerator, MatchCrossReferenceVariable) { SymbolTable symbol_table; SymbolGenerator symbol_generator(symbol_table); query->Accept(symbol_generator); - // Symbols for `n`, `m` and `AS n` - EXPECT_EQ(symbol_table.max_position(), 3); + // Symbols for pattern * 2, `n`, `m` and `AS n` + EXPECT_EQ(symbol_table.max_position(), 5); auto n = symbol_table.at(*node_n->identifier_); EXPECT_EQ(n, symbol_table.at(*n_prop->expression_)); EXPECT_EQ(n, symbol_table.at(*ident_n)); @@ -800,8 +831,8 @@ TEST(TestSymbolGenerator, MatchWithAsteriskReturnAsterisk) { SymbolTable symbol_table; SymbolGenerator symbol_generator(symbol_table); query->Accept(symbol_generator); - // Symbols for `n`, `e`, `m`, `AS n.prop`. - EXPECT_EQ(symbol_table.max_position(), 4); + // Symbols for pattern, `n`, `e`, `m`, `AS n.prop`. + EXPECT_EQ(symbol_table.max_position(), 5); auto n = symbol_table.at(*node_n->identifier_); EXPECT_EQ(n, symbol_table.at(*n_prop->expression_)); } @@ -860,8 +891,8 @@ TEST(TestSymbolGenerator, MatchEdgeWithIdentifierInProperty) { SymbolTable symbol_table; SymbolGenerator symbol_generator(symbol_table); query->Accept(symbol_generator); - // Symbols for `n`, `r`, `m` and implicit in RETURN `r AS r` - EXPECT_EQ(symbol_table.max_position(), 4); + // Symbols for pattern, `n`, `r`, `m` and implicit in RETURN `r AS r` + EXPECT_EQ(symbol_table.max_position(), 5); auto n = symbol_table.at(*node_n->identifier_); EXPECT_EQ(n, symbol_table.at(*n_prop->expression_)); } @@ -882,8 +913,8 @@ TEST(TestSymbolGenerator, MatchVariablePathUsingIdentifier) { SymbolTable symbol_table; SymbolGenerator symbol_generator(symbol_table); query->Accept(symbol_generator); - // Symbols for `n`, `r`, `m`, `l` and implicit in RETURN `r AS r` - EXPECT_EQ(symbol_table.max_position(), 5); + // Symbols for pattern * 2, `n`, `r`, `m`, `l` and implicit in RETURN `r AS r` + EXPECT_EQ(symbol_table.max_position(), 7); auto l = symbol_table.at(*node_l->identifier_); EXPECT_EQ(l, symbol_table.at(*l_prop->expression_)); auto r = symbol_table.at(*edge->identifier_); @@ -1027,8 +1058,8 @@ TEST(TestSymbolGenerator, MatchBfsReturn) { SymbolTable symbol_table; SymbolGenerator symbol_generator(symbol_table); query->Accept(symbol_generator); - // Symbols for `n`, `[r]`, `r|`, `n|`, `m` and `AS r`. - EXPECT_EQ(symbol_table.max_position(), 6); + // 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->traversed_edge_identifier_)); diff --git a/tests/unit/typed_value.cpp b/tests/unit/typed_value.cpp index b6ed58be0..97ef4864f 100644 --- a/tests/unit/typed_value.cpp +++ b/tests/unit/typed_value.cpp @@ -38,7 +38,7 @@ class AllTypesFixture : public testing::Test { values_.emplace_back(vertex); values_.emplace_back( dba_->InsertEdge(vertex, vertex, dba_->EdgeType("et"))); - values_.emplace_back(query::Path{}); + values_.emplace_back(query::Path(dba_->InsertVertex())); } };