From c1d0090fe1c8352252cf485ea1105e86b118a8d6 Mon Sep 17 00:00:00 2001 From: Teon Banek Date: Tue, 18 Apr 2017 11:40:07 +0200 Subject: [PATCH] During type check `Any` type should always match Reviewers: florijan, mislav.bradac Reviewed By: florijan Subscribers: pullbot Differential Revision: https://phabricator.memgraph.io/D286 --- .../frontend/semantic/symbol_generator.cpp | 13 +++++++--- src/query/frontend/semantic/symbol_table.hpp | 2 +- tests/unit/query_semantic.cpp | 25 +++++++++++++++++++ 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/query/frontend/semantic/symbol_generator.cpp b/src/query/frontend/semantic/symbol_generator.cpp index 7095fc11f..01e3ac3e2 100644 --- a/src/query/frontend/semantic/symbol_generator.cpp +++ b/src/query/frontend/semantic/symbol_generator.cpp @@ -17,7 +17,9 @@ auto SymbolGenerator::GetOrCreateSymbol(const std::string &name, auto search = scope_.symbols.find(name); if (search != scope_.symbols.end()) { auto symbol = search->second; - if (type != Symbol::Type::Any && type != symbol.type_) { + // Unless we have `Any` type, check that types match. + if (type != Symbol::Type::Any && symbol.type_ != Symbol::Type::Any && + type != symbol.type_) { throw TypeMismatchError(name, Symbol::TypeToString(symbol.type_), Symbol::TypeToString(type)); } @@ -39,6 +41,7 @@ void SymbolGenerator::PostVisit(Return &ret) { // Named expressions establish bindings for expressions which come after // return, but not for the expressions contained inside. symbol_table_[*named_expr] = CreateSymbol(named_expr->name_); + // Improvement to type checking system would be to infer the type of the expression. } scope_.in_return = false; } @@ -54,6 +57,7 @@ bool SymbolGenerator::PreVisit(With &with) { // be visible inside named expressions themselves. scope_.symbols.clear(); for (auto &named_expr : with.named_expressions_) { + // Improvement would be to infer the type of the expression. symbol_table_[*named_expr] = CreateSymbol(named_expr->name_); } if (with.where_) with.where_->Accept(*this); @@ -108,7 +112,8 @@ void SymbolGenerator::Visit(Aggregation &aggr) { "allowed"); } // Create a virtual symbol for aggregation result. - symbol_table_[aggr] = symbol_table_.CreateSymbol(""); + // Currently, we only have aggregation operators which return numbers. + symbol_table_[aggr] = symbol_table_.CreateSymbol("", Symbol::Type::Number); scope_.in_aggregation = true; } @@ -120,7 +125,7 @@ void SymbolGenerator::PostVisit(Aggregation &aggr) { void SymbolGenerator::Visit(Pattern &pattern) { scope_.in_pattern = true; - if (scope_.in_create && pattern.atoms_.size() == 1) { + if (scope_.in_create && pattern.atoms_.size() == 1U) { debug_assert(dynamic_cast(pattern.atoms_[0]), "Expected a single NodeAtom in Pattern"); scope_.in_create_node = true; @@ -157,7 +162,7 @@ void SymbolGenerator::Visit(EdgeAtom &edge_atom) { scope_.in_edge_atom = true; if (scope_.in_create) { scope_.in_create_edge = true; - if (edge_atom.edge_types_.size() != 1) { + if (edge_atom.edge_types_.size() != 1U) { throw SemanticException( "A single relationship type must be specified " "when creating an edge."); diff --git a/src/query/frontend/semantic/symbol_table.hpp b/src/query/frontend/semantic/symbol_table.hpp index bbf597fae..b40500e55 100644 --- a/src/query/frontend/semantic/symbol_table.hpp +++ b/src/query/frontend/semantic/symbol_table.hpp @@ -10,7 +10,7 @@ namespace query { class Symbol { public: // This is similar to TypedValue::Type, but this has `Any` type. - enum class Type { Any, Vertex, Edge, Path }; + enum class Type { Any, Vertex, Edge, Path, Number }; static std::string TypeToString(Type type) { const char *enum_string[] = {"Any", "Vertex", "Edge", "Path"}; diff --git a/tests/unit/query_semantic.cpp b/tests/unit/query_semantic.cpp index 2a37a8e98..341456a6b 100644 --- a/tests/unit/query_semantic.cpp +++ b/tests/unit/query_semantic.cpp @@ -509,4 +509,29 @@ TEST(TestSymbolGenerator, CreateNodeEdge) { EXPECT_NE(n, symbol_table.at(*edge->identifier_)); } +TEST(TestSymbolGenerator, MatchWithCreate) { + // Test MATCH (n) WITH n AS m CREATE (m) -[r :r]-> (m) + Dbms dbms; + auto dba = dbms.active(); + auto r_type = dba->edge_type("r"); + AstTreeStorage storage; + auto node_1 = NODE("n"); + auto node_2 = NODE("m"); + auto edge = EDGE("r", r_type, EdgeAtom::Direction::RIGHT); + auto node_3 = NODE("m"); + auto query = QUERY(MATCH(PATTERN(node_1)), WITH(IDENT("n"), AS("m")), + CREATE(PATTERN(node_2, edge, node_3))); + SymbolTable symbol_table; + SymbolGenerator symbol_generator(symbol_table); + query->Accept(symbol_generator); + EXPECT_EQ(symbol_table.max_position(), 3); + auto n = symbol_table.at(*node_1->identifier_); + EXPECT_EQ(n.type_, Symbol::Type::Vertex); + auto m = symbol_table.at(*node_2->identifier_); + EXPECT_NE(n, m); + // Currently we don't infer expression types, so we lost true type of 'm'. + EXPECT_EQ(m.type_, Symbol::Type::Any); + EXPECT_EQ(m, symbol_table.at(*node_3->identifier_)); +} + }