Add query parameters support

Reviewers: buda

Reviewed By: buda

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D570
This commit is contained in:
Mislav Bradac 2017-07-19 18:14:59 +02:00
parent 0324b61e0b
commit e8a465e4b5
18 changed files with 286 additions and 39 deletions

View File

@ -5,14 +5,15 @@
### Major Features and Improvements
* User specified transaction execution timeout.
* Support for query parameters (except for parameters in place of property maps).
## v0.6.0
### Major Features and Improvements
* AST caching
* Label + property index support
* Different logging setup & format
* AST caching.
* Label + property index support.
* Different logging setup & format.
## v0.5.0
@ -27,7 +28,7 @@
### Bug Fixes and Other Changes
* Fixed race condition in MVCC. Hints exp+aborted race condition prevented.
* Fixed conceptual bug in MVCC GC. Evaluate old records w.r.t. the oldest
* Fixed conceptual bug in MVCC GC. Evaluate old records w.r.t. the oldest.
transaction's id AND snapshot.
* User friendly error messages thrown from the query engine.
@ -35,33 +36,33 @@
### Bug Fixes and Other Changes
* List indexing supported with preceeding IN (for example in query `RETURN 1 IN [[1,2]][0]`)
* List indexing supported with preceeding IN (for example in query `RETURN 1 IN [[1,2]][0]`).
## Build 825
### Major Features and Improvements
* RETURN *, count(*), OPTIONAL MATCH, UNWIND, DISTINCT (except DISTINCT in aggregate functions), list indexing and slicing, escaped labels, IN LIST operator, range function
* RETURN *, count(*), OPTIONAL MATCH, UNWIND, DISTINCT (except DISTINCT in aggregate functions), list indexing and slicing, escaped labels, IN LIST operator, range function.
### Bug Fixes and Other Changes
* TCP_NODELAY -> import should be faster
* Clear hint bits
* TCP_NODELAY -> import should be faster.
* Clear hint bits.
## Build 783
### Major Features and Improvements
* SKIP, LIMIT, ORDER BY
* Math functions
* Initial support for MERGE clause
* SKIP, LIMIT, ORDER BY.
* Math functions.
* Initial support for MERGE clause.
### Bug Fixes and Other Changes
* Unhandled Lock Timeout Exception
* Unhandled Lock Timeout Exception.
## Build 755
### Major Features and Improvements
* MATCH, CREATE, WHERE, SET, REMOVE, DELETE
* MATCH, CREATE, WHERE, SET, REMOVE, DELETE.

View File

@ -1,5 +1,6 @@
#pragma once
#include <map>
#include <string>
#include <glog/logging.h>
@ -59,7 +60,8 @@ State StateIdleResultRun(Session &session, State state) {
try {
DLOG(INFO) << fmt::format("[Run] '{}'", query.Value<std::string>());
auto is_successfully_executed = session.query_engine_.Run(
query.Value<std::string>(), *db_accessor, session.output_stream_);
query.Value<std::string>(), *db_accessor, session.output_stream_,
params.Value<std::map<std::string, query::TypedValue>>());
if (!is_successfully_executed) {
// abort transaction

View File

@ -113,4 +113,21 @@ double ParseDoubleLiteral(const std::string &s) {
throw SemanticException("Couldn't parse string to double");
}
}
std::string ParseParameter(const std::string &s) {
debug_assert(s[0] == '$', "Invalid string passed as parameter name");
if (s[1] != '`') return s.substr(1);
// If parameter name is escaped symbolic name then symbolic name should be
// unescaped and leading and trailing backquote should be removed.
debug_assert(s.size() > 3U && s.back() == '`',
"Invalid string passed as parameter name");
std::string out;
for (int i = 2; i < static_cast<int>(s.size()) - 1; ++i) {
if (s[i] == '`') {
++i;
}
out.push_back(s[i]);
}
return out;
}
}

View File

