tursom/memgraph
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix multi-command transaction handling

Browse Source 
Reviewers: buda

Reviewed By: buda

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D2784
This commit is contained in:
Matej Ferencevic 2020-06-10 11:26:13 +02:00
parent 6628d20e5a
commit 3dd393f8eb
3 changed files with 134 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/query/exceptions.hpp
View File

@ -84,6 +84,23 @@ class IndexInMulticommandTxException : public QueryException {
"Index manipulation not allowed in multicommand transactions.") {}
};
class ConstraintInMulticommandTxException : public QueryException {
public:
using QueryException::QueryException;
ConstraintInMulticommandTxException()
: QueryException(
"Constraint manipulation not allowed in multicommand "
"transactions.") {}
};
class InfoInMulticommandTxException : public QueryException {
public:
using QueryException::QueryException;
InfoInMulticommandTxException()
: QueryException(
"Info reporting not allowed in multicommand transactions.") {}
};
/**
* An exception for an illegal operation that can not be detected
* before the query starts executing over data.

46
src/query/interpreter.cpp
View File

@ -598,6 +598,18 @@ PreparedQuery PrepareProfileQuery(
kProfileQueryStart))
<< "Expected stripped query to start with '" << kProfileQueryStart << "'";
// PROFILE isn't allowed inside multi-command (explicit) transactions. This is
// because PROFILE executes each PROFILE'd query and collects additional
// perfomance metadata that it displays to the user instead of the results
// yielded by the query. Because PROFILE has side-effects, each transaction
// that is used to execute a PROFILE query *MUST* be aborted. That isn't
// possible when using multicommand (explicit) transactions (because the user
// controls the lifetime of the transaction) and that is why PROFILE is
// explicitly disabled here in multicommand (explicit) transactions.
// NOTE: Unlike PROFILE, EXPLAIN doesn't have any unwanted side-effects (in
// transaction terms) because it doesn't execute the query, it just prints its
// query plan. That is why EXPLAIN can be used in multicommand (explicit)
// transactions.
if (in_explicit_transaction) {
throw ProfileInMulticommandTxException();
}
@ -769,10 +781,14 @@ PreparedQuery PrepareAuthQuery(
}
PreparedQuery PrepareInfoQuery(
ParsedQuery parsed_query, std::map<std::string, TypedValue> *summary,
InterpreterContext *interpreter_context,
storage::Storage *db,
ParsedQuery parsed_query, bool in_explicit_transaction,
std::map<std::string, TypedValue> *summary,
InterpreterContext *interpreter_context, storage::Storage *db,
utils::MonotonicBufferResource *execution_memory) {
if (in_explicit_transaction) {
throw InfoInMulticommandTxException();
}
auto *info_query = utils::Downcast<InfoQuery>(parsed_query.query);
std::vector<std::string> header;
std::function<
@ -857,9 +873,14 @@ PreparedQuery PrepareInfoQuery(
}
PreparedQuery PrepareConstraintQuery(
ParsedQuery parsed_query, std::map<std::string, TypedValue> *summary,
ParsedQuery parsed_query, bool in_explicit_transaction,
std::map<std::string, TypedValue> *summary,
InterpreterContext *interpreter_context,
utils::MonotonicBufferResource *execution_memory) {
if (in_explicit_transaction) {
throw ConstraintInMulticommandTxException();
}
auto *constraint_query = utils::Downcast<ConstraintQuery>(parsed_query.query);
std::function<void()> handler;
@ -1061,9 +1082,10 @@ Interpreter::Prepare(
// Some queries require an active transaction in order to be prepared.
if (!in_explicit_transaction_ &&
!utils::Downcast<IndexQuery>(parsed_query.query) &&
!utils::Downcast<ConstraintQuery>(parsed_query.query) &&
!utils::Downcast<InfoQuery>(parsed_query.query)) {
(utils::Downcast<CypherQuery>(parsed_query.query) ||
utils::Downcast<ExplainQuery>(parsed_query.query) ||
utils::Downcast<ProfileQuery>(parsed_query.query) ||
utils::Downcast<DumpQuery>(parsed_query.query))) {
db_accessor_.emplace(interpreter_context_->db->Access());
execution_db_accessor_.emplace(&*db_accessor_);
}
@ -1097,12 +1119,12 @@ Interpreter::Prepare(
interpreter_context_, &*execution_db_accessor_, &execution_memory_);
} else if (utils::Downcast<InfoQuery>(parsed_query.query)) {
prepared_query = PrepareInfoQuery(
std::move(parsed_query), &summary_, interpreter_context_,
interpreter_context_->db, &execution_memory_);
std::move(parsed_query), in_explicit_transaction_, &summary_,
interpreter_context_, interpreter_context_->db, &execution_memory_);
} else if (utils::Downcast<ConstraintQuery>(parsed_query.query)) {
prepared_query =
PrepareConstraintQuery(std::move(parsed_query), &summary_,
interpreter_context_, &execution_memory_);
prepared_query = PrepareConstraintQuery(
std::move(parsed_query), in_explicit_transaction_, &summary_,
interpreter_context_, &execution_memory_);
} else {
LOG(FATAL) << "Should not get here -- unknown query type!";
}

90
tests/unit/interpreter.cpp
View File

@ -281,13 +281,6 @@ TEST_F(InterpreterTest, Bfs) {
}
}
TEST_F(InterpreterTest, CreateIndexInMulticommandTransaction) {
Interpret("BEGIN");
ASSERT_THROW(Interpret("CREATE INDEX ON :X(y)"),
query::IndexInMulticommandTxException);
Interpret("ROLLBACK");
}
// Test shortest path end to end.
TEST_F(InterpreterTest, ShortestPath) {
Interpret(
@ -327,6 +320,56 @@ TEST_F(InterpreterTest, ShortestPath) {
}
}
TEST_F(InterpreterTest, CreateLabelIndexInMulticommandTransaction) {
Interpret("BEGIN");
ASSERT_THROW(Interpret("CREATE INDEX ON :X"),
query::IndexInMulticommandTxException);
Interpret("ROLLBACK");
}
TEST_F(InterpreterTest, CreateLabelPropertyIndexInMulticommandTransaction) {
Interpret("BEGIN");
ASSERT_THROW(Interpret("CREATE INDEX ON :X(y)"),
query::IndexInMulticommandTxException);
Interpret("ROLLBACK");
}
TEST_F(InterpreterTest, CreateExistenceConstraintInMulticommandTransaction) {
Interpret("BEGIN");
ASSERT_THROW(Interpret("CREATE CONSTRAINT ON (n:A) ASSERT EXISTS (n.a)"),
query::ConstraintInMulticommandTxException);
Interpret("ROLLBACK");
}
TEST_F(InterpreterTest, CreateUniqueConstraintInMulticommandTransaction) {
Interpret("BEGIN");
ASSERT_THROW(
Interpret("CREATE CONSTRAINT ON (n:A) ASSERT n.a, n.b IS UNIQUE"),
query::ConstraintInMulticommandTxException);
Interpret("ROLLBACK");
}
TEST_F(InterpreterTest, ShowIndexInfoInMulticommandTransaction) {
Interpret("BEGIN");
ASSERT_THROW(Interpret("SHOW INDEX INFO"),
query::InfoInMulticommandTxException);
Interpret("ROLLBACK");
}
TEST_F(InterpreterTest, ShowConstraintInfoInMulticommandTransaction) {
Interpret("BEGIN");
ASSERT_THROW(Interpret("SHOW CONSTRAINT INFO"),
query::InfoInMulticommandTxException);
Interpret("ROLLBACK");
}
TEST_F(InterpreterTest, ShowStorageInfoInMulticommandTransaction) {
Interpret("BEGIN");
ASSERT_THROW(Interpret("SHOW STORAGE INFO"),
query::InfoInMulticommandTxException);
Interpret("ROLLBACK");
}
// NOLINTNEXTLINE(hicpp-special-member-functions)
TEST_F(InterpreterTest, ExistenceConstraintTest) {
Interpret("CREATE CONSTRAINT ON (n:A) ASSERT EXISTS (n.a);");
@ -438,6 +481,32 @@ TEST_F(InterpreterTest, ExplainQuery) {
EXPECT_EQ(interpreter_context_.ast_cache.size(), 2U);
}
TEST_F(InterpreterTest, ExplainQueryInMulticommandTransaction) {
EXPECT_EQ(interpreter_context_.plan_cache.size(), 0U);
EXPECT_EQ(interpreter_context_.ast_cache.size(), 0U);
Interpret("BEGIN");
auto stream = Interpret("EXPLAIN MATCH (n) RETURN *;");
Interpret("COMMIT");
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);
@ -488,6 +557,13 @@ TEST_F(InterpreterTest, ProfileQuery) {
EXPECT_EQ(interpreter_context_.ast_cache.size(), 2U);
}
TEST_F(InterpreterTest, ProfileQueryInMulticommandTransaction) {
Interpret("BEGIN");
ASSERT_THROW(Interpret("PROFILE MATCH (n) RETURN *;"),
query::ProfileInMulticommandTxException);
Interpret("ROLLBACK");
}
TEST_F(InterpreterTest, ProfileQueryWithParams) {
EXPECT_EQ(interpreter_context_.plan_cache.size(), 0U);
EXPECT_EQ(interpreter_context_.ast_cache.size(), 0U);