From 65c7e42c0975e05e90874dff4d7f75338644dfa8 Mon Sep 17 00:00:00 2001 From: "kyle.cao" Date: Tue, 24 Aug 2021 19:17:16 +0800 Subject: [PATCH] support cypher parameter --- src/common/expression/CMakeLists.txt | 1 + src/common/expression/ExprVisitor.h | 3 ++ src/common/expression/Expression.cpp | 9 ++++ src/common/expression/Expression.h | 1 + src/common/expression/ParameterExpression.cpp | 30 ++++++++++++ src/common/expression/ParameterExpression.h | 47 +++++++++++++++++++ src/common/expression/test/CMakeLists.txt | 18 +++++++ .../expression/test/EncodeDecodeTest.cpp | 6 +++ .../expression/test/ExpressionContextMock.cpp | 2 + .../test/ParameterExpressionTest.cpp | 31 ++++++++++++ src/common/expression/test/TestBase.h | 1 + src/graph/context/QueryContext.cpp | 6 +++ src/graph/context/QueryContext.h | 2 + src/graph/context/QueryExpressionContext.h | 2 +- src/graph/service/GraphService.cpp | 23 ++++++++- src/graph/service/GraphService.h | 10 ++++ src/graph/service/RequestContext.h | 7 +++ src/graph/visitor/DeducePropsVisitor.h | 1 + src/graph/visitor/DeduceTypeVisitor.cpp | 5 ++ src/graph/visitor/DeduceTypeVisitor.h | 2 + src/graph/visitor/EvaluableExprVisitor.h | 5 ++ .../visitor/ExtractFilterExprVisitor.cpp | 5 ++ src/graph/visitor/ExtractFilterExprVisitor.h | 1 + src/graph/visitor/ExtractPropExprVisitor.cpp | 2 + src/graph/visitor/ExtractPropExprVisitor.h | 1 + src/graph/visitor/FindVisitor.cpp | 2 + src/graph/visitor/FindVisitor.h | 1 + src/graph/visitor/FoldConstantExprVisitor.cpp | 5 ++ src/graph/visitor/FoldConstantExprVisitor.h | 2 + src/graph/visitor/RewriteSymExprVisitor.cpp | 5 ++ src/graph/visitor/RewriteSymExprVisitor.h | 1 + src/graph/visitor/RewriteVisitor.h | 1 + src/graph/visitor/VidExtractVisitor.h | 1 + src/interface/graph.thrift | 2 + src/parser/parser.yy | 6 ++- src/storage/query/QueryBaseProcessor-inl.h | 3 +- tests/Makefile | 3 +- tests/common/utils.py | 9 ++-- tests/tck/conftest.py | 30 ++++++++++++ tests/tck/features/yield/parameter.feature | 41 ++++++++++++++++ 40 files changed, 324 insertions(+), 9 deletions(-) create mode 100644 src/common/expression/ParameterExpression.cpp create mode 100644 src/common/expression/ParameterExpression.h create mode 100644 src/common/expression/test/ParameterExpressionTest.cpp create mode 100644 tests/tck/features/yield/parameter.feature diff --git a/src/common/expression/CMakeLists.txt b/src/common/expression/CMakeLists.txt index 1cca3b5321b..0e45b544375 100644 --- a/src/common/expression/CMakeLists.txt +++ b/src/common/expression/CMakeLists.txt @@ -32,6 +32,7 @@ nebula_add_library( PredicateExpression.cpp ListComprehensionExpression.cpp ReduceExpression.cpp + ParameterExpression.cpp ) nebula_add_subdirectory(test) diff --git a/src/common/expression/ExprVisitor.h b/src/common/expression/ExprVisitor.h index abcb96f0ac2..8bdd5f0e5b5 100644 --- a/src/common/expression/ExprVisitor.h +++ b/src/common/expression/ExprVisitor.h @@ -21,6 +21,7 @@ #include "common/expression/LabelExpression.h" #include "common/expression/ListComprehensionExpression.h" #include "common/expression/LogicalExpression.h" +#include "common/expression/ParameterExpression.h" #include "common/expression/PathBuildExpression.h" #include "common/expression/PredicateExpression.h" #include "common/expression/PropertyExpression.h" @@ -89,6 +90,8 @@ class ExprVisitor { virtual void visit(ReduceExpression *expr) = 0; // subscript range expression virtual void visit(SubscriptRangeExpression *expr) = 0; + // parameter expression + virtual void visit(ParameterExpression *expr) = 0; }; } // namespace nebula diff --git a/src/common/expression/Expression.cpp b/src/common/expression/Expression.cpp index 2178b268842..10bf0476e82 100644 --- a/src/common/expression/Expression.cpp +++ b/src/common/expression/Expression.cpp @@ -22,6 +22,7 @@ #include "common/expression/LabelExpression.h" #include "common/expression/ListComprehensionExpression.h" #include "common/expression/LogicalExpression.h" +#include "common/expression/ParameterExpression.h" #include "common/expression/PathBuildExpression.h" #include "common/expression/PredicateExpression.h" #include "common/expression/PropertyExpression.h" @@ -497,6 +498,11 @@ Expression* Expression::decode(ObjectPool* pool, Expression::Decoder& decoder) { exp->resetFrom(decoder); return exp; } + case Expression::Kind::kParam: { + exp = ParameterExpression::make(pool); + exp->resetFrom(decoder); + return exp; + } case Expression::Kind::kTSPrefix: case Expression::Kind::kTSWildcard: case Expression::Kind::kTSRegexp: @@ -719,6 +725,9 @@ std::ostream& operator<<(std::ostream& os, Expression::Kind kind) { case Expression::Kind::kReduce: os << "Reduce"; break; + case Expression::Kind::kParam: + os << "Parameter"; + break; } return os; } diff --git a/src/common/expression/Expression.h b/src/common/expression/Expression.h index 5bee6009a21..ecb8e215f01 100644 --- a/src/common/expression/Expression.h +++ b/src/common/expression/Expression.h @@ -108,6 +108,7 @@ class Expression { kIsNotEmpty, kSubscriptRange, + kParam, }; Expression(ObjectPool* pool, Kind kind); diff --git a/src/common/expression/ParameterExpression.cpp b/src/common/expression/ParameterExpression.cpp new file mode 100644 index 00000000000..237277cb80e --- /dev/null +++ b/src/common/expression/ParameterExpression.cpp @@ -0,0 +1,30 @@ +/* Copyright (c) 2021 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#include "common/expression/ParameterExpression.h" + +#include "common/expression/ExprVisitor.h" + +namespace nebula { + +const Value& ParameterExpression::eval(ExpressionContext& ectx) { return ectx.getVar(name_); } + +std::string ParameterExpression::toString() const { return name_; } + +bool ParameterExpression::operator==(const Expression& rhs) const { + return kind_ == rhs.kind() && name_ == rhs.toString(); +} + +void ParameterExpression::writeTo(Encoder& encoder) const { + encoder << kind_; + encoder << name_; +} + +void ParameterExpression::resetFrom(Decoder& decoder) { name_ = decoder.readStr(); } + +void ParameterExpression::accept(ExprVisitor* visitor) { visitor->visit(this); } + +} // namespace nebula diff --git a/src/common/expression/ParameterExpression.h b/src/common/expression/ParameterExpression.h new file mode 100644 index 00000000000..b478f6fe905 --- /dev/null +++ b/src/common/expression/ParameterExpression.h @@ -0,0 +1,47 @@ +/* Copyright (c) 2021 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#pragma once + +#include "common/expression/Expression.h" + +// The ParameterExpression use for parameterized statement +namespace nebula { + +class ParameterExpression : public Expression { + public: + ParameterExpression& operator=(const ParameterExpression& rhs) = delete; + ParameterExpression& operator=(ParameterExpression&&) = delete; + + static ParameterExpression* make(ObjectPool* pool, const std::string& name = "") { + return pool->add(new ParameterExpression(pool, name)); + } + + bool operator==(const Expression& rhs) const override; + + const Value& eval(ExpressionContext& ctx) override; + + const std::string& name() const { return name_; } + + std::string toString() const override; + + void accept(ExprVisitor* visitor) override; + + Expression* clone() const override { return ParameterExpression::make(pool_, name()); } + + protected: + explicit ParameterExpression(ObjectPool* pool, const std::string& name = "") + : Expression(pool, Kind::kParam), name_(name) {} + + void writeTo(Encoder& encoder) const override; + void resetFrom(Decoder& decoder) override; + + protected: + std::string name_; + Value result_; +}; + +} // namespace nebula diff --git a/src/common/expression/test/CMakeLists.txt b/src/common/expression/test/CMakeLists.txt index 6ea325a9648..7aab89733ca 100644 --- a/src/common/expression/test/CMakeLists.txt +++ b/src/common/expression/test/CMakeLists.txt @@ -272,6 +272,24 @@ nebula_add_test( ${THRIFT_LIBRARIES} ) +nebula_add_test( + NAME param_expression_test + SOURCES ParameterExpressionTest.cpp + OBJECTS + $ + $ + $ + $ + $ + $ + $ + $ + $ + LIBRARIES + gtest + ${THRIFT_LIBRARIES} +) + nebula_add_test( NAME list_comprehension_expression_test SOURCES ListComprehensionExpressionTest.cpp diff --git a/src/common/expression/test/EncodeDecodeTest.cpp b/src/common/expression/test/EncodeDecodeTest.cpp index b5d46c5425c..3d0a327bd55 100644 --- a/src/common/expression/test/EncodeDecodeTest.cpp +++ b/src/common/expression/test/EncodeDecodeTest.cpp @@ -303,6 +303,12 @@ TEST(ExpressionEncodeDecode, LabelExpression) { ASSERT_EQ(*origin, *decoded); } +TEST(ExpressionEncodeDecode, ParameterExpression) { + auto origin = ParameterExpression::make(&pool, "name"); + auto decoded = Expression::decode(&pool, Expression::encode(*origin)); + ASSERT_EQ(*origin, *decoded); +} + TEST(ExpressionEncodeDecode, CaseExpression) { { // CASE 23 WHEN 24 THEN 1 END diff --git a/src/common/expression/test/ExpressionContextMock.cpp b/src/common/expression/test/ExpressionContextMock.cpp index 7b049e72505..45a08f695dc 100644 --- a/src/common/expression/test/ExpressionContextMock.cpp +++ b/src/common/expression/test/ExpressionContextMock.cpp @@ -43,6 +43,8 @@ std::unordered_map ExpressionContextMock::vals_ = { {"path_edge2", Value(Edge("2", "3", 1, "edge", 0, {}))}, {"path_v2", Value(Vertex("3", {}))}, {"path_edge3", Value(Edge("3", "4", 1, "edge", 0, {}))}, + {"param1", Value(1)}, + {"param2", Value(List(std::vector{1, 2, 3, 4, 5, 6, 7, 8}))}, }; Value ExpressionContextMock::getColumn(int32_t index) const { diff --git a/src/common/expression/test/ParameterExpressionTest.cpp b/src/common/expression/test/ParameterExpressionTest.cpp new file mode 100644 index 00000000000..e0f4e48fac4 --- /dev/null +++ b/src/common/expression/test/ParameterExpressionTest.cpp @@ -0,0 +1,31 @@ +/* Copyright (c) 2021 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ +#include "common/expression/test/TestBase.h" + +namespace nebula { + +class ParameterExpressionTest : public ExpressionTest {}; + +TEST_F(ParameterExpressionTest, ParamExprToString) { + auto expr = ParameterExpression::make(&pool, "$param1"); + ASSERT_EQ("$param1", expr->toString()); +} + +TEST_F(ParameterExpressionTest, ParamEvaluate) { + auto expr = ParameterExpression::make(&pool, "param1"); + auto value = Expression::eval(expr, gExpCtxt); + ASSERT_TRUE(value.isInt()); + ASSERT_EQ(1, value.getInt()); +} +} // namespace nebula + +int main(int argc, char **argv) { + testing::InitGoogleTest(&argc, argv); + folly::init(&argc, &argv, true); + google::SetStderrLogging(google::INFO); + + return RUN_ALL_TESTS(); +} diff --git a/src/common/expression/test/TestBase.h b/src/common/expression/test/TestBase.h index 2ba5a6c629a..a79aa13d97c 100644 --- a/src/common/expression/test/TestBase.h +++ b/src/common/expression/test/TestBase.h @@ -34,6 +34,7 @@ #include "common/expression/LabelExpression.h" #include "common/expression/ListComprehensionExpression.h" #include "common/expression/LogicalExpression.h" +#include "common/expression/ParameterExpression.h" #include "common/expression/PathBuildExpression.h" #include "common/expression/PredicateExpression.h" #include "common/expression/PropertyExpression.h" diff --git a/src/graph/context/QueryContext.cpp b/src/graph/context/QueryContext.cpp index 46f7fe02af6..2e7499e8dc9 100644 --- a/src/graph/context/QueryContext.cpp +++ b/src/graph/context/QueryContext.cpp @@ -30,6 +30,12 @@ void QueryContext::init() { objPool_ = std::make_unique(); ep_ = std::make_unique(); ectx_ = std::make_unique(); + // copy parameterMap into ExecutionContext + if (rctx_) { + for (auto item : rctx_->parameterMap()) { + ectx_->setValue(std::move(item.first), std::move(item.second)); + } + } idGen_ = std::make_unique(0); symTable_ = std::make_unique(objPool_.get()); vctx_ = std::make_unique(std::make_unique(symTable_.get())); diff --git a/src/graph/context/QueryContext.h b/src/graph/context/QueryContext.h index c85aed49221..a393b768cd5 100644 --- a/src/graph/context/QueryContext.h +++ b/src/graph/context/QueryContext.h @@ -97,6 +97,8 @@ class QueryContext { bool isKilled() const { return killed_.load(); } + bool existParameter(const std::string& param) const { return ectx_->exist(param); } + private: void init(); diff --git a/src/graph/context/QueryExpressionContext.h b/src/graph/context/QueryExpressionContext.h index 0fb423435a7..f2ae198e4ef 100644 --- a/src/graph/context/QueryExpressionContext.h +++ b/src/graph/context/QueryExpressionContext.h @@ -54,7 +54,7 @@ class QueryExpressionContext final : public ExpressionContext { void setVar(const std::string&, Value val) override; - QueryExpressionContext& operator()(Iterator* iter) { + QueryExpressionContext& operator()(Iterator* iter = nullptr) { iter_ = iter; return *this; } diff --git a/src/graph/service/GraphService.cpp b/src/graph/service/GraphService.cpp index 5bed12c6187..714701882dd 100644 --- a/src/graph/service/GraphService.cpp +++ b/src/graph/service/GraphService.cpp @@ -116,6 +116,14 @@ void GraphService::signout(int64_t sessionId) { folly::Future GraphService::future_execute(int64_t sessionId, const std::string& query) { + std::unordered_map params; + return future_executeWithParameter(sessionId, query, std::move(params)).get(); +} + +folly::Future GraphService::future_executeWithParameter( + int64_t sessionId, + const std::string& query, + const std::unordered_map& parameterMap) { auto ctx = std::make_unique>(); ctx->setQuery(query); ctx->setRunner(getThreadManager()); @@ -129,7 +137,7 @@ folly::Future GraphService::future_execute(int64_t sessionId, ctx->finish(); return future; } - auto cb = [this, sessionId, ctx = std::move(ctx)]( + auto cb = [this, sessionId, ctx = std::move(ctx), parameterMap = std::move(parameterMap)]( StatusOr> ret) mutable { if (!ret.ok()) { LOG(ERROR) << "Get session for sessionId: " << sessionId << " failed: " << ret.status(); @@ -147,6 +155,7 @@ folly::Future GraphService::future_execute(int64_t sessionId, return ctx->finish(); } ctx->setSession(std::move(sessionPtr)); + ctx->setParameterMap(parameterMap); queryEngine_->execute(std::move(ctx)); }; sessionManager_->findSession(sessionId, getThreadManager()).thenValue(std::move(cb)); @@ -155,7 +164,17 @@ folly::Future GraphService::future_execute(int64_t sessionId, folly::Future GraphService::future_executeJson(int64_t sessionId, const std::string& query) { - auto rawResp = future_execute(sessionId, query).get(); + std::unordered_map params; + auto rawResp = future_executeWithParameter(sessionId, query, std::move(params)).get(); + auto respJsonObj = rawResp.toJson(); + return folly::toJson(respJsonObj); +} + +folly::Future GraphService::future_executeJsonWithParameter( + int64_t sessionId, + const std::string& query, + const std::unordered_map& parameterMap) { + auto rawResp = future_executeWithParameter(sessionId, query, parameterMap).get(); auto respJsonObj = rawResp.toJson(); return folly::toJson(respJsonObj); } diff --git a/src/graph/service/GraphService.h b/src/graph/service/GraphService.h index f72c36bb39e..94f7b43c851 100644 --- a/src/graph/service/GraphService.h +++ b/src/graph/service/GraphService.h @@ -36,9 +36,19 @@ class GraphService final : public cpp2::GraphServiceSvIf { folly::Future future_execute(int64_t sessionId, const std::string& stmt) override; + folly::Future future_executeWithParameter( + int64_t sessionId, + const std::string& stmt, + const std::unordered_map& parameterMap) override; + folly::Future future_executeJson(int64_t sessionId, const std::string& stmt) override; + folly::Future future_executeJsonWithParameter( + int64_t sessionId, + const std::string& stmt, + const std::unordered_map& parameterMap) override; + private: bool auth(const std::string& username, const std::string& password); diff --git a/src/graph/service/RequestContext.h b/src/graph/service/RequestContext.h index e74eaf05c3f..9a1a3ec03d5 100644 --- a/src/graph/service/RequestContext.h +++ b/src/graph/service/RequestContext.h @@ -68,6 +68,12 @@ class RequestContext final : public cpp::NonCopyable, public cpp::NonMovable { GraphSessionManager* sessionMgr() const { return sessionMgr_; } + void setParameterMap(std::unordered_map parameterMap) { + parameterMap_ = std::move(parameterMap); + } + + const std::unordered_map& parameterMap() const { return parameterMap_; } + private: time::Duration duration_; std::string query_; @@ -76,6 +82,7 @@ class RequestContext final : public cpp::NonCopyable, public cpp::NonMovable { std::shared_ptr session_; folly::Executor* runner_{nullptr}; GraphSessionManager* sessionMgr_{nullptr}; + std::unordered_map parameterMap_; }; } // namespace graph diff --git a/src/graph/visitor/DeducePropsVisitor.h b/src/graph/visitor/DeducePropsVisitor.h index 7578f792a68..63cf1ced34c 100644 --- a/src/graph/visitor/DeducePropsVisitor.h +++ b/src/graph/visitor/DeducePropsVisitor.h @@ -100,6 +100,7 @@ class DeducePropsVisitor : public ExprVisitorImpl { void visit(VertexExpression* expr) override; void visit(EdgeExpression* expr) override; void visit(ColumnExpression* expr) override; + void visit(ParameterExpression*) override {} void visitEdgePropExpr(PropertyExpression* expr); void reportError(const Expression* expr); diff --git a/src/graph/visitor/DeduceTypeVisitor.cpp b/src/graph/visitor/DeduceTypeVisitor.cpp index f03ccd3b5a3..b71f28b369e 100644 --- a/src/graph/visitor/DeduceTypeVisitor.cpp +++ b/src/graph/visitor/DeduceTypeVisitor.cpp @@ -116,6 +116,11 @@ void DeduceTypeVisitor::visit(ConstantExpression *expr) { type_ = expr->eval(ctx(nullptr)).type(); } +void DeduceTypeVisitor::visit(ParameterExpression *expr) { + QueryExpressionContext ctx(qctx_->ectx()); + type_ = expr->eval(ctx()).type(); +} + void DeduceTypeVisitor::visit(UnaryExpression *expr) { expr->operand()->accept(this); if (!ok()) return; diff --git a/src/graph/visitor/DeduceTypeVisitor.h b/src/graph/visitor/DeduceTypeVisitor.h index a129873206a..04969b9c732 100644 --- a/src/graph/visitor/DeduceTypeVisitor.h +++ b/src/graph/visitor/DeduceTypeVisitor.h @@ -85,6 +85,8 @@ class DeduceTypeVisitor final : public ExprVisitor { void visit(ReduceExpression *expr) override; // subscript range void visit(SubscriptRangeExpression *expr) override; + // parameter expression + void visit(ParameterExpression *expr) override; void visitVertexPropertyExpr(PropertyExpression *expr); diff --git a/src/graph/visitor/EvaluableExprVisitor.h b/src/graph/visitor/EvaluableExprVisitor.h index 8c9270e5640..6378fa1e893 100644 --- a/src/graph/visitor/EvaluableExprVisitor.h +++ b/src/graph/visitor/EvaluableExprVisitor.h @@ -29,6 +29,11 @@ class EvaluableExprVisitor : public ExprVisitorImpl { void visit(VersionedVariableExpression *) override { isEvaluable_ = false; } + void visit(ParameterExpression *) override { + // TODO: ParameterExpression is evaluable but not foldable + isEvaluable_ = false; + } + void visit(TagPropertyExpression *) override { isEvaluable_ = false; } void visit(EdgePropertyExpression *) override { isEvaluable_ = false; } diff --git a/src/graph/visitor/ExtractFilterExprVisitor.cpp b/src/graph/visitor/ExtractFilterExprVisitor.cpp index e43d48062fb..49ccb020844 100644 --- a/src/graph/visitor/ExtractFilterExprVisitor.cpp +++ b/src/graph/visitor/ExtractFilterExprVisitor.cpp @@ -13,6 +13,11 @@ void ExtractFilterExprVisitor::visit(ConstantExpression *) { canBePushed_ = true void ExtractFilterExprVisitor::visit(LabelExpression *) { canBePushed_ = false; } +void ExtractFilterExprVisitor::visit(ParameterExpression *) { + // TODO: support parameter push down + canBePushed_ = false; +} + void ExtractFilterExprVisitor::visit(UUIDExpression *) { canBePushed_ = false; } void ExtractFilterExprVisitor::visit(VariableExpression *expr) { diff --git a/src/graph/visitor/ExtractFilterExprVisitor.h b/src/graph/visitor/ExtractFilterExprVisitor.h index 72051d10d95..d2412b1ce38 100644 --- a/src/graph/visitor/ExtractFilterExprVisitor.h +++ b/src/graph/visitor/ExtractFilterExprVisitor.h @@ -44,6 +44,7 @@ class ExtractFilterExprVisitor final : public ExprVisitorImpl { void visit(LogicalExpression *) override; void visit(ColumnExpression *) override; void visit(SubscriptRangeExpression *) override; + void visit(ParameterExpression *) override; private: ObjectPool *pool_; diff --git a/src/graph/visitor/ExtractPropExprVisitor.cpp b/src/graph/visitor/ExtractPropExprVisitor.cpp index 6f60c26873a..16004fcb7ab 100644 --- a/src/graph/visitor/ExtractPropExprVisitor.cpp +++ b/src/graph/visitor/ExtractPropExprVisitor.cpp @@ -31,6 +31,8 @@ void ExtractPropExprVisitor::visit(VariableExpression* expr) { UNUSED(expr); } void ExtractPropExprVisitor::visit(SubscriptExpression* expr) { reportError(expr); } +void ExtractPropExprVisitor::visit(ParameterExpression*) {} + void ExtractPropExprVisitor::visit(LabelExpression* expr) { reportError(expr); } void ExtractPropExprVisitor::visit(LabelAttributeExpression* expr) { reportError(expr); } diff --git a/src/graph/visitor/ExtractPropExprVisitor.h b/src/graph/visitor/ExtractPropExprVisitor.h index afd311e34be..fd2420cb5d6 100644 --- a/src/graph/visitor/ExtractPropExprVisitor.h +++ b/src/graph/visitor/ExtractPropExprVisitor.h @@ -58,6 +58,7 @@ class ExtractPropExprVisitor final : public ExprVisitorImpl { void visit(SubscriptExpression *) override; // column expression void visit(ColumnExpression *) override; + void visit(ParameterExpression *) override; void visitVertexEdgePropExpr(PropertyExpression *); void visitPropertyExpr(PropertyExpression *); diff --git a/src/graph/visitor/FindVisitor.cpp b/src/graph/visitor/FindVisitor.cpp index ec0d5505975..1c7db063189 100644 --- a/src/graph/visitor/FindVisitor.cpp +++ b/src/graph/visitor/FindVisitor.cpp @@ -183,6 +183,8 @@ void FindVisitor::visit(SubscriptRangeExpression* expr) { } } +void FindVisitor::visit(ParameterExpression* expr) { findInCurrentExpr(expr); } + void FindVisitor::visitBinaryExpr(BinaryExpression* expr) { findInCurrentExpr(expr); if (!needFindAll_ && !foundExprs_.empty()) return; diff --git a/src/graph/visitor/FindVisitor.h b/src/graph/visitor/FindVisitor.h index 64936264416..ea7366fd36c 100644 --- a/src/graph/visitor/FindVisitor.h +++ b/src/graph/visitor/FindVisitor.h @@ -66,6 +66,7 @@ class FindVisitor final : public ExprVisitorImpl { void visit(ColumnExpression* expr) override; void visit(ListComprehensionExpression* expr) override; void visit(SubscriptRangeExpression* expr) override; + void visit(ParameterExpression* expr) override; void visitBinaryExpr(BinaryExpression* expr) override; void findInCurrentExpr(Expression* expr); diff --git a/src/graph/visitor/FoldConstantExprVisitor.cpp b/src/graph/visitor/FoldConstantExprVisitor.cpp index 93b0430b964..917f97832cb 100644 --- a/src/graph/visitor/FoldConstantExprVisitor.cpp +++ b/src/graph/visitor/FoldConstantExprVisitor.cpp @@ -275,6 +275,11 @@ void FoldConstantExprVisitor::visit(DestPropertyExpression *expr) { canBeFolded_ = false; } +void FoldConstantExprVisitor::visit(ParameterExpression *expr) { + UNUSED(expr); + canBeFolded_ = false; +} + void FoldConstantExprVisitor::visit(SourcePropertyExpression *expr) { UNUSED(expr); canBeFolded_ = false; diff --git a/src/graph/visitor/FoldConstantExprVisitor.h b/src/graph/visitor/FoldConstantExprVisitor.h index adae9393e03..2597a142762 100644 --- a/src/graph/visitor/FoldConstantExprVisitor.h +++ b/src/graph/visitor/FoldConstantExprVisitor.h @@ -75,6 +75,8 @@ class FoldConstantExprVisitor final : public ExprVisitor { void visit(ReduceExpression *expr) override; // subscript range expression void visit(SubscriptRangeExpression *expr) override; + // parameter expression + void visit(ParameterExpression *expr) override; void visitBinaryExpr(BinaryExpression *expr); Expression *fold(Expression *expr); diff --git a/src/graph/visitor/RewriteSymExprVisitor.cpp b/src/graph/visitor/RewriteSymExprVisitor.cpp index 52bf0dd578e..983a08ebefc 100644 --- a/src/graph/visitor/RewriteSymExprVisitor.cpp +++ b/src/graph/visitor/RewriteSymExprVisitor.cpp @@ -19,6 +19,11 @@ void RewriteSymExprVisitor::visit(ConstantExpression *expr) { expr_ = nullptr; } +void RewriteSymExprVisitor::visit(ParameterExpression *expr) { + UNUSED(expr); + expr_ = nullptr; +} + void RewriteSymExprVisitor::visit(UnaryExpression *expr) { expr->operand()->accept(this); if (expr_) { diff --git a/src/graph/visitor/RewriteSymExprVisitor.h b/src/graph/visitor/RewriteSymExprVisitor.h index 25795ba209d..84c4628ee97 100644 --- a/src/graph/visitor/RewriteSymExprVisitor.h +++ b/src/graph/visitor/RewriteSymExprVisitor.h @@ -75,6 +75,7 @@ class RewriteSymExprVisitor final : public ExprVisitor { void visit(ReduceExpression *expr) override; // subscript range expression void visit(SubscriptRangeExpression *expr) override; + void visit(ParameterExpression *expr) override; private: void visitBinaryExpr(BinaryExpression *expr); diff --git a/src/graph/visitor/RewriteVisitor.h b/src/graph/visitor/RewriteVisitor.h index 1a688b5c2a5..be7410c0038 100644 --- a/src/graph/visitor/RewriteVisitor.h +++ b/src/graph/visitor/RewriteVisitor.h @@ -83,6 +83,7 @@ class RewriteVisitor final : public ExprVisitorImpl { void visit(VertexExpression*) override {} void visit(EdgeExpression*) override {} void visit(ColumnExpression*) override {} + void visit(ParameterExpression*) override {} void visitBinaryExpr(BinaryExpression*) override; bool care(Expression::Kind kind); diff --git a/src/graph/visitor/VidExtractVisitor.h b/src/graph/visitor/VidExtractVisitor.h index 5f7f480ac79..e03e2cfc306 100644 --- a/src/graph/visitor/VidExtractVisitor.h +++ b/src/graph/visitor/VidExtractVisitor.h @@ -101,6 +101,7 @@ class VidExtractVisitor final : public ExprVisitor { void visit(ReduceExpression *expr) override; // subscript range expression void visit(SubscriptRangeExpression *expr) override; + void visit(ParameterExpression *) override {} private: void visitBinaryExpr(BinaryExpression *expr); diff --git a/src/interface/graph.thrift b/src/interface/graph.thrift index 2a6268bc39d..e2b3c548661 100644 --- a/src/interface/graph.thrift +++ b/src/interface/graph.thrift @@ -105,7 +105,9 @@ service GraphService { oneway void signout(1: i64 sessionId) ExecutionResponse execute(1: i64 sessionId, 2: binary stmt) + ExecutionResponse executeWithParameter(1: i64 sessionId, 2: binary stmt, 3: map(cpp.template = "std::unordered_map") parameterMap) // Same as execute(), but response will be a json string binary executeJson(1: i64 sessionId, 2: binary stmt) + binary executeJsonWithParameter(1: i64 sessionId, 2: binary stmt, 3: map(cpp.template = "std::unordered_map") parameterMap) } diff --git a/src/parser/parser.yy b/src/parser/parser.yy index 6022c7d917e..297cdc713b2 100644 --- a/src/parser/parser.yy +++ b/src/parser/parser.yy @@ -535,7 +535,11 @@ expression delete $1; } | VARIABLE { - $$ = VariableExpression::make(qctx->objPool(), *$1); + if (qctx->existParameter(*$1)) { + $$ = ParameterExpression::make(qctx->objPool(), *$1); + } else { + $$ = VariableExpression::make(qctx->objPool(), *$1); + } delete $1; } | compound_expression { diff --git a/src/storage/query/QueryBaseProcessor-inl.h b/src/storage/query/QueryBaseProcessor-inl.h index 1622a66c1c5..e96667885ff 100644 --- a/src/storage/query/QueryBaseProcessor-inl.h +++ b/src/storage/query/QueryBaseProcessor-inl.h @@ -577,7 +577,8 @@ nebula::cpp2::ErrorCode QueryBaseProcessor::checkExp(const Expression case Expression::Kind::kTSFuzzy: case Expression::Kind::kAggregate: case Expression::Kind::kSubscriptRange: - case Expression::Kind::kVersionedVar: { + case Expression::Kind::kVersionedVar: + case Expression::Kind::kParam: { LOG(ERROR) << "Unimplemented expression type! kind = " << exp->kind(); return nebula::cpp2::ErrorCode::E_INVALID_FILTER; } diff --git a/tests/Makefile b/tests/Makefile index 79d25120ab7..03fd4a2fd84 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -26,7 +26,8 @@ install-deps: pip3 install --user -r $(CURR_DIR)/requirements.txt -i $(PYPI_MIRROR) install-nebula-py: install-deps - git clone --branch master https://github.com/vesoft-inc/nebula-python $(CURR_DIR)/nebula-python + # TODO: just for test (czp) + git clone --branch param-cmd https://github.com/czpmango/nebula-python $(CURR_DIR)/nebula-python cd $(CURR_DIR)/nebula-python \ && pip3 install --user -r requirements.txt -i $(PYPI_MIRROR) \ && python3 setup.py install --user diff --git a/tests/common/utils.py b/tests/common/utils.py index 722298e10b7..238fcbd0de3 100644 --- a/tests/common/utils.py +++ b/tests/common/utils.py @@ -21,6 +21,9 @@ from tests.common.path_value import PathVal from tests.common.types import SpaceDesc +# just for cypher parameter test +params={} + def utf8b(s: str): return bytes(s, encoding='utf-8') @@ -346,8 +349,8 @@ def wrapper(*args, **kwargs): @retry(30) -def try_execute(sess: Session, stmt: str): - return sess.execute(stmt) +def try_execute(sess: Session, stmt: str): + return sess.execute_parameter(stmt, params) def return_if_not_leader_changed(resp) -> bool: @@ -362,7 +365,7 @@ def return_if_not_leader_changed(resp) -> bool: @retry(30, return_if_not_leader_changed) def process_leader_changed(sess: Session, stmt: str): - return sess.execute(stmt) + return sess.execute_parameter(stmt, params) def response(sess: Session, stmt: str, need_try: bool = False): diff --git a/tests/tck/conftest.py b/tests/tck/conftest.py index 663273145b0..f1515f57811 100644 --- a/tests/tck/conftest.py +++ b/tests/tck/conftest.py @@ -11,6 +11,7 @@ import csv import re import threading +import json from nebula2.common.ttypes import Value, ErrorCode from nebula2.data.DataObject import ValueWrapper @@ -28,6 +29,7 @@ check_resp, response, resp_ok, + params, ) from tests.tck.utils.table import dataset, table from tests.tck.utils.nbv import murmurhash2 @@ -113,6 +115,34 @@ def graph_spaces(): return dict(result_set=None) +@given(parse('parameters: {parameters}')) +def preload_parameters( + parameters +): + try: + paramMap = json.loads(parameters) + for (k,v) in paramMap.items(): + params[k]=value(v) + except: + raise ValueError("preload parameters failed!") + + +# construct python-type to nebula.Value +def value(any): + v = Value() + if (isinstance(any, bool)): + v.set_bVal(any) + elif (isinstance(any, int)): + v.set_iVal(any) + elif (isinstance(any, str)): + v.set_sVal(any) + elif (isinstance(any, float)): + v.set_fVal(any) + else: + raise TypeError("Do not support convert "+str(type(any))+" to nebula.Value") + return v + + @given(parse('a graph with space named "{space}"')) def preload_space( request, diff --git a/tests/tck/features/yield/parameter.feature b/tests/tck/features/yield/parameter.feature new file mode 100644 index 00000000000..c26c237ccc1 --- /dev/null +++ b/tests/tck/features/yield/parameter.feature @@ -0,0 +1,41 @@ +# Copyright (c) 2021 vesoft inc. All rights reserved. +# +# This source code is licensed under Apache 2.0 License, +# attached with Common Clause Condition 1.0, found in the LICENSES directory. +Feature: Parameter + + Background: + Given a graph with space named "nba" + Given parameters: {"p1":1,"p2":true,"p3":"Tim Duncan","p4":3.3} + + Scenario: return parameters + When executing query: + """ + RETURN abs($p1)+1 AS ival, $p2 and false AS bval, $p3+"ef" AS sval, round($p4)+1.1 AS fval + """ + Then the result should be, in any order: + | ival | bval | sval | fval | + | 2 | false | "Tim Duncanef" | 4.1 | + + Scenario: match with parameters + When executing query: + """ + MATCH (v:player)-[:like]->(n) WHERE id(v)==$p3 and n.age>$p1+29 + RETURN n.name AS dst + """ + Then the result should be, in any order: + | dst | + | "Manu Ginobili" | + | "Tony Parker" | + + Scenario: var with parameters + When executing query: + """ + $p1=GO FROM "Tim Duncan" OVER like WHERE like.likeness>$p1 yield like._dst as dst; + GO FROM $p1.dst OVER like YIELD DISTINCT $$.player.name AS name + """ + Then the result should be, in any order: + | name | + | "Tim Duncan" | + | "LaMarcus Aldridge" | + | "Manu Ginobili" |