From f8dd6b723f26724ab967cef49de86c582316027d Mon Sep 17 00:00:00 2001 From: jimingquan Date: Sat, 23 Jul 2022 19:58:53 +0800 Subject: [PATCH 1/9] add unwind --- src/graph/service/PermissionCheck.cpp | 1 + src/graph/validator/CMakeLists.txt | 1 + src/graph/validator/UnwindValidator.cpp | 42 +++++++++++++++++++++++++ src/graph/validator/UnwindValidator.h | 36 +++++++++++++++++++++ src/graph/validator/Validator.cpp | 3 ++ src/parser/Sentence.h | 1 + src/parser/TraverseSentences.cpp | 4 +++ src/parser/TraverseSentences.h | 27 ++++++++++++++++ src/parser/parser.yy | 10 +++++- 9 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 src/graph/validator/UnwindValidator.cpp create mode 100644 src/graph/validator/UnwindValidator.h diff --git a/src/graph/service/PermissionCheck.cpp b/src/graph/service/PermissionCheck.cpp index 8939a249ae5..da0dd3789dd 100644 --- a/src/graph/service/PermissionCheck.cpp +++ b/src/graph/service/PermissionCheck.cpp @@ -131,6 +131,7 @@ namespace graph { case Sentence::Kind::kGetSubgraph: case Sentence::Kind::kLimit: case Sentence::Kind::kGroupBy: + case Sentence::Kind::kUnwind: case Sentence::Kind::kReturn: { return PermissionManager::canReadSchemaOrData(session, vctx); } diff --git a/src/graph/validator/CMakeLists.txt b/src/graph/validator/CMakeLists.txt index ac7dd18981e..9c8c7f66fbf 100644 --- a/src/graph/validator/CMakeLists.txt +++ b/src/graph/validator/CMakeLists.txt @@ -27,6 +27,7 @@ nebula_add_library( FindPathValidator.cpp LookupValidator.cpp MatchValidator.cpp + UnwindValidator.cpp ) nebula_add_subdirectory(test) diff --git a/src/graph/validator/UnwindValidator.cpp b/src/graph/validator/UnwindValidator.cpp new file mode 100644 index 00000000000..cd0d44d219c --- /dev/null +++ b/src/graph/validator/UnwindValidator.cpp @@ -0,0 +1,42 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License. + */ + +#include "graph/validator/UnwindValidator.h" + +#include "graph/context/QueryContext.h" +#include "graph/planner/plan/Query.h" +#include "graph/util/ExpressionUtils.h" +#include "parser/TraverseSentences.h" + +namespace nebula { +namespace graph { + +Status UnwindValidator::validateImpl() { + auto *unwindSentence = static_cast(sentence_); + if (unwindSentence->alias().empty()) { + return Status::SemanticError("Expression in UNWIND must be aliased (use AS)"); + } + alias_ = unwindSentence->alias(); + unwindExpr_ = unwindSentence->expr()->clone(); + if (ExpressionUtils::hasAny(unwindExpr_, {Expression::Kind::kAggregate})) { + return Status::SemanticError("Can't use aggregating expressions in unwind clause, `%s'", + unwindExpr_->toString().c_str()); + } + + auto typeStatus = deduceExprType(unwindExpr_); + NG_RETURN_IF_ERROR(typeStatus); + outputs_.emplace_back(alias_, Value::Type::__EMPTY__); + return Status::OK(); +} + +Status UnwindValidator::toPlan() { + auto *unwind = Unwind::make(qctx_, nullptr, unwindExpr_, alias_); + unwind->setColNames({alias_}); + root_ = tail_ = unwind; + return Status::OK(); +} + +} // namespace graph +} // namespace nebula diff --git a/src/graph/validator/UnwindValidator.h b/src/graph/validator/UnwindValidator.h new file mode 100644 index 00000000000..66aa3aaaa79 --- /dev/null +++ b/src/graph/validator/UnwindValidator.h @@ -0,0 +1,36 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License. + */ + +#ifndef GRAPH_VALIDATOR_UNWINDVALIDATOR_H_ +#define GRAPH_VALIDATOR_UNWINDVALIDATOR_H_ + +#include "graph/planner/plan/Query.h" +#include "graph/validator/Validator.h" + +namespace nebula { + +class ObjectPool; + +namespace graph { + +class QueryContext; + +class UnwindValidator final : public Validator { + public: + UnwindValidator(Sentence *sentence, QueryContext *context) : Validator(sentence, context) {} + + Status validateImpl() override; + + Status toPlan() override; + + private: + Expression *unwindExpr_{nullptr}; + std::string alias_; +}; + +} // namespace graph +} // namespace nebula + +#endif // GRAPH_VALIDATOR_UNWINDVALIDATOR_H_ diff --git a/src/graph/validator/Validator.cpp b/src/graph/validator/Validator.cpp index 58330d75132..aed693d427f 100644 --- a/src/graph/validator/Validator.cpp +++ b/src/graph/validator/Validator.cpp @@ -33,6 +33,7 @@ #include "graph/validator/ReportError.h" #include "graph/validator/SequentialValidator.h" #include "graph/validator/SetValidator.h" +#include "graph/validator/UnwindValidator.h" #include "graph/validator/UseValidator.h" #include "graph/validator/YieldValidator.h" #include "graph/visitor/DeduceTypeVisitor.h" @@ -252,6 +253,8 @@ std::unique_ptr Validator::makeValidator(Sentence* sentence, QueryCon return std::make_unique(sentence, context); case Sentence::Kind::kClearSpace: return std::make_unique(sentence, context); + case Sentence::Kind::kUnwind: + return std::make_unique(sentence, context); case Sentence::Kind::kUnknown: case Sentence::Kind::kReturn: { // nothing diff --git a/src/parser/Sentence.h b/src/parser/Sentence.h index 18b998a4d65..6afcde73784 100644 --- a/src/parser/Sentence.h +++ b/src/parser/Sentence.h @@ -132,6 +132,7 @@ class Sentence { kShowMetaLeader, kAlterSpace, kClearSpace, + kUnwind, }; Kind kind() const { diff --git a/src/parser/TraverseSentences.cpp b/src/parser/TraverseSentences.cpp index 9114bed6124..828c9e0bed3 100644 --- a/src/parser/TraverseSentences.cpp +++ b/src/parser/TraverseSentences.cpp @@ -39,6 +39,10 @@ std::string GoSentence::toString() const { return buf; } +std::string UnwindSentence::toString() const { + return ""; +} + LookupSentence::LookupSentence(std::string *from, WhereClause *where, YieldClause *yield) : Sentence(Kind::kLookup), from_(DCHECK_NOTNULL(from)), diff --git a/src/parser/TraverseSentences.h b/src/parser/TraverseSentences.h index f4044e0aae1..54d14713aa4 100644 --- a/src/parser/TraverseSentences.h +++ b/src/parser/TraverseSentences.h @@ -78,6 +78,33 @@ class GoSentence final : public Sentence { std::unique_ptr truncateClause_; }; +class UnwindSentence final : public Sentence { + public: + UnwindSentence(Expression* expr, const std::string& alias) { + expr_ = expr; + alias_ = alias; + kind_ = Kind::kUnwind; + } + + Expression* expr() { + return expr_; + } + + const Expression* expr() const { + return expr_; + } + + const std::string& alias() const { + return alias_; + } + + std::string toString() const override; + + private: + Expression* expr_{nullptr}; + std::string alias_; +}; + class LookupSentence final : public Sentence { public: LookupSentence(std::string* from, WhereClause* where, YieldClause* yield); diff --git a/src/parser/parser.yy b/src/parser/parser.yy index 05e96dfa713..e9f5be4b63a 100644 --- a/src/parser/parser.yy +++ b/src/parser/parser.yy @@ -390,7 +390,7 @@ using namespace nebula; %type update_vertex_sentence update_edge_sentence %type download_sentence ingest_sentence -%type traverse_sentence +%type traverse_sentence unwind_sentence %type go_sentence match_sentence lookup_sentence find_path_sentence get_subgraph_sentence %type group_by_sentence order_by_sentence limit_sentence %type fetch_sentence fetch_vertices_sentence fetch_edges_sentence @@ -1673,6 +1673,13 @@ unwind_clause } ; +unwind_sentence + : KW_UNWIND expression KW_AS name_label { + $$ = new UnwindSentence($2, *$4); + delete $4; + } + ; + with_clause : KW_WITH match_return_items match_order_by match_skip match_limit where_clause { $$ = new WithClause($2, $3, $4, $5, $6, false/*distinct*/); @@ -2939,6 +2946,7 @@ traverse_sentence piped_sentence : traverse_sentence { $$ = $1; } + | unwind_sentence { $$ = $1; } | piped_sentence PIPE traverse_sentence { $$ = new PipedSentence($1, $3); } | piped_sentence PIPE limit_sentence { $$ = new PipedSentence($1, $3); } ; From 08ce4dcdde55e370cd80975ecb4740eb11674d6c Mon Sep 17 00:00:00 2001 From: jimingquan Date: Sat, 23 Jul 2022 21:44:58 +0800 Subject: [PATCH 2/9] check --- src/graph/executor/StorageAccessExecutor.cpp | 16 ++++++++++++---- src/graph/executor/StorageAccessExecutor.h | 5 ++++- .../executor/query/AppendVerticesExecutor.cpp | 2 +- src/graph/executor/query/GetEdgesExecutor.cpp | 3 +++ src/graph/validator/FetchEdgesValidator.cpp | 18 +++++++++++++----- src/graph/validator/FetchEdgesValidator.h | 2 +- src/graph/validator/Validator.cpp | 11 ++++++----- .../test/GetSubgraphValidatorTest.cpp | 6 ++---- 8 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/graph/executor/StorageAccessExecutor.cpp b/src/graph/executor/StorageAccessExecutor.cpp index 48fcdb58d3f..7df1a35ac53 100644 --- a/src/graph/executor/StorageAccessExecutor.cpp +++ b/src/graph/executor/StorageAccessExecutor.cpp @@ -41,7 +41,8 @@ DataSet buildRequestDataSet(const SpaceInfo &space, QueryExpressionContext &exprCtx, Iterator *iter, Expression *expr, - bool dedup) { + bool dedup, + bool isCypher) { DCHECK(iter && expr) << "iter=" << iter << ", expr=" << expr; nebula::DataSet vertices({kVid}); auto s = iter->size(); @@ -54,7 +55,13 @@ DataSet buildRequestDataSet(const SpaceInfo &space, for (; iter->valid(); iter->next()) { auto vid = expr->eval(exprCtx(iter)); + if (vid.empty()) { + continue; + } if (!SchemaUtil::isValidVid(vid, vidType)) { + if (isCypher) { + continue; + } LOG(WARNING) << "Mismatched vid type: " << vid.type() << ", space vid type: " << SchemaUtil::typeToString(vidType); continue; @@ -75,14 +82,15 @@ bool StorageAccessExecutor::isIntVidType(const SpaceInfo &space) const { DataSet StorageAccessExecutor::buildRequestDataSetByVidType(Iterator *iter, Expression *expr, - bool dedup) { + bool dedup, + bool isCypher) { const auto &space = qctx()->rctx()->session()->space(); QueryExpressionContext exprCtx(qctx()->ectx()); if (isIntVidType(space)) { - return internal::buildRequestDataSet(space, exprCtx, iter, expr, dedup); + return internal::buildRequestDataSet(space, exprCtx, iter, expr, dedup, isCypher); } - return internal::buildRequestDataSet(space, exprCtx, iter, expr, dedup); + return internal::buildRequestDataSet(space, exprCtx, iter, expr, dedup, isCypher); } std::string StorageAccessExecutor::getStorageDetail( diff --git a/src/graph/executor/StorageAccessExecutor.h b/src/graph/executor/StorageAccessExecutor.h index 0978ca5bba3..f6dae4a1197 100644 --- a/src/graph/executor/StorageAccessExecutor.h +++ b/src/graph/executor/StorageAccessExecutor.h @@ -158,7 +158,10 @@ class StorageAccessExecutor : public Executor { bool isIntVidType(const SpaceInfo &space) const; - DataSet buildRequestDataSetByVidType(Iterator *iter, Expression *expr, bool dedup); + DataSet buildRequestDataSetByVidType(Iterator *iter, + Expression *expr, + bool dedup, + bool isCypher = false); }; } // namespace graph diff --git a/src/graph/executor/query/AppendVerticesExecutor.cpp b/src/graph/executor/query/AppendVerticesExecutor.cpp index d64a470cbe8..221dd50461d 100644 --- a/src/graph/executor/query/AppendVerticesExecutor.cpp +++ b/src/graph/executor/query/AppendVerticesExecutor.cpp @@ -21,7 +21,7 @@ DataSet AppendVerticesExecutor::buildRequestDataSet(const AppendVertices *av) { return nebula::DataSet({kVid}); } auto valueIter = ectx_->getResult(av->inputVar()).iter(); - return buildRequestDataSetByVidType(valueIter.get(), av->src(), av->dedup()); + return buildRequestDataSetByVidType(valueIter.get(), av->src(), av->dedup(), true); } folly::Future AppendVerticesExecutor::appendVertices() { diff --git a/src/graph/executor/query/GetEdgesExecutor.cpp b/src/graph/executor/query/GetEdgesExecutor.cpp index a129df1d3d2..a18541ab314 100644 --- a/src/graph/executor/query/GetEdgesExecutor.cpp +++ b/src/graph/executor/query/GetEdgesExecutor.cpp @@ -5,6 +5,7 @@ #include "graph/executor/query/GetEdgesExecutor.h" #include "graph/planner/plan/Query.h" +#include "graph/util/SchemaUtil.h" using nebula::storage::StorageClient; using nebula::storage::StorageRpcResponse; @@ -25,6 +26,7 @@ DataSet GetEdgesExecutor::buildRequestDataSet(const GetEdges *ge) { edges.rows.reserve(valueIter->size()); std::unordered_set> uniqueEdges; uniqueEdges.reserve(valueIter->size()); + const auto &space = qctx()->rctx()->session()->space(); for (; valueIter->valid(); valueIter->next()) { auto type = ge->type()->eval(exprCtx(valueIter.get())); auto src = ge->src()->eval(exprCtx(valueIter.get())); @@ -53,6 +55,7 @@ folly::Future GetEdgesExecutor::getEdges() { } auto edges = buildRequestDataSet(ge); + NG_RETURN_IF_ERROR(res); if (edges.rows.empty()) { // TODO: add test for empty input. diff --git a/src/graph/validator/FetchEdgesValidator.cpp b/src/graph/validator/FetchEdgesValidator.cpp index bcc45c9f6fd..5c155e4515f 100644 --- a/src/graph/validator/FetchEdgesValidator.cpp +++ b/src/graph/validator/FetchEdgesValidator.cpp @@ -42,8 +42,7 @@ Status FetchEdgesValidator::validateEdgeName() { // Check validity of edge key(src, type, rank, dst) // from Input/Variable expression specified in sentence -StatusOr FetchEdgesValidator::validateEdgeRef(const Expression *expr, - Value::Type type) { +StatusOr FetchEdgesValidator::validateEdgeRef(const Expression *expr) { const auto &kind = expr->kind(); if (kind != Expression::Kind::kInputProperty && kind != EdgeExpression::Kind::kVarProperty) { return Status::SemanticError("`%s', only input and variable expression is acceptable", @@ -72,13 +71,22 @@ Status FetchEdgesValidator::validateEdgeKey() { std::string inputVarName; if (sentence->isRef()) { // edge keys from Input/Variable auto *srcExpr = sentence->ref()->srcid(); - auto result = validateEdgeRef(srcExpr, vidType_); + auto result = validateEdgeRef(srcExpr); NG_RETURN_IF_ERROR(result); inputVarName = std::move(result).value(); auto *rankExpr = sentence->ref()->rank(); if (rankExpr->kind() != Expression::Kind::kConstant) { - result = validateEdgeRef(rankExpr, Value::Type::INT); + auto rankType = deduceExprType(rankExpr); + NG_RETURN_IF_ERROR(rankType); + if (rankType.value() != Value::Type::INT) { + std::stringstream ss; + ss << "`" << rankExpr->toString() << "' should be type of INT, but was " + << rankType.value(); + return Status::SemanticError(ss.str()); + } + + result = validateEdgeRef(rankExpr); NG_RETURN_IF_ERROR(result); if (inputVarName != result.value()) { return Status::SemanticError( @@ -88,7 +96,7 @@ Status FetchEdgesValidator::validateEdgeKey() { } auto *dstExpr = sentence->ref()->dstid(); - result = validateEdgeRef(dstExpr, vidType_); + result = validateEdgeRef(dstExpr); NG_RETURN_IF_ERROR(result); if (inputVarName != result.value()) { return Status::SemanticError( diff --git a/src/graph/validator/FetchEdgesValidator.h b/src/graph/validator/FetchEdgesValidator.h index 0d1ce562640..2f652af5196 100644 --- a/src/graph/validator/FetchEdgesValidator.h +++ b/src/graph/validator/FetchEdgesValidator.h @@ -22,7 +22,7 @@ class FetchEdgesValidator final : public Validator { Status validateEdgeName(); - StatusOr validateEdgeRef(const Expression* expr, Value::Type type); + StatusOr validateEdgeRef(const Expression* expr); Status validateEdgeKey(); diff --git a/src/graph/validator/Validator.cpp b/src/graph/validator/Validator.cpp index aed693d427f..a98dde58ad6 100644 --- a/src/graph/validator/Validator.cpp +++ b/src/graph/validator/Validator.cpp @@ -452,15 +452,16 @@ Status Validator::validateStarts(const VerticesClause* clause, Starts& starts) { src->toString().c_str()); } starts.fromType = src->kind() == Expression::Kind::kInputProperty ? kPipe : kVariable; - auto type = deduceExprType(src); - if (!type.ok()) { - return type.status(); + auto res = deduceExprType(src); + if (!res.ok()) { + return res.status(); } auto vidType = space_.spaceDesc.vid_type_ref().value().get_type(); - if (type.value() != SchemaUtil::propTypeToValueType(vidType)) { + auto& type = res.value(); + if (type != Value::Type::__EMPTY__ && type != SchemaUtil::propTypeToValueType(vidType)) { std::stringstream ss; ss << "`" << src->toString() << "', the srcs should be type of " - << apache::thrift::util::enumNameSafe(vidType) << ", but was`" << type.value() << "'"; + << apache::thrift::util::enumNameSafe(vidType) << ", but was`" << type << "'"; return Status::SemanticError(ss.str()); } starts.originalSrc = src; diff --git a/src/graph/validator/test/GetSubgraphValidatorTest.cpp b/src/graph/validator/test/GetSubgraphValidatorTest.cpp index 8eae1c3b757..57b4ea6324a 100644 --- a/src/graph/validator/test/GetSubgraphValidatorTest.cpp +++ b/src/graph/validator/test/GetSubgraphValidatorTest.cpp @@ -218,8 +218,7 @@ TEST_F(GetSubgraphValidatorTest, RefNotExist) { "PROP FROM $-.id YIELD vertices as nodes"; auto result = checkResult(query); EXPECT_EQ(std::string(result.message()), - "SemanticError: `$-.id', the srcs should be type of " - "FIXED_STRING, but was`INT'"); + "SemanticError: `$-.id', the srcs should be type of FIXED_STRING, but was`INT'"); } { std::string query = @@ -227,8 +226,7 @@ TEST_F(GetSubgraphValidatorTest, RefNotExist) { "FROM $a.ID YIELD edges as relationships"; auto result = checkResult(query); EXPECT_EQ(std::string(result.message()), - "SemanticError: `$a.ID', the srcs should be type of " - "FIXED_STRING, but was`INT'"); + "SemanticError: `$a.ID', the srcs should be type of FIXED_STRING, but was`INT'"); } { std::string query = From ca7a8eaa9c15d4846dd5e8b81109ee4e54081d6c Mon Sep 17 00:00:00 2001 From: jimingquan Date: Sat, 23 Jul 2022 22:00:15 +0800 Subject: [PATCH 3/9] check vidType when execute, not validate --- src/graph/executor/StorageAccessExecutor.cpp | 28 ++++++++++--------- src/graph/executor/StorageAccessExecutor.h | 8 +++--- .../executor/query/AppendVerticesExecutor.cpp | 6 ++-- .../executor/query/AppendVerticesExecutor.h | 2 +- src/graph/executor/query/GetEdgesExecutor.cpp | 21 ++++++++++++-- src/graph/executor/query/GetEdgesExecutor.h | 2 +- .../executor/query/GetNeighborsExecutor.cpp | 6 ++-- .../executor/query/GetNeighborsExecutor.h | 2 +- .../executor/query/GetVerticesExecutor.cpp | 6 ++-- .../executor/query/GetVerticesExecutor.h | 2 +- src/graph/executor/test/GetNeighborsTest.cpp | 3 +- src/graph/validator/FetchEdgesValidator.cpp | 6 ---- src/parser/TraverseSentences.cpp | 10 ++++++- 13 files changed, 65 insertions(+), 37 deletions(-) diff --git a/src/graph/executor/StorageAccessExecutor.cpp b/src/graph/executor/StorageAccessExecutor.cpp index 7df1a35ac53..86b100a6076 100644 --- a/src/graph/executor/StorageAccessExecutor.cpp +++ b/src/graph/executor/StorageAccessExecutor.cpp @@ -37,12 +37,12 @@ struct Vid { }; template -DataSet buildRequestDataSet(const SpaceInfo &space, - QueryExpressionContext &exprCtx, - Iterator *iter, - Expression *expr, - bool dedup, - bool isCypher) { +StatusOr buildRequestDataSet(const SpaceInfo &space, + QueryExpressionContext &exprCtx, + Iterator *iter, + Expression *expr, + bool dedup, + bool isCypher) { DCHECK(iter && expr) << "iter=" << iter << ", expr=" << expr; nebula::DataSet vertices({kVid}); auto s = iter->size(); @@ -62,9 +62,11 @@ DataSet buildRequestDataSet(const SpaceInfo &space, if (isCypher) { continue; } - LOG(WARNING) << "Mismatched vid type: " << vid.type() - << ", space vid type: " << SchemaUtil::typeToString(vidType); - continue; + std::stringstream ss; + ss << "`" << vid.toString() << "', the srcs should be type of " + << apache::thrift::util::enumNameSafe(vidType.get_type()) << ", but was`" << vid.type() + << "'"; + return Status::Error(ss.str()); } if (dedup && !uniqueSet.emplace(Vid::value(vid)).second) { continue; @@ -80,10 +82,10 @@ bool StorageAccessExecutor::isIntVidType(const SpaceInfo &space) const { return (*space.spaceDesc.vid_type_ref()).type == nebula::cpp2::PropertyType::INT64; } -DataSet StorageAccessExecutor::buildRequestDataSetByVidType(Iterator *iter, - Expression *expr, - bool dedup, - bool isCypher) { +StatusOr StorageAccessExecutor::buildRequestDataSetByVidType(Iterator *iter, + Expression *expr, + bool dedup, + bool isCypher) { const auto &space = qctx()->rctx()->session()->space(); QueryExpressionContext exprCtx(qctx()->ectx()); diff --git a/src/graph/executor/StorageAccessExecutor.h b/src/graph/executor/StorageAccessExecutor.h index f6dae4a1197..0e18c9333bb 100644 --- a/src/graph/executor/StorageAccessExecutor.h +++ b/src/graph/executor/StorageAccessExecutor.h @@ -158,10 +158,10 @@ class StorageAccessExecutor : public Executor { bool isIntVidType(const SpaceInfo &space) const; - DataSet buildRequestDataSetByVidType(Iterator *iter, - Expression *expr, - bool dedup, - bool isCypher = false); + StatusOr buildRequestDataSetByVidType(Iterator *iter, + Expression *expr, + bool dedup, + bool isCypher = false); }; } // namespace graph diff --git a/src/graph/executor/query/AppendVerticesExecutor.cpp b/src/graph/executor/query/AppendVerticesExecutor.cpp index 221dd50461d..509e5cb4c2a 100644 --- a/src/graph/executor/query/AppendVerticesExecutor.cpp +++ b/src/graph/executor/query/AppendVerticesExecutor.cpp @@ -16,7 +16,7 @@ folly::Future AppendVerticesExecutor::execute() { return appendVertices(); } -DataSet AppendVerticesExecutor::buildRequestDataSet(const AppendVertices *av) { +StatusOr AppendVerticesExecutor::buildRequestDataSet(const AppendVertices *av) { if (av == nullptr) { return nebula::DataSet({kVid}); } @@ -30,7 +30,9 @@ folly::Future AppendVerticesExecutor::appendVertices() { auto *av = asNode(node()); StorageClient *storageClient = qctx()->getStorageClient(); - DataSet vertices = buildRequestDataSet(av); + auto res = buildRequestDataSet(av); + NG_RETURN_IF_ERROR(res); + auto vertices = std::move(res).value(); if (vertices.rows.empty()) { return finish(ResultBuilder().value(Value(DataSet(av->colNames()))).build()); } diff --git a/src/graph/executor/query/AppendVerticesExecutor.h b/src/graph/executor/query/AppendVerticesExecutor.h index bbe3d697d54..7138fdb6a08 100644 --- a/src/graph/executor/query/AppendVerticesExecutor.h +++ b/src/graph/executor/query/AppendVerticesExecutor.h @@ -22,7 +22,7 @@ class AppendVerticesExecutor final : public GetPropExecutor { folly::Future execute() override; private: - DataSet buildRequestDataSet(const AppendVertices *gv); + StatusOr buildRequestDataSet(const AppendVertices *gv); folly::Future appendVertices(); diff --git a/src/graph/executor/query/GetEdgesExecutor.cpp b/src/graph/executor/query/GetEdgesExecutor.cpp index a18541ab314..8e248df8b9b 100644 --- a/src/graph/executor/query/GetEdgesExecutor.cpp +++ b/src/graph/executor/query/GetEdgesExecutor.cpp @@ -18,7 +18,7 @@ folly::Future GetEdgesExecutor::execute() { return getEdges(); } -DataSet GetEdgesExecutor::buildRequestDataSet(const GetEdges *ge) { +StatusOr GetEdgesExecutor::buildRequestDataSet(const GetEdges *ge) { auto valueIter = ectx_->getResult(ge->inputVar()).iter(); QueryExpressionContext exprCtx(qctx()->ectx()); @@ -26,12 +26,28 @@ DataSet GetEdgesExecutor::buildRequestDataSet(const GetEdges *ge) { edges.rows.reserve(valueIter->size()); std::unordered_set> uniqueEdges; uniqueEdges.reserve(valueIter->size()); + const auto &space = qctx()->rctx()->session()->space(); + const auto &vidType = *(space.spaceDesc.vid_type_ref()); for (; valueIter->valid(); valueIter->next()) { auto type = ge->type()->eval(exprCtx(valueIter.get())); auto src = ge->src()->eval(exprCtx(valueIter.get())); auto dst = ge->dst()->eval(exprCtx(valueIter.get())); auto rank = ge->ranking()->eval(exprCtx(valueIter.get())); + if (!SchemaUtil::isValidVid(src, vidType)) { + std::stringstream ss; + ss << "`" << src.toString() << "', the src should be type of " + << apache::thrift::util::enumNameSafe(vidType.get_type()) << ", but was`" << src.type() + << "'"; + return Status::Error(ss.str()); + } + if (!SchemaUtil::isValidVid(dst, vidType)) { + std::stringstream ss; + ss << "`" << dst.toString() << "', the dst should be type of " + << apache::thrift::util::enumNameSafe(vidType.get_type()) << ", but was`" << dst.type() + << "'"; + return Status::Error(ss.str()); + } type = type < 0 ? -type : type; auto edgeKey = std::make_tuple(src, type, rank, dst); if (ge->dedup() && !uniqueEdges.emplace(std::move(edgeKey)).second) { @@ -54,8 +70,9 @@ folly::Future GetEdgesExecutor::getEdges() { return Status::Error("ptr is nullptr"); } - auto edges = buildRequestDataSet(ge); + auto res = buildRequestDataSet(ge); NG_RETURN_IF_ERROR(res); + auto edges = std::move(res).value(); if (edges.rows.empty()) { // TODO: add test for empty input. diff --git a/src/graph/executor/query/GetEdgesExecutor.h b/src/graph/executor/query/GetEdgesExecutor.h index 39802c21f43..20446d87d23 100644 --- a/src/graph/executor/query/GetEdgesExecutor.h +++ b/src/graph/executor/query/GetEdgesExecutor.h @@ -19,7 +19,7 @@ class GetEdgesExecutor final : public GetPropExecutor { folly::Future execute() override; private: - DataSet buildRequestDataSet(const GetEdges *ge); + StatusOr buildRequestDataSet(const GetEdges *ge); folly::Future getEdges(); }; diff --git a/src/graph/executor/query/GetNeighborsExecutor.cpp b/src/graph/executor/query/GetNeighborsExecutor.cpp index 9c90e66c578..3737f741088 100644 --- a/src/graph/executor/query/GetNeighborsExecutor.cpp +++ b/src/graph/executor/query/GetNeighborsExecutor.cpp @@ -13,7 +13,7 @@ using nebula::storage::cpp2::GetNeighborsResponse; namespace nebula { namespace graph { -DataSet GetNeighborsExecutor::buildRequestDataSet() { +StatusOr GetNeighborsExecutor::buildRequestDataSet() { SCOPED_TIMER(&execTime_); auto inputVar = gn_->inputVar(); auto iter = ectx_->getResult(inputVar).iter(); @@ -21,7 +21,9 @@ DataSet GetNeighborsExecutor::buildRequestDataSet() { } folly::Future GetNeighborsExecutor::execute() { - DataSet reqDs = buildRequestDataSet(); + auto res = buildRequestDataSet(); + NG_RETURN_IF_ERROR(res); + auto reqDs = std::move(res).value(); if (reqDs.rows.empty()) { List emptyResult; return finish(ResultBuilder() diff --git a/src/graph/executor/query/GetNeighborsExecutor.h b/src/graph/executor/query/GetNeighborsExecutor.h index e7d79786e59..7ba20c19cf5 100644 --- a/src/graph/executor/query/GetNeighborsExecutor.h +++ b/src/graph/executor/query/GetNeighborsExecutor.h @@ -21,7 +21,7 @@ class GetNeighborsExecutor final : public StorageAccessExecutor { folly::Future execute() override; - DataSet buildRequestDataSet(); + StatusOr buildRequestDataSet(); private: using RpcResponse = storage::StorageRpcResponse; diff --git a/src/graph/executor/query/GetVerticesExecutor.cpp b/src/graph/executor/query/GetVerticesExecutor.cpp index 6a71c737e67..20014b74090 100644 --- a/src/graph/executor/query/GetVerticesExecutor.cpp +++ b/src/graph/executor/query/GetVerticesExecutor.cpp @@ -21,7 +21,9 @@ folly::Future GetVerticesExecutor::getVertices() { auto *gv = asNode(node()); StorageClient *storageClient = qctx()->getStorageClient(); - DataSet vertices = buildRequestDataSet(gv); + auto res = buildRequestDataSet(gv); + NG_RETURN_IF_ERROR(res); + auto vertices = std::move(res).value(); if (vertices.rows.empty()) { // TODO: add test for empty input. return finish( @@ -55,7 +57,7 @@ folly::Future GetVerticesExecutor::getVertices() { }); } -DataSet GetVerticesExecutor::buildRequestDataSet(const GetVertices *gv) { +StatusOr GetVerticesExecutor::buildRequestDataSet(const GetVertices *gv) { if (gv == nullptr) { return nebula::DataSet({kVid}); } diff --git a/src/graph/executor/query/GetVerticesExecutor.h b/src/graph/executor/query/GetVerticesExecutor.h index 24d2c249039..d52ff39adb8 100644 --- a/src/graph/executor/query/GetVerticesExecutor.h +++ b/src/graph/executor/query/GetVerticesExecutor.h @@ -19,7 +19,7 @@ class GetVerticesExecutor final : public GetPropExecutor { folly::Future execute() override; private: - DataSet buildRequestDataSet(const GetVertices *gv); + StatusOr buildRequestDataSet(const GetVertices *gv); folly::Future getVertices(); }; diff --git a/src/graph/executor/test/GetNeighborsTest.cpp b/src/graph/executor/test/GetNeighborsTest.cpp index 9edf1352e05..55c3ba5e48c 100644 --- a/src/graph/executor/test/GetNeighborsTest.cpp +++ b/src/graph/executor/test/GetNeighborsTest.cpp @@ -69,7 +69,8 @@ TEST_F(GetNeighborsTest, BuildRequestDataSet) { gn->setInputVar("input_gn"); auto gnExe = std::make_unique(gn, qctx_.get()); - auto reqDs = gnExe->buildRequestDataSet(); + auto res = gnExe->buildRequestDataSet(); + auto reqDs = std::move(res).value(); DataSet expected; expected.colNames = {kVid}; diff --git a/src/graph/validator/FetchEdgesValidator.cpp b/src/graph/validator/FetchEdgesValidator.cpp index 5c155e4515f..57b9477e82d 100644 --- a/src/graph/validator/FetchEdgesValidator.cpp +++ b/src/graph/validator/FetchEdgesValidator.cpp @@ -50,12 +50,6 @@ StatusOr FetchEdgesValidator::validateEdgeRef(const Expression *exp } auto exprType = deduceExprType(expr); NG_RETURN_IF_ERROR(exprType); - if (exprType.value() != type) { - std::stringstream ss; - ss << "`" << expr->toString() << "' should be type of " << type << ", but was " - << exprType.value(); - return Status::SemanticError(ss.str()); - } if (kind == Expression::Kind::kInputProperty) { return inputVarName_; } diff --git a/src/parser/TraverseSentences.cpp b/src/parser/TraverseSentences.cpp index 828c9e0bed3..5e21a6b3c44 100644 --- a/src/parser/TraverseSentences.cpp +++ b/src/parser/TraverseSentences.cpp @@ -40,7 +40,15 @@ std::string GoSentence::toString() const { } std::string UnwindSentence::toString() const { - return ""; + std::string buf; + buf.reserve(256); + + buf += "UNWIND "; + buf += expr_->toString(); + buf += " AS "; + buf += alias_; + + return buf; } LookupSentence::LookupSentence(std::string *from, WhereClause *where, YieldClause *yield) From 71f6901684826efe9f81c5dceeb441ecd45c9e6b Mon Sep 17 00:00:00 2001 From: jimingquan Date: Sat, 23 Jul 2022 22:20:24 +0800 Subject: [PATCH 4/9] fix parser error --- src/parser/parser.yy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parser/parser.yy b/src/parser/parser.yy index e9f5be4b63a..5ed69f0fc79 100644 --- a/src/parser/parser.yy +++ b/src/parser/parser.yy @@ -2946,7 +2946,7 @@ traverse_sentence piped_sentence : traverse_sentence { $$ = $1; } - | unwind_sentence { $$ = $1; } + | piped_sentence PIPE unwind_sentence { $$ = new PipedSentence($1, $3); } | piped_sentence PIPE traverse_sentence { $$ = new PipedSentence($1, $3); } | piped_sentence PIPE limit_sentence { $$ = new PipedSentence($1, $3); } ; From 796131cbc1073c78cde4acfeb7b3920a68ddfa2e Mon Sep 17 00:00:00 2001 From: jimingquan Date: Sat, 23 Jul 2022 22:51:03 +0800 Subject: [PATCH 5/9] fix error --- src/graph/executor/query/UnwindExecutor.cpp | 2 +- src/graph/planner/plan/Query.h | 9 +++++++++ src/graph/validator/UnwindValidator.cpp | 1 + 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/graph/executor/query/UnwindExecutor.cpp b/src/graph/executor/query/UnwindExecutor.cpp index de1d774e0a7..1e42993fa3b 100644 --- a/src/graph/executor/query/UnwindExecutor.cpp +++ b/src/graph/executor/query/UnwindExecutor.cpp @@ -26,7 +26,7 @@ folly::Future UnwindExecutor::execute() { std::vector vals = extractList(list); for (auto &v : vals) { Row row; - if (!emptyInput) { + if (!unwind->fromPipe() && !emptyInput) { row = *(iter->row()); } row.values.emplace_back(std::move(v)); diff --git a/src/graph/planner/plan/Query.h b/src/graph/planner/plan/Query.h index 8144e27ef5d..0f74d65dc68 100644 --- a/src/graph/planner/plan/Query.h +++ b/src/graph/planner/plan/Query.h @@ -841,6 +841,14 @@ class Unwind final : public SingleInputNode { return alias_; } + bool fromPipe() const { + return fromPipe_; + } + + void setFromPipe() { + fromPipe_ = true; + } + PlanNode* clone() const override; std::unique_ptr explain() const override; @@ -854,6 +862,7 @@ class Unwind final : public SingleInputNode { private: Expression* unwindExpr_{nullptr}; std::string alias_; + bool fromPipe_{false}; }; // Sort the given record set. diff --git a/src/graph/validator/UnwindValidator.cpp b/src/graph/validator/UnwindValidator.cpp index cd0d44d219c..7075b5bcebf 100644 --- a/src/graph/validator/UnwindValidator.cpp +++ b/src/graph/validator/UnwindValidator.cpp @@ -34,6 +34,7 @@ Status UnwindValidator::validateImpl() { Status UnwindValidator::toPlan() { auto *unwind = Unwind::make(qctx_, nullptr, unwindExpr_, alias_); unwind->setColNames({alias_}); + unwind->setFromPipe(); root_ = tail_ = unwind; return Status::OK(); } From 786b2e96c830b22ab20d289cb1906dbaa3adb0d5 Mon Sep 17 00:00:00 2001 From: jimingquan Date: Sat, 23 Jul 2022 23:07:33 +0800 Subject: [PATCH 6/9] add test case --- tests/tck/features/match/Unwind.feature | 42 ++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/tests/tck/features/match/Unwind.feature b/tests/tck/features/match/Unwind.feature index 8d9b865ce46..67b87392dab 100644 --- a/tests/tck/features/match/Unwind.feature +++ b/tests/tck/features/match/Unwind.feature @@ -116,6 +116,46 @@ Feature: Unwind clause | min | max | | false | true | + Scenario: unwind pipe ngql + When executing query: + """ + YIELD ['Tim Duncan', 'Tony Parker'] AS a + | UNWIND $-.a AS b + | GO FROM $-.b OVER like YIELD edge AS e + """ + Then the result should be, in any order: + | e | + | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | + | [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] | + | [:like "Tony Parker"->"LaMarcus Aldridge" @0 {likeness: 90}] | + | [:like "Tony Parker"->"Manu Ginobili" @0 {likeness: 95}] | + | [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}] | + When executing query: + """ + YIELD {a:1, b:['Tim Duncan', 'Tony Parker'], c:'Tim Duncan'} AS a + | YIELD $-.a.b AS b + | UNWIND $-.b AS c + | GO FROM $-.c OVER like YIELD edge AS e + """ + Then the result should be, in any order: + | e | + | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | + | [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] | + | [:like "Tony Parker"->"LaMarcus Aldridge" @0 {likeness: 90}] | + | [:like "Tony Parker"->"Manu Ginobili" @0 {likeness: 95}] | + | [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}] | + When executing query: + """ + YIELD {a:1, b:['Tim Duncan', 'Tony Parker'], c:'Tim Duncan'} AS a + | YIELD $-.a.c AS b + | UNWIND $-.b AS c + | GO FROM $-.c OVER like YIELD edge AS e + """ + Then the result should be, in any order: + | e | + | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | + | [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] | + Scenario: unwind match with When executing query: """ @@ -146,7 +186,7 @@ Feature: Unwind clause WITH DISTINCT vid RETURN collect(vid) as vids """ - Then a SyntaxError should be raised at runtime: syntax error near `UNWIND' + Then a SyntaxError should be raised at runtime: syntax error near `WITH' When executing query: """ MATCH (a:player {name:"Tim Duncan"}) - [e:like] -> (b) From 82118e817f72d56578ac2427424438068fb0bddc Mon Sep 17 00:00:00 2001 From: jimingquan Date: Sun, 24 Jul 2022 00:25:24 +0800 Subject: [PATCH 7/9] fix test error --- .../optimizer/rule/RemoveNoopProjectRule.cpp | 1 - src/graph/validator/UnwindValidator.cpp | 11 ++- src/parser/parser.yy | 2 +- .../features/match/PipeAndVariable.feature | 98 +++++++++++++++++-- tests/tck/features/match/Unwind.feature | 40 -------- 5 files changed, 100 insertions(+), 52 deletions(-) diff --git a/src/graph/optimizer/rule/RemoveNoopProjectRule.cpp b/src/graph/optimizer/rule/RemoveNoopProjectRule.cpp index 91baafe1502..e3b79b30903 100644 --- a/src/graph/optimizer/rule/RemoveNoopProjectRule.cpp +++ b/src/graph/optimizer/rule/RemoveNoopProjectRule.cpp @@ -41,7 +41,6 @@ namespace opt { // PlanNode::Kind::kIntersect, intersect has no value in result // PlanNode::Kind::kMinus, minus has no value in result PlanNode::Kind::kProject, - PlanNode::Kind::kUnwind, PlanNode::Kind::kSort, PlanNode::Kind::kTopN, // PlanNode::Kind::kLimit, limit has no value in result diff --git a/src/graph/validator/UnwindValidator.cpp b/src/graph/validator/UnwindValidator.cpp index 7075b5bcebf..c653b49918e 100644 --- a/src/graph/validator/UnwindValidator.cpp +++ b/src/graph/validator/UnwindValidator.cpp @@ -20,8 +20,15 @@ Status UnwindValidator::validateImpl() { } alias_ = unwindSentence->alias(); unwindExpr_ = unwindSentence->expr()->clone(); - if (ExpressionUtils::hasAny(unwindExpr_, {Expression::Kind::kAggregate})) { - return Status::SemanticError("Can't use aggregating expressions in unwind clause, `%s'", + + if (ExpressionUtils::findAny(unwindExpr_, + {Expression::Kind::kSrcProperty, + Expression::Kind::kDstProperty, + Expression::Kind::kVarProperty, + Expression::Kind::kLabel, + Expression::Kind::kAggregate, + Expression::Kind::kEdgeProperty})) { + return Status::SemanticError("Not support `%s' in UNWIND sentence.", unwindExpr_->toString().c_str()); } diff --git a/src/parser/parser.yy b/src/parser/parser.yy index 5ed69f0fc79..91f5c8e269b 100644 --- a/src/parser/parser.yy +++ b/src/parser/parser.yy @@ -2942,11 +2942,11 @@ traverse_sentence | show_queries_sentence { $$ = $1; } | kill_query_sentence { $$ = $1; } | describe_user_sentence { $$ = $1; } + | unwind_sentence { $$ = $1; } ; piped_sentence : traverse_sentence { $$ = $1; } - | piped_sentence PIPE unwind_sentence { $$ = new PipedSentence($1, $3); } | piped_sentence PIPE traverse_sentence { $$ = new PipedSentence($1, $3); } | piped_sentence PIPE limit_sentence { $$ = new PipedSentence($1, $3); } ; diff --git a/tests/tck/features/match/PipeAndVariable.feature b/tests/tck/features/match/PipeAndVariable.feature index 8a0f8cfb1b2..badf68f33bd 100644 --- a/tests/tck/features/match/PipeAndVariable.feature +++ b/tests/tck/features/match/PipeAndVariable.feature @@ -79,26 +79,108 @@ Feature: Pipe or use variable to store the lookup results | true | 42 | Scenario: mixed usage of cypher and ngql + When executing query: + """ + YIELD ['Tim Duncan', 'Tony Parker'] AS a + | UNWIND $-.a AS b + | GO FROM $-.b OVER like YIELD edge AS e + """ + Then the result should be, in any order: + | e | + | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | + | [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] | + | [:like "Tony Parker"->"LaMarcus Aldridge" @0 {likeness: 90}] | + | [:like "Tony Parker"->"Manu Ginobili" @0 {likeness: 95}] | + | [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}] | + When executing query: + """ + YIELD {a:1, b:['Tim Duncan', 'Tony Parker'], c:'Tim Duncan'} AS a + | YIELD $-.a.b AS b + | UNWIND $-.b AS c + | GO FROM $-.c OVER like YIELD edge AS e + """ + Then the result should be, in any order: + | e | + | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | + | [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] | + | [:like "Tony Parker"->"LaMarcus Aldridge" @0 {likeness: 90}] | + | [:like "Tony Parker"->"Manu Ginobili" @0 {likeness: 95}] | + | [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}] | + When executing query: + """ + YIELD {a:1, b:['Tim Duncan', 'Tony Parker'], c:'Tim Duncan'} AS a + | YIELD $-.a.c AS b + | UNWIND $-.b AS c + | GO FROM $-.c OVER like YIELD edge AS e + """ + Then the result should be, in any order: + | e | + | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | + | [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] | When executing query: """ LOOKUP ON player WHERE player.name == 'Tim Duncan' YIELD player.age as age, id(vertex) as vid - | UNWIND $-.vid as a RETURN a + | UNWIND $-.vid as a | YIELD $-.a AS id """ - Then a SyntaxError should be raised at runtime: syntax error near `UNWIND' + Then the result should be, in any order, with relax comparison: + | id | + | "Tim Duncan" | When executing query: """ GET SUBGRAPH 2 STEPS FROM "Tim Duncan" BOTH like YIELD edges as e - | UNWIND $-.e as a RETURN a - """ - Then a SyntaxError should be raised at runtime: syntax error near `UNWIND' + | UNWIND $-.e as a | YIELD $-.a AS a + """ + Then the result should be, in any order, with relax comparison: + | a | + | [:like "Aron Baynes"->"Tim Duncan" @0 {}] | + | [:like "Boris Diaw"->"Tim Duncan" @0 {}] | + | [:like "Danny Green"->"Tim Duncan" @0 {}] | + | [:like "Dejounte Murray"->"Tim Duncan" @0 {}] | + | [:like "LaMarcus Aldridge"->"Tim Duncan" @0 {}] | + | [:like "Manu Ginobili"->"Tim Duncan" @0 {}] | + | [:like "Marco Belinelli"->"Tim Duncan" @0 {}] | + | [:like "Shaquille O'Neal"->"Tim Duncan" @0 {}] | + | [:like "Tiago Splitter"->"Tim Duncan" @0 {}] | + | [:like "Tony Parker"->"Tim Duncan" @0 {}] | + | [:like "Tim Duncan"->"Manu Ginobili" @0 {}] | + | [:like "Tim Duncan"->"Tony Parker" @0 {}] | + | [:like "Damian Lillard"->"LaMarcus Aldridge" @0 {}] | + | [:like "Rudy Gay"->"LaMarcus Aldridge" @0 {}] | + | [:like "LaMarcus Aldridge"->"Tony Parker" @0 {}] | + | [:like "Boris Diaw"->"Tony Parker" @0 {}] | + | [:like "Dejounte Murray"->"Chris Paul" @0 {}] | + | [:like "Dejounte Murray"->"Danny Green" @0 {}] | + | [:like "Dejounte Murray"->"James Harden" @0 {}] | + | [:like "Dejounte Murray"->"Kevin Durant" @0 {}] | + | [:like "Dejounte Murray"->"Kyle Anderson" @0 {}] | + | [:like "Dejounte Murray"->"LeBron James" @0 {}] | + | [:like "Dejounte Murray"->"Manu Ginobili" @0 {}] | + | [:like "Dejounte Murray"->"Marco Belinelli" @0 {}] | + | [:like "Dejounte Murray"->"Russell Westbrook" @0 {}] | + | [:like "Dejounte Murray"->"Tony Parker" @0 {}] | + | [:like "Danny Green"->"LeBron James" @0 {}] | + | [:like "Danny Green"->"Marco Belinelli" @0 {}] | + | [:like "Marco Belinelli"->"Danny Green" @0 {}] | + | [:like "Marco Belinelli"->"Tony Parker" @0 {}] | + | [:like "Yao Ming"->"Shaquille O'Neal" @0 {}] | + | [:like "Shaquille O'Neal"->"JaVale McGee" @0 {}] | + | [:like "Tiago Splitter"->"Manu Ginobili" @0 {}] | + | [:like "Tony Parker"->"LaMarcus Aldridge" @0 {}] | + | [:like "Tony Parker"->"Manu Ginobili" @0 {}] | + | [:like "James Harden"->"Russell Westbrook" @0 {}] | + | [:like "Chris Paul"->"LeBron James" @0 {}] | + | [:like "Russell Westbrook"->"James Harden" @0 {}] | When executing query: """ - FIND SHORTEST PATH FROM "Tim Duncan" TO "Yao Ming" OVER like YIELD path as p - | UNWIND $-.p as a RETURN a + FIND SHORTEST PATH FROM "Tim Duncan" TO "Tony Parker" OVER like YIELD path as p + | YIELD nodes($-.p) AS nodes | UNWIND $-.nodes AS a | YIELD $-.a AS a """ - Then a SyntaxError should be raised at runtime: syntax error near `UNWIND' + Then the result should be, in any order, with relax comparison: + | a | + | ("Tim Duncan") | + | ("Tony Parker") | When executing query: """ GO 2 STEPS FROM "Tim Duncan" OVER * YIELD dst(edge) as id diff --git a/tests/tck/features/match/Unwind.feature b/tests/tck/features/match/Unwind.feature index 67b87392dab..40e4893e853 100644 --- a/tests/tck/features/match/Unwind.feature +++ b/tests/tck/features/match/Unwind.feature @@ -116,46 +116,6 @@ Feature: Unwind clause | min | max | | false | true | - Scenario: unwind pipe ngql - When executing query: - """ - YIELD ['Tim Duncan', 'Tony Parker'] AS a - | UNWIND $-.a AS b - | GO FROM $-.b OVER like YIELD edge AS e - """ - Then the result should be, in any order: - | e | - | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | - | [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] | - | [:like "Tony Parker"->"LaMarcus Aldridge" @0 {likeness: 90}] | - | [:like "Tony Parker"->"Manu Ginobili" @0 {likeness: 95}] | - | [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}] | - When executing query: - """ - YIELD {a:1, b:['Tim Duncan', 'Tony Parker'], c:'Tim Duncan'} AS a - | YIELD $-.a.b AS b - | UNWIND $-.b AS c - | GO FROM $-.c OVER like YIELD edge AS e - """ - Then the result should be, in any order: - | e | - | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | - | [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] | - | [:like "Tony Parker"->"LaMarcus Aldridge" @0 {likeness: 90}] | - | [:like "Tony Parker"->"Manu Ginobili" @0 {likeness: 95}] | - | [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}] | - When executing query: - """ - YIELD {a:1, b:['Tim Duncan', 'Tony Parker'], c:'Tim Duncan'} AS a - | YIELD $-.a.c AS b - | UNWIND $-.b AS c - | GO FROM $-.c OVER like YIELD edge AS e - """ - Then the result should be, in any order: - | e | - | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | - | [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] | - Scenario: unwind match with When executing query: """ From e4da9c387ccbdc5c2db0af306c45ec79b9c46ee9 Mon Sep 17 00:00:00 2001 From: jimingquan Date: Mon, 25 Jul 2022 11:58:18 +0800 Subject: [PATCH 8/9] fix removeproject error --- src/graph/optimizer/rule/RemoveNoopProjectRule.cpp | 1 + src/graph/planner/plan/Query.cpp | 1 + src/graph/planner/plan/Query.h | 4 ++-- src/graph/validator/UnwindValidator.cpp | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/graph/optimizer/rule/RemoveNoopProjectRule.cpp b/src/graph/optimizer/rule/RemoveNoopProjectRule.cpp index e3b79b30903..91baafe1502 100644 --- a/src/graph/optimizer/rule/RemoveNoopProjectRule.cpp +++ b/src/graph/optimizer/rule/RemoveNoopProjectRule.cpp @@ -41,6 +41,7 @@ namespace opt { // PlanNode::Kind::kIntersect, intersect has no value in result // PlanNode::Kind::kMinus, minus has no value in result PlanNode::Kind::kProject, + PlanNode::Kind::kUnwind, PlanNode::Kind::kSort, PlanNode::Kind::kTopN, // PlanNode::Kind::kLimit, limit has no value in result diff --git a/src/graph/planner/plan/Query.cpp b/src/graph/planner/plan/Query.cpp index aa1c8bed387..a03841b8d4d 100644 --- a/src/graph/planner/plan/Query.cpp +++ b/src/graph/planner/plan/Query.cpp @@ -369,6 +369,7 @@ std::unique_ptr Unwind::explain() const { PlanNode* Unwind::clone() const { auto* newUnwind = Unwind::make(qctx_, nullptr); + newUnwind->setFromPipe(fromPipe_); newUnwind->cloneMembers(*this); return newUnwind; } diff --git a/src/graph/planner/plan/Query.h b/src/graph/planner/plan/Query.h index 0f74d65dc68..c10cf992f9b 100644 --- a/src/graph/planner/plan/Query.h +++ b/src/graph/planner/plan/Query.h @@ -845,8 +845,8 @@ class Unwind final : public SingleInputNode { return fromPipe_; } - void setFromPipe() { - fromPipe_ = true; + void setFromPipe(bool fromPipe) { + fromPipe_ = fromPipe; } PlanNode* clone() const override; diff --git a/src/graph/validator/UnwindValidator.cpp b/src/graph/validator/UnwindValidator.cpp index c653b49918e..6cee4c8886d 100644 --- a/src/graph/validator/UnwindValidator.cpp +++ b/src/graph/validator/UnwindValidator.cpp @@ -41,7 +41,7 @@ Status UnwindValidator::validateImpl() { Status UnwindValidator::toPlan() { auto *unwind = Unwind::make(qctx_, nullptr, unwindExpr_, alias_); unwind->setColNames({alias_}); - unwind->setFromPipe(); + unwind->setFromPipe(true); root_ = tail_ = unwind; return Status::OK(); } From cd08097f2fb1610285bf5751f91739b8948bbfdc Mon Sep 17 00:00:00 2001 From: jimingquan Date: Mon, 25 Jul 2022 14:02:30 +0800 Subject: [PATCH 9/9] address comment --- tests/tck/features/match/PipeAndVariable.feature | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tck/features/match/PipeAndVariable.feature b/tests/tck/features/match/PipeAndVariable.feature index badf68f33bd..68abc31edc2 100644 --- a/tests/tck/features/match/PipeAndVariable.feature +++ b/tests/tck/features/match/PipeAndVariable.feature @@ -122,10 +122,10 @@ Feature: Pipe or use variable to store the lookup results LOOKUP ON player WHERE player.name == 'Tim Duncan' YIELD player.age as age, id(vertex) as vid - | UNWIND $-.vid as a | YIELD $-.a AS id + | UNWIND $-.vid as a | YIELD $-.a AS a """ Then the result should be, in any order, with relax comparison: - | id | + | a | | "Tim Duncan" | When executing query: """