From 63c2e492e001bb2d50e6b90e63e67d0c75970604 Mon Sep 17 00:00:00 2001 From: "mingquan.ji" Date: Mon, 25 Apr 2022 17:17:46 +0800 Subject: [PATCH] fix error --- src/graph/context/ast/CypherAstContext.h | 8 +-- .../executor/algo/ShortestPathExecutor.cpp | 12 ++-- .../executor/algo/ShortestPathExecutor.h | 4 +- .../planner/match/MatchClausePlanner.cpp | 6 +- .../planner/match/ShortestPathPlanner.cpp | 61 +++++++++++++------ src/graph/planner/plan/Algo.cpp | 3 +- src/graph/planner/plan/Algo.h | 8 ++- src/graph/validator/MatchValidator.cpp | 6 +- src/graph/validator/MatchValidator.h | 3 - src/parser/MatchPath.h | 2 +- 10 files changed, 72 insertions(+), 41 deletions(-) diff --git a/src/graph/context/ast/CypherAstContext.h b/src/graph/context/ast/CypherAstContext.h index bd7bc7f81d0..66ec7a2ce5c 100644 --- a/src/graph/context/ast/CypherAstContext.h +++ b/src/graph/context/ast/CypherAstContext.h @@ -83,7 +83,7 @@ struct Path final { // "(v)-[:like]->()" in (v)-[:like]->() std::string collectVariable; - enum PathType { kDefault, kAllShortest, kSingleShortest }; + enum PathType : int8_t { kDefault, kAllShortest, kSingleShortest }; PathType pathType{PathType::kDefault}; }; @@ -215,8 +215,8 @@ struct NodeContext final : PatternContext { QueryContext* qctx; Expression* bindFilter; GraphSpaceID spaceId; - NodeInfo* info{nullptr}; - std::unordered_set* nodeAliasesAvailable; + NodeInfo* info; + std::unordered_set* nodeAliasesAvailable{nullptr}; // Output fields ScanInfo scanInfo; @@ -232,7 +232,7 @@ struct EdgeContext final : PatternContext { QueryContext* qctx; Expression* bindFilter; GraphSpaceID spaceId; - EdgeInfo* info{nullptr}; + EdgeInfo* info; // Output fields ScanInfo scanInfo; diff --git a/src/graph/executor/algo/ShortestPathExecutor.cpp b/src/graph/executor/algo/ShortestPathExecutor.cpp index 9126e5b2a0c..ba343a2a975 100644 --- a/src/graph/executor/algo/ShortestPathExecutor.cpp +++ b/src/graph/executor/algo/ShortestPathExecutor.cpp @@ -11,7 +11,8 @@ namespace graph { folly::Future ShortestPathExecutor::execute() { SCOPED_TIMER(&execTime_); single_ = pathNode_->singleShortest(); - range_ = {pathNode_->stepRange()->min(), pathNode_->stepRange()->max()}; + maxStep_ = pathNode_->stepRange()->max(); + auto& colNames = pathNode_->colNames(); auto rowSize = buildRequestDataSet(); std::vector> futures; @@ -51,9 +52,10 @@ size_t ShortestPathExecutor::buildRequestDataSet() { auto start = iter->getColumn(0); auto end = iter->getColumn(1); if (!SchemaUtil::isValidVid(start, vidType) || !SchemaUtil::isValidVid(end, vidType)) { - LOG(ERROR) << "Mismatched vid type. start type : " << start.type() + LOG(ERROR) << "Mismatched shortestPath vid type. start type : " << start.type() << ", end type: " << end.type() << ", space vid type: " << SchemaUtil::typeToString(vidType); + rowSize--; continue; } if (start == end) { @@ -139,7 +141,9 @@ folly::Future ShortestPathExecutor::handleResponse(size_t rowNum, size_t return Status::OK(); } stepNum++; - if (stepNum * 2 >= range_.second) { + auto& leftVids = leftVids_[rowNum].rows; + auto& rightVids = rightVids_[rowNum].rows; + if (stepNum * 2 >= maxStep_ || leftVids.empty() || rightVids.empty()) { return Status::OK(); } return shortestPath(rowNum, stepNum); @@ -158,7 +162,7 @@ bool ShortestPathExecutor::conjunctPath(size_t rowNum, size_t stepNum) { buildOddPath(rowNum, meetVids); return true; } - if (stepNum * 2 >= range_.second) { + if (stepNum * 2 >= maxStep_) { return false; } diff --git a/src/graph/executor/algo/ShortestPathExecutor.h b/src/graph/executor/algo/ShortestPathExecutor.h index 5f1f3762f52..c3f2279e013 100644 --- a/src/graph/executor/algo/ShortestPathExecutor.h +++ b/src/graph/executor/algo/ShortestPathExecutor.h @@ -5,7 +5,7 @@ #define GRAPH_EXECUTOR_QUERY_SHORTESTPATHEXECUTOR_H_ #include "graph/executor/StorageAccessExecutor.h" -#include "graph/planner/plan/Query.h" +#include "graph/planner/plan/Algo.h" using nebula::storage::StorageRpcResponse; using nebula::storage::cpp2::GetNeighborsResponse; @@ -52,7 +52,7 @@ class ShortestPathExecutor final : public StorageAccessExecutor { private: const ShortestPath* pathNode_{nullptr}; - std::pair range_; + size_t maxStep_; bool single_{true}; std::vector resultDs_; diff --git a/src/graph/planner/match/MatchClausePlanner.cpp b/src/graph/planner/match/MatchClausePlanner.cpp index d31ff810bfd..d9ffd9c4207 100644 --- a/src/graph/planner/match/MatchClausePlanner.cpp +++ b/src/graph/planner/match/MatchClausePlanner.cpp @@ -26,9 +26,10 @@ StatusOr MatchClausePlanner::transform(CypherClauseContextBase* clauseC // TODO: Maybe it is better to rebuild the graph and find all connected components. auto& pathInfos = matchClauseCtx->paths; for (auto iter = pathInfos.begin(); iter < pathInfos.end(); ++iter) { + auto& nodeInfos = iter->nodeInfos; auto bindFilter = matchClauseCtx->where ? matchClauseCtx->where->filter : nullptr; SubPlan pathPlan; - if (iter->type != PATH::Type::kDefault) { + if (iter->pathType == Path::PathType::kDefault) { MatchPathPlanner matchPathPlanner; auto result = matchPathPlanner.transform(matchClauseCtx->qctx, matchClauseCtx->space.id, @@ -36,7 +37,7 @@ StatusOr MatchClausePlanner::transform(CypherClauseContextBase* clauseC matchClauseCtx->aliasesAvailable, nodeAliasesSeen, *iter); - NG_RETURN_IF_ERROF(result); + NG_RETURN_IF_ERROR(result); pathPlan = std::move(result).value(); } else { ShortestPathPlanner shortestPathPlanner; @@ -49,7 +50,6 @@ StatusOr MatchClausePlanner::transform(CypherClauseContextBase* clauseC NG_RETURN_IF_ERROR(result); pathPlan = std::move(result).value(); } - auto& nodeInfos = iter->nodeInfos; NG_RETURN_IF_ERROR( connectPathPlan(nodeInfos, matchClauseCtx, pathPlan, nodeAliasesSeen, matchClausePlan)); } diff --git a/src/graph/planner/match/ShortestPathPlanner.cpp b/src/graph/planner/match/ShortestPathPlanner.cpp index 5ed26a24ba2..205be0159f1 100644 --- a/src/graph/planner/match/ShortestPathPlanner.cpp +++ b/src/graph/planner/match/ShortestPathPlanner.cpp @@ -4,6 +4,16 @@ #include "graph/planner/match/ShortestPathPlanner.h" +#include "graph/context/ast/CypherAstContext.h" +#include "graph/planner/match/MatchSolver.h" +#include "graph/planner/match/SegmentsConnector.h" +#include "graph/planner/match/StartVidFinder.h" +#include "graph/planner/plan/Algo.h" +#include "graph/planner/plan/Logic.h" +#include "graph/planner/plan/Query.h" +#include "graph/util/ExpressionUtils.h" +#include "graph/util/SchemaUtil.h" + namespace nebula { namespace graph { // Match (start:tagName{propName:xxx}), (end:tagName{propName:yyy}) @@ -74,11 +84,11 @@ static std::unique_ptr> genEdgeProps(const return edgeProps; } -static YieldColumn* buildVertexColumn(ObjectPool* pool, const std::string& alias) const { +static YieldColumn* buildVertexColumn(ObjectPool* pool, const std::string& alias) { return new YieldColumn(InputPropertyExpression::make(pool, alias), alias); } -static YieldColumn* buildEdgeColumn(ObjectPool* pool, EdgeInfo& edge) const { +static YieldColumn* buildEdgeColumn(ObjectPool* pool, EdgeInfo& edge) { Expression* expr = nullptr; if (edge.range == nullptr) { expr = SubscriptExpression::make( @@ -93,7 +103,7 @@ static YieldColumn* buildEdgeColumn(ObjectPool* pool, EdgeInfo& edge) const { return new YieldColumn(expr, edge.alias); } -static YieldColumn* buildPathColumn(Expression* pathBuild, const std::string& alias) const { +static YieldColumn* buildPathColumn(Expression* pathBuild, const std::string& alias) { return new YieldColumn(pathBuild, alias); } @@ -103,14 +113,14 @@ static void buildProjectColumns(QueryContext* qctx, Path& path, SubPlan& plan) { auto& nodeInfos = path.nodeInfos; auto& edgeInfos = path.edgeInfos; - auto addNode = [this, columns, &colNames, qctx](auto& nodeInfo) { + auto addNode = [columns, &colNames, qctx](auto& nodeInfo) { if (!nodeInfo.alias.empty() && !nodeInfo.anonymous) { columns->addColumn(buildVertexColumn(qctx->objPool(), nodeInfo.alias)); colNames.emplace_back(nodeInfo.alias); } }; - auto addEdge = [this, columns, &colNames, qctx](auto& edgeInfo) { + auto addEdge = [columns, &colNames, qctx](auto& edgeInfo) { if (!edgeInfo.alias.empty() && !edgeInfo.anonymous) { columns->addColumn(buildEdgeColumn(qctx->objPool(), edgeInfo)); colNames.emplace_back(edgeInfo.alias); @@ -145,26 +155,42 @@ StatusOr ShortestPathPlanner::transform( const std::unordered_map& aliasesAvailable, std::unordered_set nodeAliasesSeen, Path& path) { - UNUSED(aliasesAvailable); - UNUSED(nodeAliasesSeen); + std::unordered_set allNodeAliasesAvailable; + allNodeAliasesAvailable.merge(nodeAliasesSeen); + std::for_each( + aliasesAvailable.begin(), aliasesAvailable.end(), [&allNodeAliasesAvailable](auto& kv) { + if (kv.second == AliasType::kNode) { + allNodeAliasesAvailable.emplace(kv.first); + } + }); SubPlan subplan; - bool singleShortest = path.pathType == MatchPath::PathType::kSingleShortest; + bool singleShortest = path.pathType == Path::PathType::kSingleShortest; auto& nodeInfos = path.nodeInfos; - auto& edge = path.edgeInfos.front(); auto& startVidFinders = StartVidFinder::finders(); std::vector plans; - for (size_t i = 0; i < nodeInfos.size(); ++i) { + for (auto& nodeInfo : nodeInfos) { bool foundIndex = false; for (auto& finder : startVidFinders) { - auto nodeCtx = NodeContext(qctx, bindFilter, spaceId, &nodeInfos[i]); + auto nodeCtx = NodeContext(qctx, bindFilter, spaceId, &nodeInfo); + nodeCtx.nodeAliasesAvailable = &allNodeAliasesAvailable; auto nodeFinder = finder(); if (nodeFinder->match(&nodeCtx)) { - auto plan = nodeFinder->transform(&nodeCtx); - NG_RETURN_IF_ERROR(plan); - plans.emplace_back(std::move(plan).value()); + auto status = nodeFinder->transform(&nodeCtx); + NG_RETURN_IF_ERROR(status); + auto plan = status.value(); + auto start = StartNode::make(qctx); + plan.tail->setDep(0, start); + plan.tail = start; + + auto initExpr = nodeCtx.initialExpr->clone(); + auto columns = qctx->objPool()->makeAndAdd(); + columns->addColumn(new YieldColumn(initExpr, nodeInfo.alias)); + plan.root = Project::make(qctx, plan.root, columns); + + plans.emplace_back(std::move(plan)); foundIndex = true; break; } @@ -173,24 +199,25 @@ StatusOr ShortestPathPlanner::transform( return Status::SemanticError("Can't find index from path pattern"); } } - auto& leftPlan = plans.front(); auto& rightPlan = plans.back(); + auto cp = BiCartesianProduct::make(qctx, leftPlan.root, rightPlan.root); + auto& edge = path.edgeInfos.front(); auto shortestPath = ShortestPath::make(qctx, cp, spaceId, singleShortest); auto vertexProp = genVertexProps(nodeInfos.front(), qctx, spaceId); NG_RETURN_IF_ERROR(vertexProp); shortestPath->setVertexProps(std::move(vertexProp).value()); shortestPath->setEdgeProps(genEdgeProps(edge, false, qctx, spaceId)); - shortestPath->setReverseEdgeProps(getEdgeProps(edge, true, qctx, spaceId)); + shortestPath->setReverseEdgeProps(genEdgeProps(edge, true, qctx, spaceId)); shortestPath->setEdgeDirection(edge.direction); shortestPath->setStepRange(edge.range); subplan.root = shortestPath; subplan.tail = leftPlan.tail; - bulildProjectColumns(qctx, path, subplan); + buildProjectColumns(qctx, path, subplan); return subplan; } diff --git a/src/graph/planner/plan/Algo.cpp b/src/graph/planner/plan/Algo.cpp index c602ecc0b55..aa9631a7083 100644 --- a/src/graph/planner/plan/Algo.cpp +++ b/src/graph/planner/plan/Algo.cpp @@ -47,14 +47,13 @@ std::unique_ptr ShortestPath::explain() const { } PlanNode* ShortestPath::clone() const { - auto* path = ShortestPath::make(qctx_, nullptr, space_, singleShorest_); + auto* path = ShortestPath::make(qctx_, nullptr, space_, singleShortest_); path->cloneMembers(*this); return path; } void ShortestPath::cloneMembers(const ShortestPath& path) { SingleInputNode::cloneMembers(path); - setSingle(path.singleShortest_); setStepRange(path.range_); setEdgeDirection(path.edgeDirection_); if (path.vertexProps_) { diff --git a/src/graph/planner/plan/Algo.h b/src/graph/planner/plan/Algo.h index f4d8705c958..6b00d6f3e0e 100644 --- a/src/graph/planner/plan/Algo.h +++ b/src/graph/planner/plan/Algo.h @@ -145,6 +145,10 @@ class ProduceAllPaths final : public BinaryInputNode { std::string rightVidVar_; }; +using VertexProp = nebula::storage::cpp2::VertexProp; +using EdgeProp = nebula::storage::cpp2::EdgeProp; +using Direction = nebula::storage::cpp2::EdgeDirection; + class ShortestPath final : public SingleInputNode { public: static ShortestPath* make(QueryContext* qctx, @@ -182,7 +186,7 @@ class ShortestPath final : public SingleInputNode { return space_; } - bool singleShorest() const { + bool singleShortest() const { return singleShortest_; } @@ -210,7 +214,7 @@ class ShortestPath final : public SingleInputNode { ShortestPath(QueryContext* qctx, PlanNode* node, GraphSpaceID space, bool singleShortest) : SingleInputNode(qctx, Kind::kShortestPath, node), space_(space), - singleShorest_(singleShorest) {} + singleShortest_(singleShortest) {} void cloneMembers(const ShortestPath&); diff --git a/src/graph/validator/MatchValidator.cpp b/src/graph/validator/MatchValidator.cpp index 01fba80602d..876cd1a3916 100644 --- a/src/graph/validator/MatchValidator.cpp +++ b/src/graph/validator/MatchValidator.cpp @@ -156,11 +156,12 @@ Status MatchValidator::buildPathExpr(const MatchPath *path, return Status::SemanticError( "`shortestPath(...)' only support pattern like (start)-[edge*..hop]-(end)"); } - auto &edge = edgeInfos.front(); - if (edge.range->min() != 0 || edge.range->min() != 1) { + auto min = edgeInfos.front().range->min(); + if (min != 0 && min != 1) { return Status::SemanticError( "`shortestPath(...)' does not support a minimal length different from 0 or 1"); } + pathInfo.pathType = static_cast(pathType); } auto *pool = qctx_->objPool(); @@ -173,7 +174,6 @@ Status MatchValidator::buildPathExpr(const MatchPath *path, pathInfo.pathBuild = std::move(pathBuild); pathInfo.anonymous = false; pathInfo.alias = *pathAlias; - pathInfo.pathType = pathType; return Status::OK(); } diff --git a/src/graph/validator/MatchValidator.h b/src/graph/validator/MatchValidator.h index 63235fe5bb6..b30fa8784d5 100644 --- a/src/graph/validator/MatchValidator.h +++ b/src/graph/validator/MatchValidator.h @@ -31,9 +31,6 @@ class MatchValidator final : public Validator { Status validateFilter(const Expression *filter, WhereClauseContext &whereClauseCtx); - Status validateShortestPath(const MatchPath *path); - - Status validateReturn(MatchReturn *ret, const std::vector &queryParts, ReturnClauseContext &retClauseCtx); diff --git a/src/parser/MatchPath.h b/src/parser/MatchPath.h index 07253b54036..5908da54b05 100644 --- a/src/parser/MatchPath.h +++ b/src/parser/MatchPath.h @@ -301,7 +301,7 @@ class MatchPath final { return edges_[i].get(); } - enum PathType { kDefault, kAllShortest, kSingleShortest }; + enum PathType : int8_t { kDefault, kAllShortest, kSingleShortest }; PathType pathType() const { return pathType_;