Support CALL ... YIELD * syntax

Reviewers: mferencevic, ipaljak

Reviewed By: mferencevic

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D2592
This commit is contained in:
Teon Banek 2019-12-07 09:59:21 +01:00
parent d68106051c
commit 5067cf1e23
4 changed files with 167 additions and 30 deletions

View File

@ -17,6 +17,7 @@
#include "query/exceptions.hpp"
#include "query/frontend/parsing.hpp"
#include "query/interpret/awesome_memgraph_functions.hpp"
#include "query/procedure/module.hpp"
#include "utils/exceptions.hpp"
#include "utils/string.hpp"
@ -369,24 +370,66 @@ antlrcpp::Any CypherMainVisitor::visitCallProcedure(
}
auto *yield_ctx = ctx->yieldProcedureResults();
if (!yield_ctx) {
// TODO: Standalone CallProcedure clause may omit YIELD only if the function
// never returns anything.
const auto &maybe_found = procedure::FindProcedure(
procedure::gModuleRegistry, call_proc->procedure_name_,
utils::NewDeleteResource());
if (!maybe_found) {
throw SemanticException("There is no procedure named '{}'.",
call_proc->procedure_name_);
}
if (!maybe_found->second->results.empty()) {
throw SemanticException(
"CALL without YIELD may only be used on procedures which do not "
"return any result fields.");
}
// When we return, we will release the lock on modules. This means that
// someone may reload the procedure and change the result signature. But to
// keep the implementation simple, we ignore the case as the rest of the
// code doesn't really care whether we yield or not, so it should not break.
return call_proc;
}
call_proc->result_fields_.reserve(yield_ctx->procedureResult().size());
call_proc->result_identifiers_.reserve(yield_ctx->procedureResult().size());
for (auto *result : yield_ctx->procedureResult()) {
CHECK(result->variable().size() == 1 || result->variable().size() == 2);
call_proc->result_fields_.push_back(
result->variable()[0]->accept(this).as<std::string>());
std::string result_alias;
if (result->variable().size() == 2) {
result_alias = result->variable()[1]->accept(this).as<std::string>();
} else {
result_alias = result->variable()[0]->accept(this).as<std::string>();
if (yield_ctx->getTokens(MemgraphCypher::ASTERISK).empty()) {
call_proc->result_fields_.reserve(yield_ctx->procedureResult().size());
call_proc->result_identifiers_.reserve(yield_ctx->procedureResult().size());
for (auto *result : yield_ctx->procedureResult()) {
CHECK(result->variable().size() == 1 || result->variable().size() == 2);
call_proc->result_fields_.push_back(
result->variable()[0]->accept(this).as<std::string>());
std::string result_alias;
if (result->variable().size() == 2) {
result_alias = result->variable()[1]->accept(this).as<std::string>();
} else {
result_alias = result->variable()[0]->accept(this).as<std::string>();
}
call_proc->result_identifiers_.push_back(
storage_->Create<Identifier>(result_alias));
}
call_proc->result_identifiers_.push_back(
storage_->Create<Identifier>(result_alias));
} else {
const auto &maybe_found = procedure::FindProcedure(
procedure::gModuleRegistry, call_proc->procedure_name_,
utils::NewDeleteResource());
if (!maybe_found) {
throw SemanticException("There is no procedure named '{}'.",
call_proc->procedure_name_);
}
const auto &[module, proc] = *maybe_found;
call_proc->result_fields_.reserve(proc->results.size());
call_proc->result_identifiers_.reserve(proc->results.size());
for (const auto &[result_name, desc] : proc->results) {
bool is_deprecated = desc.second;
if (is_deprecated) continue;
call_proc->result_fields_.emplace_back(result_name);
call_proc->result_identifiers_.push_back(
storage_->Create<Identifier>(std::string(result_name)));
}
// When we leave the scope, we will release the lock on modules. This means
// that someone may reload the procedure and change its result signature. We
// are fine with this, because if new result fields were added then we yield
// the subset of those and that will appear to a user as if they used the
// procedure before reload. Any subsequent `CALL ... YIELD *` will fetch the
// new fields as well. In case the result signature has had some result
// fields removed, then the query execution will report an error that we are
// yielding missing fields. The user can then just retry the query.
}
return call_proc;
}

View File

@ -112,7 +112,7 @@ callProcedure : CALL procedureName '(' ( expression ( ',' expression )* )? ')' (
procedureName : symbolicName ( '.' symbolicName )* ;
yieldProcedureResults : YIELD ( procedureResult ( ',' procedureResult )* ) ;
yieldProcedureResults : YIELD ( '*' | ( procedureResult ( ',' procedureResult )* ) ) ;
procedureResult : ( variable AS variable ) | variable ;

View File

@ -2672,7 +2672,7 @@ TEST_P(CypherMainVisitorTest, DumpDatabase) {
TEST_P(CypherMainVisitorTest, CallProcedureWithDotsInName) {
auto &ast_generator = *GetParam();
auto *query = dynamic_cast<CypherQuery *>(
ast_generator.ParseQuery("CALL proc.with.dots()"));
ast_generator.ParseQuery("CALL proc.with.dots() YIELD res"));
ASSERT_TRUE(query);
ASSERT_TRUE(query->single_query_);
auto *single_query = query->single_query_;
@ -2681,14 +2681,21 @@ TEST_P(CypherMainVisitorTest, CallProcedureWithDotsInName) {
ASSERT_TRUE(call_proc);
ASSERT_EQ(call_proc->procedure_name_, "proc.with.dots");
ASSERT_TRUE(call_proc->arguments_.empty());
ASSERT_TRUE(call_proc->result_fields_.empty());
ASSERT_TRUE(call_proc->result_identifiers_.empty());
std::vector<std::string> identifier_names;
identifier_names.reserve(call_proc->result_identifiers_.size());
for (const auto *identifier : call_proc->result_identifiers_) {
ASSERT_TRUE(identifier->user_declared_);
identifier_names.push_back(identifier->name_);
}
std::vector<std::string> expected_names{"res"};
ASSERT_EQ(identifier_names, expected_names);
ASSERT_EQ(identifier_names, call_proc->result_fields_);
}
TEST_P(CypherMainVisitorTest, CallProcedureWithDashesInName) {
auto &ast_generator = *GetParam();
auto *query = dynamic_cast<CypherQuery *>(
ast_generator.ParseQuery("CALL `proc-with-dashes`()"));
ast_generator.ParseQuery("CALL `proc-with-dashes`() YIELD res"));
ASSERT_TRUE(query);
ASSERT_TRUE(query->single_query_);
auto *single_query = query->single_query_;
@ -2697,8 +2704,15 @@ TEST_P(CypherMainVisitorTest, CallProcedureWithDashesInName) {
ASSERT_TRUE(call_proc);
ASSERT_EQ(call_proc->procedure_name_, "proc-with-dashes");
ASSERT_TRUE(call_proc->arguments_.empty());
ASSERT_TRUE(call_proc->result_fields_.empty());
ASSERT_TRUE(call_proc->result_identifiers_.empty());
std::vector<std::string> identifier_names;
identifier_names.reserve(call_proc->result_identifiers_.size());
for (const auto *identifier : call_proc->result_identifiers_) {
ASSERT_TRUE(identifier->user_declared_);
identifier_names.push_back(identifier->name_);
}
std::vector<std::string> expected_names{"res"};
ASSERT_EQ(identifier_names, expected_names);
ASSERT_EQ(identifier_names, call_proc->result_fields_);
}
TEST_P(CypherMainVisitorTest, CallProcedureWithYieldSomeFields) {
@ -2760,7 +2774,7 @@ TEST_P(CypherMainVisitorTest, CallProcedureWithYieldAliasedFields) {
TEST_P(CypherMainVisitorTest, CallProcedureWithArguments) {
auto &ast_generator = *GetParam();
auto *query = dynamic_cast<CypherQuery *>(
ast_generator.ParseQuery("CALL proc(0, 1, 2)"));
ast_generator.ParseQuery("CALL proc(0, 1, 2) YIELD res"));
ASSERT_TRUE(query);
ASSERT_TRUE(query->single_query_);
auto *single_query = query->single_query_;
@ -2768,13 +2782,84 @@ TEST_P(CypherMainVisitorTest, CallProcedureWithArguments) {
auto *call_proc = dynamic_cast<CallProcedure *>(single_query->clauses_[0]);
ASSERT_TRUE(call_proc);
ASSERT_EQ(call_proc->procedure_name_, "proc");
ASSERT_TRUE(call_proc->result_fields_.empty());
ASSERT_EQ(call_proc->result_identifiers_.size(),
call_proc->result_fields_.size());
ASSERT_EQ(call_proc->arguments_.size(), 3U);
for (int64_t i = 0; i < 3; ++i) {
ast_generator.CheckLiteral(call_proc->arguments_[i], i);
}
std::vector<std::string> identifier_names;
identifier_names.reserve(call_proc->result_identifiers_.size());
for (const auto *identifier : call_proc->result_identifiers_) {
ASSERT_TRUE(identifier->user_declared_);
identifier_names.push_back(identifier->name_);
}
std::vector<std::string> expected_names{"res"};
ASSERT_EQ(identifier_names, expected_names);
ASSERT_EQ(identifier_names, call_proc->result_fields_);
}
TEST_P(CypherMainVisitorTest, CallYieldAsterisk) {
auto &ast_generator = *GetParam();
auto *query = dynamic_cast<CypherQuery *>(
ast_generator.ParseQuery("CALL mg.procedures() YIELD *"));
ASSERT_TRUE(query);
ASSERT_TRUE(query->single_query_);
auto *single_query = query->single_query_;
ASSERT_EQ(single_query->clauses_.size(), 1U);
auto *call_proc = dynamic_cast<CallProcedure *>(single_query->clauses_[0]);
ASSERT_TRUE(call_proc);
ASSERT_EQ(call_proc->procedure_name_, "mg.procedures");
ASSERT_TRUE(call_proc->arguments_.empty());
std::vector<std::string> identifier_names;
identifier_names.reserve(call_proc->result_identifiers_.size());
for (const auto *identifier : call_proc->result_identifiers_) {
ASSERT_TRUE(identifier->user_declared_);
identifier_names.push_back(identifier->name_);
}
std::vector<std::string> expected_names{"name", "signature"};
ASSERT_EQ(identifier_names, expected_names);
ASSERT_EQ(identifier_names, call_proc->result_fields_);
}
TEST_P(CypherMainVisitorTest, CallYieldAsteriskReturnAsterisk) {
auto &ast_generator = *GetParam();
auto *query = dynamic_cast<CypherQuery *>(
ast_generator.ParseQuery("CALL mg.procedures() YIELD * RETURN *"));
ASSERT_TRUE(query);
ASSERT_TRUE(query->single_query_);
auto *single_query = query->single_query_;
ASSERT_EQ(single_query->clauses_.size(), 2U);
auto *ret = dynamic_cast<Return *>(single_query->clauses_[1]);
ASSERT_TRUE(ret);
ASSERT_TRUE(ret->body_.all_identifiers);
auto *call_proc = dynamic_cast<CallProcedure *>(single_query->clauses_[0]);
ASSERT_TRUE(call_proc);
ASSERT_EQ(call_proc->procedure_name_, "mg.procedures");
ASSERT_TRUE(call_proc->arguments_.empty());
std::vector<std::string> identifier_names;
identifier_names.reserve(call_proc->result_identifiers_.size());
for (const auto *identifier : call_proc->result_identifiers_) {
ASSERT_TRUE(identifier->user_declared_);
identifier_names.push_back(identifier->name_);
}
std::vector<std::string> expected_names{"name", "signature"};
ASSERT_EQ(identifier_names, expected_names);
ASSERT_EQ(identifier_names, call_proc->result_fields_);
}
TEST_P(CypherMainVisitorTest, CallWithoutYield) {
auto &ast_generator = *GetParam();
auto *query = dynamic_cast<CypherQuery *>(
ast_generator.ParseQuery("CALL mg.reload_all()"));
ASSERT_TRUE(query);
ASSERT_TRUE(query->single_query_);
auto *single_query = query->single_query_;
ASSERT_EQ(single_query->clauses_.size(), 1U);
auto *call_proc = dynamic_cast<CallProcedure *>(single_query->clauses_[0]);
ASSERT_TRUE(call_proc);
ASSERT_EQ(call_proc->procedure_name_, "mg.reload_all");
ASSERT_TRUE(call_proc->arguments_.empty());
ASSERT_TRUE(call_proc->result_fields_.empty());
ASSERT_TRUE(call_proc->result_identifiers_.empty());
}
TEST_P(CypherMainVisitorTest, IncorrectCallProcedure) {
@ -2803,15 +2888,14 @@ TEST_P(CypherMainVisitorTest, IncorrectCallProcedure) {
ASSERT_THROW(
ast_generator.ParseQuery("RETURN 42 AS x CALL procedure() YIELD res"),
SemanticException);
// mg.procedures returns something, so it needs to have a YIELD.
ASSERT_THROW(ast_generator.ParseQuery("CALL mg.procedures()"),
SemanticException);
// TODO: Implement support for the following syntax. These are defined in
// Neo4j and accepted in openCypher CIP.
ASSERT_THROW(ast_generator.ParseQuery("CALL proc"), SyntaxException);
ASSERT_THROW(ast_generator.ParseQuery("CALL proc RETURN 42"),
SyntaxException);
ASSERT_THROW(ast_generator.ParseQuery("CALL proc() YIELD *"),
SyntaxException);
ASSERT_THROW(ast_generator.ParseQuery("CALL proc() YIELD * RETURN *"),
SyntaxException);
ASSERT_THROW(ast_generator.ParseQuery("CALL proc() YIELD res WHERE res > 42"),
SyntaxException);
ASSERT_THROW(

View File

@ -1187,3 +1187,13 @@ TEST_F(TestSymbolGenerator, CallProcedureUnboundArgument) {
auto query = QUERY(SINGLE_QUERY(call));
EXPECT_THROW(query::MakeSymbolTable(query), SemanticException);
}
TEST_F(TestSymbolGenerator, CallWithoutFieldsReturnAsterisk) {
// CALL proc() RETURN *
auto call = storage.Create<CallProcedure>();
call->procedure_name_ = "proc";
auto ret = storage.Create<Return>();
ret->body_.all_identifiers = true;
auto query = QUERY(SINGLE_QUERY(call, ret));
EXPECT_THROW(query::MakeSymbolTable(query), SemanticException);
}