diff --git a/src/query/frontend/semantic/symbol_generator.cpp b/src/query/frontend/semantic/symbol_generator.cpp index c92db6f2c..613f94df4 100644 --- a/src/query/frontend/semantic/symbol_generator.cpp +++ b/src/query/frontend/semantic/symbol_generator.cpp @@ -157,25 +157,33 @@ void SymbolGenerator::VisitReturnBody(ReturnBody &body, Where *where) { // Query bool SymbolGenerator::PreVisit(SingleQuery &) { - prev_return_names_ = curr_return_names_; - curr_return_names_.clear(); + auto &scope = scopes_.back(); + + scope.prev_return_names = scope.curr_return_names; + scope.curr_return_names.clear(); return true; } // Union bool SymbolGenerator::PreVisit(CypherUnion &) { + auto scope = scopes_.back(); scopes_.back() = Scope(); + + scopes_.back().curr_return_names = scope.curr_return_names; + return true; } bool SymbolGenerator::PostVisit(CypherUnion &cypher_union) { - if (prev_return_names_ != curr_return_names_) { + auto &scope = scopes_.back(); + + if (scope.prev_return_names != scope.curr_return_names) { throw SemanticException("All subqueries in an UNION must have the same column names."); } // create new symbols for the result of the union - for (const auto &name : curr_return_names_) { + for (const auto &name : scope.curr_return_names) { auto symbol = CreateSymbol(name, false); cypher_union.union_symbols_.push_back(symbol); } @@ -259,7 +267,9 @@ bool SymbolGenerator::PreVisit(Return &ret) { } bool SymbolGenerator::PostVisit(Return &) { - for (const auto &name_symbol : scopes_.back().symbols) curr_return_names_.insert(name_symbol.first); + auto &scope = scopes_.back(); + + for (const auto &name_symbol : scope.symbols) scope.curr_return_names.insert(name_symbol.first); return true; } diff --git a/src/query/frontend/semantic/symbol_generator.hpp b/src/query/frontend/semantic/symbol_generator.hpp index 25b8dc648..5b98958c6 100644 --- a/src/query/frontend/semantic/symbol_generator.hpp +++ b/src/query/frontend/semantic/symbol_generator.hpp @@ -140,6 +140,8 @@ class SymbolGenerator : public HierarchicalTreeVisitor { std::vector identifiers_in_match; // Number of nested IfOperators. int num_if_operators{0}; + std::unordered_set prev_return_names{}; + std::unordered_set curr_return_names{}; }; static std::optional FindSymbolInScope(const std::string &name, const Scope &scope, Symbol::Type type); @@ -171,8 +173,6 @@ class SymbolGenerator : public HierarchicalTreeVisitor { // is mapped by its name. std::unordered_map predefined_identifiers_; std::vector scopes_; - std::unordered_set prev_return_names_; - std::unordered_set curr_return_names_; }; inline SymbolTable MakeSymbolTable(CypherQuery *query, const std::vector &predefined_identifiers = {}) { diff --git a/src/query/plan/cost_estimator.hpp b/src/query/plan/cost_estimator.hpp index ab1a928e8..4940f939a 100644 --- a/src/query/plan/cost_estimator.hpp +++ b/src/query/plan/cost_estimator.hpp @@ -283,6 +283,7 @@ class CostEstimator : public HierarchicalLogicalOperatorVisitor { bool PostVisit(Produce &op) override { auto scope = Scope(); + // translate all the stats to the scope outside the return for (const auto &symbol : op.ModifiedSymbols(table_)) { auto stats = GetStatsFor(symbol); if (stats.has_value()) { diff --git a/tests/e2e/analyze_graph/optimize_indexes.py b/tests/e2e/analyze_graph/optimize_indexes.py index e78a57c16..54b2a33c8 100644 --- a/tests/e2e/analyze_graph/optimize_indexes.py +++ b/tests/e2e/analyze_graph/optimize_indexes.py @@ -14,6 +14,11 @@ import sys import pytest from common import connect, execute_and_fetch_all, memgraph + +class OptimizeIndexesConstants: + QUERY_PLAN = "QUERY PLAN" + + # E2E tests for checking query semantic # ------------------------------------ @@ -287,12 +292,8 @@ def test_given_supernode_when_expanding_then_expand_other_way_around(memgraph): memgraph.execute("CREATE INDEX ON :Node;") memgraph.execute("match (n:Node) match (s:SuperNode {id: 1}) merge (n)<-[:HAS_REL_TO]-(s);") - result_without_analysis = list( - memgraph.execute_and_fetch("explain match (n:Node) match (s:SuperNode {id: 1}) merge (n)<-[:HAS_REL_TO]-(s);") - ) - result_without_analysis = [x["QUERY PLAN"] for x in result_without_analysis] - - expected_explain_before_analysis = [ + query = "explain match (n:Node) match (s:SuperNode {id: 1}) merge (n)<-[:HAS_REL_TO]-(s);" + expected_explain = [ f" * EmptyResult", f" * Merge", f" |\\ On Match", @@ -306,29 +307,21 @@ def test_given_supernode_when_expanding_then_expand_other_way_around(memgraph): f" * Once", ] + result_without_analysis = list(memgraph.execute_and_fetch(query)) + result_without_analysis = [x[OptimizeIndexesConstants.QUERY_PLAN] for x in result_without_analysis] + assert expected_explain == result_without_analysis + memgraph.execute("analyze graph;") - result_with_analysis = list( - memgraph.execute_and_fetch("explain match (n:Node) match (s:SuperNode {id: 1}) merge (n)<-[:HAS_REL_TO]-(s);") - ) - result_with_analysis = [x["QUERY PLAN"] for x in result_with_analysis] - - expected_explain_after_analysis = [ - f" * EmptyResult", - f" * Merge", - f" |\\ On Match", - f" | * Expand (n)<-[anon3:HAS_REL_TO]-(s)", - f" | * Once", - f" |\\ On Create", - f" | * CreateExpand (n)<-[anon3:HAS_REL_TO]-(s)", - f" | * Once", - f" * ScanAllByLabel (n :Node)", - f" * ScanAllByLabelPropertyValue (s :SuperNode {{id}})", - f" * Once", + expected_explain = [ + x.replace(f" | * Expand (s)-[anon3:HAS_REL_TO]->(n)", f" | * Expand (n)<-[anon3:HAS_REL_TO]-(s)") + for x in expected_explain ] - assert expected_explain_before_analysis == result_without_analysis - assert expected_explain_after_analysis == result_with_analysis + result_with_analysis = list(memgraph.execute_and_fetch(query)) + result_with_analysis = [x[OptimizeIndexesConstants.QUERY_PLAN] for x in result_with_analysis] + + assert expected_explain == result_with_analysis def test_given_supernode_when_subquery_then_carry_information_to_subquery(memgraph): @@ -345,14 +338,10 @@ def test_given_supernode_when_subquery_then_carry_information_to_subquery(memgra memgraph.execute("match (n:Node) match (s:SuperNode {id: 1}) merge (n)<-[:HAS_REL_TO]-(s);") memgraph.execute("match (n:Node2) match (s:SuperNode {id: 1}) merge (n)<-[:HAS_REL_TO]-(s);") - result_without_analysis = list( - memgraph.execute_and_fetch( - "explain match (n:Node) match (s:SuperNode {id: 1}) call { with n, s merge (n)<-[:HAS_REL_TO]-(s) } return 1" - ) + query = ( + "explain match (n:Node) match (s:SuperNode {id: 1}) call { with n, s merge (n)<-[:HAS_REL_TO]-(s) } return 1" ) - result_without_analysis = [x["QUERY PLAN"] for x in result_without_analysis] - - expected_explain_before_analysis = [ + expected_explain = [ f" * Produce {{0}}", f" * Accumulate", f" * Accumulate", @@ -373,17 +362,59 @@ def test_given_supernode_when_subquery_then_carry_information_to_subquery(memgra f" * Once", ] + result_without_analysis = list(memgraph.execute_and_fetch(query)) + result_without_analysis = [x[OptimizeIndexesConstants.QUERY_PLAN] for x in result_without_analysis] + assert expected_explain == result_without_analysis + memgraph.execute("analyze graph;") - result_with_analysis = list( - memgraph.execute_and_fetch( - "explain match (n:Node) match (s:SuperNode {id: 1}) call { with n, s merge (n)<-[:HAS_REL_TO]-(s) } return 1" - ) - ) - result_with_analysis = [x["QUERY PLAN"] for x in result_with_analysis] + expected_explain = [ + x.replace(f" | | * Expand (s)-[anon3:HAS_REL_TO]->(n)", f" | | * Expand (n)<-[anon3:HAS_REL_TO]-(s)") + for x in expected_explain + ] + result_with_analysis = list(memgraph.execute_and_fetch(query)) + result_with_analysis = [x[OptimizeIndexesConstants.QUERY_PLAN] for x in result_with_analysis] - expected_explain_after_analysis = [ - f" * Produce {{0}}", + assert expected_explain == result_with_analysis + + +def test_given_supernode_when_subquery_and_union_then_carry_information(memgraph): + memgraph.execute("FOREACH (i in range(1, 1000) | CREATE (:Node {id: i}));") + memgraph.execute("FOREACH (i in range(1, 1000) | CREATE (:Node2 {id: i}));") + memgraph.execute("CREATE (:SuperNode {id: 1});") + memgraph.execute("CREATE INDEX ON :SuperNode(id);") + memgraph.execute("CREATE INDEX ON :SuperNode;") + memgraph.execute("CREATE INDEX ON :Node(id);") + memgraph.execute("CREATE INDEX ON :Node;") + memgraph.execute("CREATE INDEX ON :Node2(id);") + memgraph.execute("CREATE INDEX ON :Node2;") + + memgraph.execute("match (n:Node) match (s:SuperNode {id: 1}) merge (n)<-[:HAS_REL_TO]-(s);") + memgraph.execute("match (n:Node2) match (s:SuperNode {id: 1}) merge (n)<-[:HAS_REL_TO]-(s);") + + query = "explain match (n:Node) match (s:SuperNode {id: 1}) call { with n, s merge (n)<-[:HAS_REL_TO]-(s) } return s union all match (n:Node) match (s:SuperNode {id: 1}) call { with n, s merge (n)<-[:HAS_REL_TO]-(s) } return s;" + expected_explain = [ + f" * Union {{s : s}}", + f" |\\ ", + f" | * Produce {{s}}", + f" | * Accumulate", + f" | * Accumulate", + f" | * Apply", + f" | |\\ ", + f" | | * EmptyResult", + f" | | * Merge", + f" | | |\\ On Match", + f" | | | * Expand (s)-[anon7:HAS_REL_TO]->(n)", + f" | | | * Once", + f" | | |\\ On Create", + f" | | | * CreateExpand (n)<-[anon7:HAS_REL_TO]-(s)", + f" | | | * Once", + f" | | * Produce {{n, s}}", + f" | | * Once", + f" | * ScanAllByLabel (n :Node)", + f" | * ScanAllByLabelPropertyValue (s :SuperNode {{id}})", + f" | * Once", + f" * Produce {{s}}", f" * Accumulate", f" * Accumulate", f" * Apply", @@ -391,7 +422,7 @@ def test_given_supernode_when_subquery_then_carry_information_to_subquery(memgra f" | * EmptyResult", f" | * Merge", f" | |\\ On Match", - f" | | * Expand (n)<-[anon3:HAS_REL_TO]-(s)", + f" | | * Expand (s)-[anon3:HAS_REL_TO]->(n)", f" | | * Once", f" | |\\ On Create", f" | | * CreateExpand (n)<-[anon3:HAS_REL_TO]-(s)", @@ -403,8 +434,24 @@ def test_given_supernode_when_subquery_then_carry_information_to_subquery(memgra f" * Once", ] - assert expected_explain_before_analysis == result_without_analysis - assert expected_explain_after_analysis == result_with_analysis + result_without_analysis = list(memgraph.execute_and_fetch(query)) + result_without_analysis = [x[OptimizeIndexesConstants.QUERY_PLAN] for x in result_without_analysis] + assert expected_explain == result_without_analysis + + memgraph.execute("analyze graph;") + + expected_explain = [ + x.replace(f" | | * Expand (s)-[anon3:HAS_REL_TO]->(n)", f" | | * Expand (n)<-[anon3:HAS_REL_TO]-(s)") + for x in expected_explain + ] + expected_explain = [ + x.replace(f" | | | * Expand (s)-[anon7:HAS_REL_TO]->(n)", f" | | | * Expand (n)<-[anon7:HAS_REL_TO]-(s)") + for x in expected_explain + ] + result_with_analysis = list(memgraph.execute_and_fetch(query)) + result_with_analysis = [x[OptimizeIndexesConstants.QUERY_PLAN] for x in result_with_analysis] + + assert expected_explain == result_with_analysis if __name__ == "__main__": diff --git a/tests/unit/query_semantic.cpp b/tests/unit/query_semantic.cpp index be3fd60d9..ee18380b2 100644 --- a/tests/unit/query_semantic.cpp +++ b/tests/unit/query_semantic.cpp @@ -1252,4 +1252,11 @@ TEST_F(TestSymbolGenerator, Subqueries) { query = QUERY(SINGLE_QUERY(MATCH(PATTERN(NODE("n"))), CALL_SUBQUERY(subquery), RETURN("n", "m"))); symbol_table = MakeSymbolTable(query); ASSERT_EQ(symbol_table.max_position(), 11); + + // MATCH (n) CALL { MATCH (s) RETURN s } RETURN n UNION MATCH (n) CALL { MATCH (s) RETURN s } RETURN n + subquery = QUERY(SINGLE_QUERY(MATCH(PATTERN(NODE("s"))), RETURN("s"))); + query = QUERY(SINGLE_QUERY(MATCH(PATTERN(NODE("n"))), CALL_SUBQUERY(subquery), RETURN("n")), + UNION(SINGLE_QUERY(MATCH(PATTERN(NODE("n"))), CALL_SUBQUERY(subquery), RETURN("n")))); + symbol_table = MakeSymbolTable(query); + ASSERT_EQ(symbol_table.max_position(), 13); }