From b4df6ba0a903c51524304f5f6d5409ce0fbcef3a Mon Sep 17 00:00:00 2001 From: Teon Banek <teon.banek@memgraph.io> Date: Tue, 12 Nov 2019 15:58:05 +0100 Subject: [PATCH] Correctly prepare Explain and Profile Summary: Also test Explain and Profile through Intepreter. Reviewers: mferencevic Reviewed By: mferencevic Subscribers: pullbot Differential Revision: https://phabricator.memgraph.io/D2546 --- src/query/interpreter.cpp | 14 ++--- tests/unit/interpreter.cpp | 101 +++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 7 deletions(-) diff --git a/src/query/interpreter.cpp b/src/query/interpreter.cpp index ee09b674c..32d527c8d 100644 --- a/src/query/interpreter.cpp +++ b/src/query/interpreter.cpp @@ -696,9 +696,9 @@ PreparedQuery PrepareExplainQuery( << "Cypher grammar should not allow other queries in EXPLAIN"; auto cypher_query_plan = CypherQueryToPlan( - parsed_query.stripped_query.hash(), std::move(parsed_query.ast_storage), - utils::Downcast<CypherQuery>(parsed_query.query), parsed_query.parameters, - &interpreter_context->plan_cache, dba); + parsed_inner_query.stripped_query.hash(), + std::move(parsed_inner_query.ast_storage), cypher_query, + parsed_inner_query.parameters, &interpreter_context->plan_cache, dba); std::stringstream printed_plan; plan::PrettyPrint(*dba, &cypher_query_plan->plan(), &printed_plan); @@ -739,7 +739,7 @@ PreparedQuery PrepareProfileQuery( throw ProfileInMulticommandTxException(); } - if (interpreter_context->is_tsc_available) { + if (!interpreter_context->is_tsc_available) { throw QueryException("TSC support is missing for PROFILE"); } @@ -759,9 +759,9 @@ PreparedQuery PrepareProfileQuery( << "Cypher grammar should not allow other queries in PROFILE"; auto cypher_query_plan = CypherQueryToPlan( - parsed_query.stripped_query.hash(), std::move(parsed_query.ast_storage), - utils::Downcast<CypherQuery>(parsed_query.query), parsed_query.parameters, - &interpreter_context->plan_cache, dba); + parsed_inner_query.stripped_query.hash(), + std::move(parsed_inner_query.ast_storage), cypher_query, + parsed_inner_query.parameters, &interpreter_context->plan_cache, dba); return PreparedQuery{ {"OPERATOR", "ACTUAL HITS", "RELATIVE TIME", "ABSOLUTE TIME"}, diff --git a/tests/unit/interpreter.cpp b/tests/unit/interpreter.cpp index df97b6c3a..38134cbeb 100644 --- a/tests/unit/interpreter.cpp +++ b/tests/unit/interpreter.cpp @@ -329,3 +329,104 @@ TEST_F(InterpreterTest, UniqueConstraintTest) { Interpret("MATCH (n:A{a:2, b:2}) DETACH DELETE n"); Interpret("CREATE (n:A{a:2, b:2})"); } + +TEST_F(InterpreterTest, ExplainQuery) { + EXPECT_EQ(interpreter_context_.plan_cache.size(), 0U); + EXPECT_EQ(interpreter_context_.ast_cache.size(), 0U); + auto stream = Interpret("EXPLAIN MATCH (n) RETURN *;"); + ASSERT_EQ(stream.GetHeader().size(), 1U); + EXPECT_EQ(stream.GetHeader().front(), "QUERY PLAN"); + std::vector<std::string> expected_rows{" * Produce {n}", " * ScanAll (n)", + " * Once"}; + ASSERT_EQ(stream.GetResults().size(), expected_rows.size()); + auto expected_it = expected_rows.begin(); + for (const auto &row : stream.GetResults()) { + ASSERT_EQ(row.size(), 1U); + EXPECT_EQ(row.front().ValueString(), *expected_it); + ++expected_it; + } + // We should have a plan cache for MATCH ... + EXPECT_EQ(interpreter_context_.plan_cache.size(), 1U); + // We should have AST cache for EXPLAIN ... and for inner MATCH ... + EXPECT_EQ(interpreter_context_.ast_cache.size(), 2U); + Interpret("MATCH (n) RETURN *;"); + EXPECT_EQ(interpreter_context_.plan_cache.size(), 1U); + EXPECT_EQ(interpreter_context_.ast_cache.size(), 2U); +} + +TEST_F(InterpreterTest, ExplainQueryWithParams) { + EXPECT_EQ(interpreter_context_.plan_cache.size(), 0U); + EXPECT_EQ(interpreter_context_.ast_cache.size(), 0U); + auto stream = Interpret("EXPLAIN MATCH (n) WHERE n.id = $id RETURN *;", + {{"id", PropertyValue(42)}}); + ASSERT_EQ(stream.GetHeader().size(), 1U); + EXPECT_EQ(stream.GetHeader().front(), "QUERY PLAN"); + std::vector<std::string> expected_rows{" * Produce {n}", " * Filter", + " * ScanAll (n)", " * Once"}; + ASSERT_EQ(stream.GetResults().size(), expected_rows.size()); + auto expected_it = expected_rows.begin(); + for (const auto &row : stream.GetResults()) { + ASSERT_EQ(row.size(), 1U); + EXPECT_EQ(row.front().ValueString(), *expected_it); + ++expected_it; + } + // We should have a plan cache for MATCH ... + EXPECT_EQ(interpreter_context_.plan_cache.size(), 1U); + // We should have AST cache for EXPLAIN ... and for inner MATCH ... + EXPECT_EQ(interpreter_context_.ast_cache.size(), 2U); + Interpret("MATCH (n) WHERE n.id = $id RETURN *;", + {{"id", PropertyValue("something else")}}); + EXPECT_EQ(interpreter_context_.plan_cache.size(), 1U); + EXPECT_EQ(interpreter_context_.ast_cache.size(), 2U); +} + +TEST_F(InterpreterTest, ProfileQuery) { + EXPECT_EQ(interpreter_context_.plan_cache.size(), 0U); + EXPECT_EQ(interpreter_context_.ast_cache.size(), 0U); + auto stream = Interpret("PROFILE MATCH (n) RETURN *;"); + std::vector<std::string> expected_header{"OPERATOR", "ACTUAL HITS", + "RELATIVE TIME", "ABSOLUTE TIME"}; + EXPECT_EQ(stream.GetHeader(), expected_header); + std::vector<std::string> expected_rows{"* Produce", "* ScanAll", "* Once"}; + ASSERT_EQ(stream.GetResults().size(), expected_rows.size()); + auto expected_it = expected_rows.begin(); + for (const auto &row : stream.GetResults()) { + ASSERT_EQ(row.size(), 4U); + EXPECT_EQ(row.front().ValueString(), *expected_it); + ++expected_it; + } + // We should have a plan cache for MATCH ... + EXPECT_EQ(interpreter_context_.plan_cache.size(), 1U); + // We should have AST cache for PROFILE ... and for inner MATCH ... + EXPECT_EQ(interpreter_context_.ast_cache.size(), 2U); + Interpret("MATCH (n) RETURN *;"); + EXPECT_EQ(interpreter_context_.plan_cache.size(), 1U); + EXPECT_EQ(interpreter_context_.ast_cache.size(), 2U); +} + +TEST_F(InterpreterTest, ProfileQueryWithParams) { + EXPECT_EQ(interpreter_context_.plan_cache.size(), 0U); + EXPECT_EQ(interpreter_context_.ast_cache.size(), 0U); + auto stream = Interpret("PROFILE MATCH (n) WHERE n.id = $id RETURN *;", + {{"id", PropertyValue(42)}}); + std::vector<std::string> expected_header{"OPERATOR", "ACTUAL HITS", + "RELATIVE TIME", "ABSOLUTE TIME"}; + EXPECT_EQ(stream.GetHeader(), expected_header); + std::vector<std::string> expected_rows{"* Produce", "* Filter", "* ScanAll", + "* Once"}; + ASSERT_EQ(stream.GetResults().size(), expected_rows.size()); + auto expected_it = expected_rows.begin(); + for (const auto &row : stream.GetResults()) { + ASSERT_EQ(row.size(), 4U); + EXPECT_EQ(row.front().ValueString(), *expected_it); + ++expected_it; + } + // We should have a plan cache for MATCH ... + EXPECT_EQ(interpreter_context_.plan_cache.size(), 1U); + // We should have AST cache for PROFILE ... and for inner MATCH ... + EXPECT_EQ(interpreter_context_.ast_cache.size(), 2U); + Interpret("MATCH (n) WHERE n.id = $id RETURN *;", + {{"id", PropertyValue("something else")}}); + EXPECT_EQ(interpreter_context_.plan_cache.size(), 1U); + EXPECT_EQ(interpreter_context_.ast_cache.size(), 2U); +}