@ -5,10 +5,12 @@
namespace query {
// These are the functions for parsing literals from opepncypher query.
// These are the functions for parsing literals and parameter names from
// opencypher query.
int64_t ParseIntegerLiteral(const std::string &s);
std::string ParseStringLiteral(const std::string &s);
double ParseDoubleLiteral(const std::string &s);
std::string ParseParameter(const std::string &s);
/**
* Indicates that some part of query execution should

View File

@ -140,7 +140,7 @@ void query::Repl(Dbms &dbms) {
try {
auto dba = dbms.active();
ResultStreamFaker results;
interpeter.Interpret(command, *dba, results);
interpeter.Interpret(command, *dba, results, {});
PrintResults(results);
dba->commit();
} catch (const query::SyntaxException &e) {

View File

@ -14,6 +14,7 @@ namespace fs = std::experimental::filesystem;
#include "query/plan_interface.hpp"
#include "utils/datetime/timestamp.hpp"
#include "utils/dynamic_lib.hpp"
#include "utils/exceptions.hpp"
#include "utils/timer.hpp"
DECLARE_bool(interpret);
@ -66,12 +67,18 @@ class QueryEngine {
* true if query execution was successfull
*/
auto Run(const std::string &query, GraphDbAccessor &db_accessor,
Stream &stream) {
Stream &stream,
const std::map<std::string, query::TypedValue> &params) {
if (FLAGS_interpret) {
interpreter_.Interpret(query, db_accessor, stream);
interpreter_.Interpret(query, db_accessor, stream, params);
return true;
}
if (!params.empty()) {
throw utils::NotYetImplemented(
"Params not yet implemented in compiled queries");
}
utils::Timer parsing_timer;
query::StrippedQuery stripped(query);
auto parsing_time = parsing_timer.Elapsed();

View File

@ -66,6 +66,12 @@ class HintedAbortError : public QueryException {
HintedAbortError() : QueryException("") {}
};
class UnprovidedParameterError : public QueryException {
public:
using QueryException::QueryException;
UnprovidedParameterError() : QueryException("") {}
};
/**
* An exception for an illegal operation that can not be detected
* before the query starts executing over data.

View File

@ -738,8 +738,8 @@ antlrcpp::Any CypherMainVisitor::visitAtom(CypherParser::AtomContext *ctx) {
return static_cast<Expression *>(
ctx->literal()->accept(this).as<BaseLiteral *>());
} else if (ctx->parameter()) {
// TODO: implement other clauses.
throw utils::NotYetImplemented("atom parameters");
return static_cast<Expression *>(
ctx->parameter()->accept(this).as<PrimitiveLiteral *>());
} else if (ctx->parenthesizedExpression()) {
return static_cast<Expression *>(
ctx->parenthesizedExpression()->accept(this));
@ -761,6 +761,15 @@ antlrcpp::Any CypherMainVisitor::visitAtom(CypherParser::AtomContext *ctx) {
throw utils::NotYetImplemented("atom expression '{}'", ctx->getText());
}
antlrcpp::Any CypherMainVisitor::visitParameter(
CypherParser::ParameterContext *ctx) {
return storage_.Create<PrimitiveLiteral>(
ctx->getText(), // Not really important since we do parameter
// substitution by token position not by its name.
// Lookup by name is already done in stage before.
ctx->getStart()->getTokenIndex());
}
antlrcpp::Any CypherMainVisitor::visitLiteral(
CypherParser::LiteralContext *ctx) {
int token_position = ctx->getStart()->getTokenIndex();

View File

@ -419,6 +419,11 @@ class CypherMainVisitor : public antlropencypher::CypherBaseVisitor {
*/
antlrcpp::Any visitAtom(CypherParser::AtomContext *ctx) override;
/**
* @return PrimitiveLiteral*
*/
antlrcpp::Any visitParameter(CypherParser::ParameterContext *ctx) override;
/**
* @return Expression*
*/

View File

@ -28,6 +28,7 @@ StrippedQuery::StrippedQuery(const std::string &query) : original_(query) {
STRING,
INT, // Decimal, octal and hexadecimal.
REAL,
PARAMETER,
ESCAPED_NAME,
UNESCAPED_NAME,
SPACE
@ -50,6 +51,7 @@ StrippedQuery::StrippedQuery(const std::string &query) : original_(query) {
update(MatchOctalInt(i), Token::INT);
update(MatchHexadecimalInt(i), Token::INT);
update(MatchReal(i), Token::REAL);
update(MatchParameter(i), Token::PARAMETER);
update(MatchEscapedName(i), Token::ESCAPED_NAME);
update(MatchUnescapedName(i), Token::UNESCAPED_NAME);
update(MatchWhitespaceAndComments(i), Token::SPACE);
@ -77,8 +79,10 @@ StrippedQuery::StrippedQuery(const std::string &query) : original_(query) {
const auto &token = tokens[i];
// Position is calculated in query after stripping and whitespace
// normalisation, not before. There will be twice as much tokens before
// this one because space tokens will be inserted between every one.
int token_index = token_strings.size() * 2;
// this one because space tokens will be inserted between every one we also
// need to shift token index for every parameter since antlr's parser thinks
// of parameter as two tokens.
int token_index = token_strings.size() * 2 + parameters_.size();
switch (token.first) {
case Token::UNMATCHED:
debug_assert(false, "Shouldn't happen");
@ -113,6 +117,10 @@ StrippedQuery::StrippedQuery(const std::string &query) : original_(query) {
case Token::UNESCAPED_NAME:
token_strings.push_back(token.second);
break;
case Token::PARAMETER:
parameters_[token_index] = ParseParameter(token.second);
token_strings.push_back(token.second);
break;
}
if (token.first != Token::SPACE) {
@ -375,6 +383,19 @@ int StrippedQuery::MatchReal(int start) const {
return i - start;
}
int StrippedQuery::MatchParameter(int start) const {
int len = original_.size();
if (start + 1 == len) return 0;
if (original_[start] != '$') return 0;
int max_len = 0;
max_len = std::max(max_len, MatchUnescapedName(start + 1));
max_len = std::max(max_len, MatchEscapedName(start + 1));
max_len = std::max(max_len, MatchKeyword(start + 1));
max_len = std::max(max_len, MatchDecimalInt(start + 1));
if (max_len == 0) return 0;
return 1 + max_len;
}
int StrippedQuery::MatchEscapedName(int start) const {
int len = original_.size();
int i = start;

View File

@ -48,8 +48,9 @@ class StrippedQuery {
StrippedQuery &operator=(StrippedQuery &&other) = default;
const std::string &query() const { return query_; }
auto &literals() const { return literals_; }
auto &named_expressions() const { return named_exprs_; }
const auto &literals() const { return literals_; }
const auto &named_expressions() const { return named_exprs_; }
const auto &parameters() const { return parameters_; }
HashType hash() const { return hash_; }
private:
@ -63,6 +64,7 @@ class StrippedQuery {
int MatchOctalInt(int start) const;
int MatchHexadecimalInt(int start) const;
int MatchReal(int start) const;
int MatchParameter(int start) const;
int MatchEscapedName(int start) const;
int MatchUnescapedName(int start) const;
int MatchWhitespaceAndComments(int start) const;
@ -74,8 +76,14 @@ class StrippedQuery {
std::string query_;
// Token positions of stripped out literals mapped to their values.
// TODO: Parameters class really doesn't provided anything interesting. This
// could be changed to std::unordered_map, but first we need to rewrite (or
// get rid of) hardcoded queries which expect Parameters.
Parameters literals_;
// Token positions of query parameters mapped to theirs names.
std::unordered_map<int, std::string> parameters_;
// Token positions of nonaliased named expressions in return statement mapped
// to theirs original/unstripped string.
std::unordered_map<int, std::string> named_exprs_;

View File

@ -8,6 +8,7 @@
#include "database/graph_db_accessor.hpp"
#include "query/context.hpp"
#include "query/exceptions.hpp"
#include "query/frontend/ast/cypher_main_visitor.hpp"
#include "query/frontend/opencypher/parser.hpp"
#include "query/frontend/semantic/symbol_generator.hpp"
@ -28,7 +29,8 @@ class Interpreter {
Interpreter() {}
template <typename Stream>
void Interpret(const std::string &query, GraphDbAccessor &db_accessor,
Stream &stream) {
Stream &stream,
const std::map<std::string, TypedValue> &params) {
utils::Timer frontend_timer;
Config config;
Context ctx(config, db_accessor);
@ -37,6 +39,13 @@ class Interpreter {
// stripped query -> high level tree
AstTreeStorage ast_storage = [&]() {
if (!FLAGS_ast_cache) {
// This is totally fine, since we don't really expect anyone to turn off
// the cache.
if (!params.empty()) {
throw utils::NotYetImplemented(
"Params not implemented if ast cache is turned off");
}
// stripped query -> AST
frontend::opencypher::Parser parser(query);
auto low_level_tree = parser.tree();
@ -67,7 +76,20 @@ class Interpreter {
CachedAst(std::move(visitor.storage())))
.first;
}
return it->second.Plug(stripped.literals(), stripped.named_expressions());
// Update literals map with provided parameters.
auto literals = stripped.literals();
for (const auto &param_pair : stripped.parameters()) {
auto param_it = params.find(param_pair.second);
if (param_it == params.end()) {
throw query::UnprovidedParameterError(
fmt::format("Parameter$ {} not provided", param_pair.second));
}
literals.Add(param_pair.first, param_it->second);
}
// Plug literals, parameters and named expressions.
return it->second.Plug(literals, stripped.named_expressions());
}();
auto frontend_time = frontend_timer.Elapsed();

View File

@ -134,7 +134,7 @@ auto ExecuteQueryPlans(QueryEngineT &engine, Dbms &dbms, const fs::path &path,
// Create new db_accessor since one query is associated with one
// transaction.
auto db_accessor = dbms.active();
engine.Run(query, *db_accessor, stream);
engine.Run(query, *db_accessor, stream, {});
}
}

View File

@ -65,7 +65,7 @@ int main(int argc, char *argv[]) {
try {
query_engine.ReloadCustom(query, event.path);
auto db_accessor = dbms.active();
query_engine.Run(query, *db_accessor, stream);
query_engine.Run(query, *db_accessor, stream, {});
} catch (query::PlanCompilationException &e) {
DLOG(ERROR) << fmt::format("Query compilation failed: {}",
e.what());

View File

@ -0,0 +1,68 @@
Feature: Parameters
Scenario: Simple parameter names:
Given an empty graph
And parameters are:
| y | 2 |
| x | 1 |
When executing query:
"""
RETURN $x, $y, 5
"""
Then the result should be:
| $x | $y | 5 |
| 1 | 2 | 5 |
Scenario: Integers as parameter names:
Given an empty graph
And parameters are:
| 0 | 5 |
| 2 | 6 |
When executing query:
"""
RETURN $0, $2
"""
Then the result should be:
| $0 | $2 |
| 5 | 6 |
Scenario: Escaped symbolic names as parameter names:
Given an empty graph
And parameters are:
| a b | 2 |
| a `b | 3 |
When executing query:
"""
RETURN $`a b`, $`a ``b`
"""
Then the result should be:
| $`a b` | $`a ``b` |
| 2 | 3 |
Scenario: Lists as parameters:
Given an empty graph
And parameters are:
| a | [1, 2, 3] |
When executing query:
"""
RETURN $a
"""
Then the result should be:
| $a |
| [1, 2, 3] |
Scenario: Parameters in match:
Given an empty graph
And having executed:
"""
CREATE (a {x : 10})
"""
And parameters are:
| a | 10 |
When executing query:
"""
MATCH (a {x : $a}) RETURN a.x
"""
Then the result should be:
| a.x |
| 10 |

View File

@ -15,19 +15,19 @@ TEST(TransactionTimeout, TransactionTimeout) {
{
ResultStreamFaker stream;
auto dba1 = dbms.active();
engine.Run("MATCH (n) RETURN n", *dba1, stream);
engine.Run("MATCH (n) RETURN n", *dba1, stream, {});
}
{
ResultStreamFaker stream;
auto dba2 = dbms.active();
std::this_thread::sleep_for(std::chrono::seconds(5));
ASSERT_THROW(engine.Run("MATCH (n) RETURN n", *dba2, stream),
ASSERT_THROW(engine.Run("MATCH (n) RETURN n", *dba2, stream, {}),
query::HintedAbortError);
}
{
ResultStreamFaker stream;
auto dba3 = dbms.active();
engine.Run("MATCH (n) RETURN n", *dba3, stream);
engine.Run("MATCH (n) RETURN n", *dba3, stream, {});
}
}

View File

@ -1,9 +1,12 @@
#include "communication/result_stream_faker.hpp"
#include "database/graph_db_accessor.hpp"
#include "database/dbms.hpp"
#include "database/graph_db_accessor.hpp"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "query/engine.hpp"
#include "query/exceptions.hpp"
#include "query/typed_value.hpp"
#include "query_common.hpp"
// TODO: This is not a unit test, but tests/integration dir is chaotic at the
// moment. After tests refactoring is done, move/rename this.
@ -18,7 +21,7 @@ TEST(QueryEngine, AstCache) {
{
ResultStreamFaker stream;
auto dba = dbms.active();
engine.Run("RETURN 2 + 3", *dba, stream);
engine.Run("RETURN 2 + 3", *dba, stream, {});
ASSERT_EQ(stream.GetHeader().size(), 1U);
EXPECT_EQ(stream.GetHeader()[0], "2 + 3");
ASSERT_EQ(stream.GetResults().size(), 1U);
@ -29,7 +32,7 @@ TEST(QueryEngine, AstCache) {
// Cached ast, different literals.
ResultStreamFaker stream;
auto dba = dbms.active();
engine.Run("RETURN 5 + 4", *dba, stream);
engine.Run("RETURN 5 + 4", *dba, stream, {});
ASSERT_EQ(stream.GetResults().size(), 1U);
ASSERT_EQ(stream.GetResults()[0].size(), 1U);
ASSERT_EQ(stream.GetResults()[0][0].Value<int64_t>(), 9);
@ -38,7 +41,7 @@ TEST(QueryEngine, AstCache) {
// Different ast (because of different types).
ResultStreamFaker stream;
auto dba = dbms.active();
engine.Run("RETURN 5.5 + 4", *dba, stream);
engine.Run("RETURN 5.5 + 4", *dba, stream, {});
ASSERT_EQ(stream.GetResults().size(), 1U);
ASSERT_EQ(stream.GetResults()[0].size(), 1U);
ASSERT_EQ(stream.GetResults()[0][0].Value<double>(), 9.5);
@ -47,7 +50,7 @@ TEST(QueryEngine, AstCache) {
// Cached ast, same literals.
ResultStreamFaker stream;
auto dba = dbms.active();
engine.Run("RETURN 2 + 3", *dba, stream);
engine.Run("RETURN 2 + 3", *dba, stream, {});
ASSERT_EQ(stream.GetResults().size(), 1U);
ASSERT_EQ(stream.GetResults()[0].size(), 1U);
ASSERT_EQ(stream.GetResults()[0][0].Value<int64_t>(), 5);
@ -56,7 +59,7 @@ TEST(QueryEngine, AstCache) {
// Cached ast, different literals.
ResultStreamFaker stream;
auto dba = dbms.active();
engine.Run("RETURN 10.5 + 1", *dba, stream);
engine.Run("RETURN 10.5 + 1", *dba, stream, {});
ASSERT_EQ(stream.GetResults().size(), 1U);
ASSERT_EQ(stream.GetResults()[0].size(), 1U);
ASSERT_EQ(stream.GetResults()[0][0].Value<double>(), 11.5);
@ -65,7 +68,7 @@ TEST(QueryEngine, AstCache) {
// Cached ast, same literals, different whitespaces.
ResultStreamFaker stream;
auto dba = dbms.active();
engine.Run("RETURN 10.5 + 1", *dba, stream);
engine.Run("RETURN 10.5 + 1", *dba, stream, {});
ASSERT_EQ(stream.GetResults().size(), 1U);
ASSERT_EQ(stream.GetResults()[0].size(), 1U);
ASSERT_EQ(stream.GetResults()[0][0].Value<double>(), 11.5);
@ -74,7 +77,7 @@ TEST(QueryEngine, AstCache) {
// Cached ast, same literals, different named header.
ResultStreamFaker stream;
auto dba = dbms.active();
engine.Run("RETURN 10.5+1", *dba, stream);
engine.Run("RETURN 10.5+1", *dba, stream, {});
ASSERT_EQ(stream.GetHeader().size(), 1U);
EXPECT_EQ(stream.GetHeader()[0], "10.5+1");
ASSERT_EQ(stream.GetResults().size(), 1U);
@ -82,4 +85,68 @@ TEST(QueryEngine, AstCache) {
ASSERT_EQ(stream.GetResults()[0][0].Value<double>(), 11.5);
}
}
// Run query with same ast multiple times with different parameters.
TEST(QueryEngine, Parameters) {
QueryEngine<ResultStreamFaker> engine;
Dbms dbms;
{
ResultStreamFaker stream;
auto dba = dbms.active();
engine.Run("RETURN $2 + $`a b`", *dba, stream, {{"2", 10}, {"a b", 15}});
ASSERT_EQ(stream.GetHeader().size(), 1U);
EXPECT_EQ(stream.GetHeader()[0], "$2 + $`a b`");
ASSERT_EQ(stream.GetResults().size(), 1U);
ASSERT_EQ(stream.GetResults()[0].size(), 1U);
ASSERT_EQ(stream.GetResults()[0][0].Value<int64_t>(), 25);
}
{
// Not needed parameter.
ResultStreamFaker stream;
auto dba = dbms.active();
engine.Run("RETURN $2 + $`a b`", *dba, stream,
{{"2", 10}, {"a b", 15}, {"c", 10}});
ASSERT_EQ(stream.GetHeader().size(), 1U);
EXPECT_EQ(stream.GetHeader()[0], "$2 + $`a b`");
ASSERT_EQ(stream.GetResults().size(), 1U);
ASSERT_EQ(stream.GetResults()[0].size(), 1U);
ASSERT_EQ(stream.GetResults()[0][0].Value<int64_t>(), 25);
}
{
// Cached ast, different parameters.
ResultStreamFaker stream;
auto dba = dbms.active();
engine.Run("RETURN $2 + $`a b`", *dba, stream,
{{"2", "da"}, {"a b", "ne"}});
ASSERT_EQ(stream.GetResults().size(), 1U);
ASSERT_EQ(stream.GetResults()[0].size(), 1U);
ASSERT_EQ(stream.GetResults()[0][0].Value<std::string>(), "dane");
}
{
// Non-primitive literal.
ResultStreamFaker stream;
auto dba = dbms.active();
engine.Run("RETURN $2", *dba, stream,
{{"2", std::vector<query::TypedValue>{5, 2, 3}}});
ASSERT_EQ(stream.GetResults().size(), 1U);
ASSERT_EQ(stream.GetResults()[0].size(), 1U);
auto result = query::test_common::ToInt64List(
stream.GetResults()[0][0].Value<std::vector<query::TypedValue>>());
ASSERT_THAT(result, testing::ElementsAre(5, 2, 3));
}
{
// Cached ast, unprovided parameter.
ResultStreamFaker stream;
auto dba = dbms.active();
ASSERT_THROW(engine.Run("RETURN $2 + $`a b`", *dba, stream,
{{"2", "da"}, {"ab", "ne"}}),
query::UnprovidedParameterError);
}
}
}
int main(int argc, char **argv) {
google::InitGoogleLogging(argv[0]);
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
}

View File

@ -287,4 +287,16 @@ TEST(QueryStripper, ReturnListsAndFunctionCalls) {
UnorderedElementsAre(Pair(2, "[1,2,[3, 4] , 5]"),
Pair(30, "f(1, 2)"), Pair(44, "3")));
}
TEST(QueryStripper, Parameters) {
StrippedQuery stripped("RETURN $123, $pero, $`mirko ``slavko`");
EXPECT_EQ(stripped.literals().size(), 0);
EXPECT_EQ(stripped.query(), "return $123 , $pero , $`mirko ``slavko`");
EXPECT_THAT(stripped.parameters(),
UnorderedElementsAre(Pair(2, "123"), Pair(7, "pero"),
Pair(12, "mirko `slavko")));
EXPECT_THAT(stripped.named_expressions(),
UnorderedElementsAre(Pair(2, "$123"), Pair(7, "$pero"),
Pair(12, "$`mirko ``slavko`")));
}
}