From b6b1ecc8fb96601487c472e1e2a0727f2f67b978 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Mon, 17 Jan 2022 17:22:15 +0800 Subject: [PATCH 01/27] property pruner --- src/common/expression/Expression.h | 8 ++++ src/common/expression/PropertyExpression.h | 8 ++-- src/graph/optimizer/Optimizer.cpp | 14 ++++++- src/graph/optimizer/Optimizer.h | 4 ++ src/graph/planner/plan/PlanNode.cpp | 16 ++++++++ src/graph/planner/plan/PlanNode.h | 8 ++++ src/graph/planner/plan/Query.cpp | 36 ++++++++++++++++++ src/graph/planner/plan/Query.h | 4 ++ src/graph/util/ExpressionUtils.cpp | 44 ++++++++++++++++++++++ src/graph/util/ExpressionUtils.h | 7 ++++ 10 files changed, 143 insertions(+), 6 deletions(-) diff --git a/src/common/expression/Expression.h b/src/common/expression/Expression.h index 96bb68451d0..3653817bae3 100644 --- a/src/common/expression/Expression.h +++ b/src/common/expression/Expression.h @@ -223,6 +223,14 @@ class Expression { std::ostream& operator<<(std::ostream& os, Expression::Kind kind); +struct PropertyTracker { + std::unordered_map>> + vertexPropsMap; + std::unordered_map>> + edgePropsMap; + std::unordered_set colsSet; +}; + } // namespace nebula #endif // COMMON_EXPRESSION_EXPRESSION_H_ diff --git a/src/common/expression/PropertyExpression.h b/src/common/expression/PropertyExpression.h index a9baf8d47a7..3b7bd5e26cf 100644 --- a/src/common/expression/PropertyExpression.h +++ b/src/common/expression/PropertyExpression.h @@ -160,10 +160,10 @@ class LabelTagPropertyExpression final : public PropertyExpression { } private: - LabelTagPropertyExpression(ObjectPool* pool, - Expression* label = nullptr, - const std::string& tag = "", - const std::string& prop = "") + explicit LabelTagPropertyExpression(ObjectPool* pool, + Expression* label = nullptr, + const std::string& tag = "", + const std::string& prop = "") : PropertyExpression(pool, Kind::kLabelTagProperty, "", tag, prop), label_(label) {} void writeTo(Encoder& encoder) const override; diff --git a/src/graph/optimizer/Optimizer.cpp b/src/graph/optimizer/Optimizer.cpp index 320a87b3529..f8555d10434 100644 --- a/src/graph/optimizer/Optimizer.cpp +++ b/src/graph/optimizer/Optimizer.cpp @@ -30,14 +30,24 @@ StatusOr Optimizer::findBestPlan(QueryContext *qctx) { auto optCtx = std::make_unique(qctx); auto root = qctx->plan()->root(); - auto status = prepare(optCtx.get(), root); + auto spaceID = qctx->rctx()->session()->space().id; + + auto status = preprocess(root, qctx, spaceID); NG_RETURN_IF_ERROR(status); - auto rootGroup = std::move(status).value(); + + auto ret = prepare(optCtx.get(), root); + NG_RETURN_IF_ERROR(ret); + auto rootGroup = std::move(ret).value(); NG_RETURN_IF_ERROR(doExploration(optCtx.get(), rootGroup)); return rootGroup->getPlan(); } +Status Optimizer::preprocess(PlanNode *root, graph::QueryContext *qctx, GraphSpaceID spaceID) { + PropertyTracker propsUsed; + return root->pruneProperties(propsUsed, qctx, spaceID); +} + StatusOr Optimizer::prepare(OptContext *ctx, PlanNode *root) { std::unordered_map visited; return convertToGroup(ctx, root, &visited); diff --git a/src/graph/optimizer/Optimizer.h b/src/graph/optimizer/Optimizer.h index 803a06bd417..efc91db3f32 100644 --- a/src/graph/optimizer/Optimizer.h +++ b/src/graph/optimizer/Optimizer.h @@ -8,6 +8,7 @@ #include "common/base/Base.h" #include "common/base/StatusOr.h" +#include "common/thrift/ThriftTypes.h" namespace nebula { namespace graph { @@ -30,7 +31,10 @@ class Optimizer final { StatusOr findBestPlan(graph::QueryContext *qctx); private: + Status preprocess(graph::PlanNode *root, graph::QueryContext *qctx, GraphSpaceID spaceID); + StatusOr prepare(OptContext *ctx, graph::PlanNode *root); + Status doExploration(OptContext *octx, OptGroup *rootGroup); OptGroup *convertToGroup(OptContext *ctx, diff --git a/src/graph/planner/plan/PlanNode.cpp b/src/graph/planner/plan/PlanNode.cpp index 77ceecacf8d..ac1657235db 100644 --- a/src/graph/planner/plan/PlanNode.cpp +++ b/src/graph/planner/plan/PlanNode.cpp @@ -379,6 +379,22 @@ void PlanNode::updateSymbols() { } } +Status PlanNode::pruneProperties(PropertyTracker& propsUsed, + graph::QueryContext* qctx, + GraphSpaceID spaceID) { + NG_RETURN_IF_ERROR(depsPruneProperties(propsUsed, qctx, spaceID)); + return Status::OK(); +} + +Status PlanNode::depsPruneProperties(PropertyTracker& propsUsed, + graph::QueryContext* qctx, + GraphSpaceID spaceID) { + for (const auto* dep : dependencies_) { + NG_RETURN_IF_ERROR(const_cast(dep)->pruneProperties(propsUsed, qctx, spaceID)); + } + return Status::OK(); +} + std::ostream& operator<<(std::ostream& os, PlanNode::Kind kind) { os << PlanNode::toString(kind); return os; diff --git a/src/graph/planner/plan/PlanNode.h b/src/graph/planner/plan/PlanNode.h index 5d50b3ac4e4..6a61415e7a6 100644 --- a/src/graph/planner/plan/PlanNode.h +++ b/src/graph/planner/plan/PlanNode.h @@ -289,11 +289,19 @@ class PlanNode { return static_cast(this); } + virtual Status pruneProperties(PropertyTracker& propsUsed, + graph::QueryContext* qctx, + GraphSpaceID spaceID); + protected: PlanNode(QueryContext* qctx, Kind kind); virtual ~PlanNode() = default; + Status depsPruneProperties(PropertyTracker& propsUsed, + graph::QueryContext* qctx, + GraphSpaceID spaceID); + static void addDescription(std::string key, std::string value, PlanNodeDescription* desc); void readVariable(const std::string& varname); diff --git a/src/graph/planner/plan/Query.cpp b/src/graph/planner/plan/Query.cpp index 5e133e82aac..acb4ef3b174 100644 --- a/src/graph/planner/plan/Query.cpp +++ b/src/graph/planner/plan/Query.cpp @@ -760,6 +760,42 @@ std::unique_ptr AppendVertices::explain() const { return desc; } +Status AppendVertices::pruneProperties(PropertyTracker& propsUsed, + graph::QueryContext* qctx, + GraphSpaceID spaceID) { + DCHECK(!colNames().empty()); + auto& nodeAlias = colNames().back(); + auto it = propsUsed.colsSet.find(nodeAlias); + if (it != propsUsed.colsSet.end()) { // All properties are used + propsUsed.colsSet.erase(it); + NG_RETURN_IF_ERROR(depsPruneProperties(propsUsed, qctx, spaceID)); + return Status::OK(); + } + + if (vFilter_ != nullptr) { + NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(vFilter_, propsUsed, qctx, spaceID)); + } + auto* vertexProps = props(); + if (vertexProps != nullptr) { + auto it2 = propsUsed.vertexPropsMap.find(nodeAlias); + if (it2 != propsUsed.vertexPropsMap.end()) { + auto& tagPropsMap = it2->second; + auto prunedTagsProps = std::make_unique>(); + prunedTagsProps->reserve(tagPropsMap.size()); + for (auto& [tagId, tagProps] : tagPropsMap) { + VertexProp vProp; + vProp.tag_ref() = tagId; + vProp.props_ref() = std::vector(tagProps.begin(), tagProps.end()); + prunedTagsProps->emplace_back(std::move(vProp)); + } + setVertexProps(std::move(prunedTagsProps)); + } + } + + NG_RETURN_IF_ERROR(depsPruneProperties(propsUsed, qctx, spaceID)); + return Status::OK(); +} + std::unique_ptr BiJoin::explain() const { auto desc = BinaryInputNode::explain(); addDescription("hashKeys", folly::toJson(util::toJson(hashKeys_)), desc.get()); diff --git a/src/graph/planner/plan/Query.h b/src/graph/planner/plan/Query.h index 48cdce7f335..a7ee8f8047c 100644 --- a/src/graph/planner/plan/Query.h +++ b/src/graph/planner/plan/Query.h @@ -1544,6 +1544,10 @@ class AppendVertices final : public GetVertices { trackPrevPath_ = track; } + Status pruneProperties(PropertyTracker& propsUsed, + graph::QueryContext* qctx, + GraphSpaceID spaceID) override; + private: AppendVertices(QueryContext* qctx, PlanNode* input, GraphSpaceID space) : GetVertices(qctx, diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp index 3926c5411c7..0e91f37828c 100644 --- a/src/graph/util/ExpressionUtils.cpp +++ b/src/graph/util/ExpressionUtils.cpp @@ -10,10 +10,12 @@ #include #include "common/base/ObjectPool.h" +#include "common/base/Status.h" #include "common/expression/ArithmeticExpression.h" #include "common/expression/Expression.h" #include "common/expression/PropertyExpression.h" #include "common/function/AggFunctionManager.h" +#include "common/thrift/ThriftTypes.h" #include "graph/context/QueryContext.h" #include "graph/context/QueryExpressionContext.h" #include "graph/visitor/FoldConstantExprVisitor.h" @@ -1058,6 +1060,48 @@ bool ExpressionUtils::isGeoIndexAcceleratedPredicate(const Expression *expr) { return false; } +Status ExpressionUtils::extractPropsFromExprs(const Expression *expr, + PropertyTracker &propsUsed, + const graph::QueryContext *qctx, + GraphSpaceID spaceID) { + auto exprs = ExpressionUtils::collectAll(expr, + {Expression::Kind::kLabelTagProperty, + Expression::Kind::kInputProperty, + Expression::Kind::kVarProperty}); + for (auto *e : exprs) { + switch (e->kind()) { + case Expression::Kind::kLabelTagProperty: { + auto *labelTagPropExpr = static_cast(e); + auto &labelName = labelTagPropExpr->label(); + auto &tagName = labelTagPropExpr->sym(); + auto &propName = labelTagPropExpr->prop(); + auto ret = qctx->schemaMng()->toTagID(spaceID, tagName); + NG_RETURN_IF_ERROR(ret); + auto tagId = ret.value(); + propsUsed.vertexPropsMap[labelName][tagId].emplace(propName); + break; + } + case Expression::Kind::kInputProperty: { + auto *inputPropExpr = static_cast(e); + auto &colName = inputPropExpr->prop(); + propsUsed.colsSet.emplace(colName); + break; + } + case Expression::Kind::kVarProperty: { + auto *varPropExpr = static_cast(e); + auto &colName = varPropExpr->prop(); + propsUsed.colsSet.emplace(colName); + break; + } + case Expression::Kind::kTagProperty: + // case Expression::Kind::kPathBuilding: + default: + break; + } + } + return Status::OK(); +} + bool ExpressionUtils::checkExprDepth(const Expression *expr) { std::queue exprQueue; exprQueue.emplace(expr); diff --git a/src/graph/util/ExpressionUtils.h b/src/graph/util/ExpressionUtils.h index b5d3f642fc3..ff7848246ba 100644 --- a/src/graph/util/ExpressionUtils.h +++ b/src/graph/util/ExpressionUtils.h @@ -169,6 +169,13 @@ class ExpressionUtils { static bool isGeoIndexAcceleratedPredicate(const Expression* expr); static bool checkExprDepth(const Expression* expr); + + static Status extractPropsFromExprs(const Expression* expr, + PropertyTracker& propsUsed, + const graph::QueryContext* qctx, + GraphSpaceID spaceID); + + static constexpr int32_t kMaxDepth = 512; }; } // namespace graph From d82606f42e4dab2e7606ac2a4afc80ca663a5651 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Tue, 18 Jan 2022 10:15:12 +0800 Subject: [PATCH 02/27] postprocess works. --- src/graph/optimizer/Optimizer.cpp | 9 ++++++--- src/graph/planner/plan/Query.cpp | 21 +++++++++++++++++---- src/graph/planner/plan/Query.h | 4 ++++ 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/graph/optimizer/Optimizer.cpp b/src/graph/optimizer/Optimizer.cpp index f8555d10434..41d754f4938 100644 --- a/src/graph/optimizer/Optimizer.cpp +++ b/src/graph/optimizer/Optimizer.cpp @@ -32,15 +32,18 @@ StatusOr Optimizer::findBestPlan(QueryContext *qctx) { auto root = qctx->plan()->root(); auto spaceID = qctx->rctx()->session()->space().id; - auto status = preprocess(root, qctx, spaceID); - NG_RETURN_IF_ERROR(status); + // auto status = preprocess(root, qctx, spaceID); + // NG_RETURN_IF_ERROR(status); auto ret = prepare(optCtx.get(), root); NG_RETURN_IF_ERROR(ret); auto rootGroup = std::move(ret).value(); NG_RETURN_IF_ERROR(doExploration(optCtx.get(), rootGroup)); - return rootGroup->getPlan(); + auto *newRoot = rootGroup->getPlan(); + auto status = preprocess(const_cast(newRoot), qctx, spaceID); + NG_RETURN_IF_ERROR(status); + return newRoot; } Status Optimizer::preprocess(PlanNode *root, graph::QueryContext *qctx, GraphSpaceID spaceID) { diff --git a/src/graph/planner/plan/Query.cpp b/src/graph/planner/plan/Query.cpp index acb4ef3b174..611f4d32a5b 100644 --- a/src/graph/planner/plan/Query.cpp +++ b/src/graph/planner/plan/Query.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include "graph/util/ExpressionUtils.h" @@ -351,6 +352,20 @@ void Project::cloneMembers(const Project& p) { } } +Status Project::pruneProperties(PropertyTracker& propsUsed, + graph::QueryContext* qctx, + GraphSpaceID spaceID) { + if (cols_) { + for (auto* col : cols_->columns()) { + DCHECK_NOTNULL(col); + auto* expr = col->expr(); + NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(expr, propsUsed, qctx, spaceID)); + } + } + + return depsPruneProperties(propsUsed, qctx, spaceID); +} + std::unique_ptr Unwind::explain() const { auto desc = SingleInputNode::explain(); addDescription("alias", alias(), desc.get()); @@ -768,8 +783,7 @@ Status AppendVertices::pruneProperties(PropertyTracker& propsUsed, auto it = propsUsed.colsSet.find(nodeAlias); if (it != propsUsed.colsSet.end()) { // All properties are used propsUsed.colsSet.erase(it); - NG_RETURN_IF_ERROR(depsPruneProperties(propsUsed, qctx, spaceID)); - return Status::OK(); + return depsPruneProperties(propsUsed, qctx, spaceID); } if (vFilter_ != nullptr) { @@ -792,8 +806,7 @@ Status AppendVertices::pruneProperties(PropertyTracker& propsUsed, } } - NG_RETURN_IF_ERROR(depsPruneProperties(propsUsed, qctx, spaceID)); - return Status::OK(); + return depsPruneProperties(propsUsed, qctx, spaceID); } std::unique_ptr BiJoin::explain() const { diff --git a/src/graph/planner/plan/Query.h b/src/graph/planner/plan/Query.h index a7ee8f8047c..f353b47caed 100644 --- a/src/graph/planner/plan/Query.h +++ b/src/graph/planner/plan/Query.h @@ -826,6 +826,10 @@ class Project final : public SingleInputNode { PlanNode* clone() const override; std::unique_ptr explain() const override; + Status pruneProperties(PropertyTracker& propsUsed, + graph::QueryContext* qctx, + GraphSpaceID spaceID) override; + private: Project(QueryContext* qctx, PlanNode* input, YieldColumns* cols); From c453057e7f9cc38b69c1aa2cd611365c854a9fc2 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Tue, 18 Jan 2022 16:31:37 +0800 Subject: [PATCH 03/27] add Travese::pruneProperties --- src/graph/planner/plan/Query.cpp | 88 ++++++++++++++++++++++++++++---- src/graph/planner/plan/Query.h | 4 ++ 2 files changed, 81 insertions(+), 11 deletions(-) diff --git a/src/graph/planner/plan/Query.cpp b/src/graph/planner/plan/Query.cpp index 611f4d32a5b..d3152e79947 100644 --- a/src/graph/planner/plan/Query.cpp +++ b/src/graph/planner/plan/Query.cpp @@ -749,6 +749,67 @@ std::unique_ptr Traverse::explain() const { return desc; } +Status Traverse::pruneProperties(PropertyTracker& propsUsed, + graph::QueryContext* qctx, + GraphSpaceID spaceID) { + auto& colNames = this->colNames(); + DCHECK_GE(colNames.size(), 2); + auto& nodeAlias = colNames[colNames.size() - 2]; + auto& edgeAlias = colNames.back(); + UNUSED(edgeAlias); + + auto it = propsUsed.colsSet.find(nodeAlias); + if (it != propsUsed.colsSet.end()) { // All properties are used + propsUsed.colsSet.erase(it); + return depsPruneProperties(propsUsed, qctx, spaceID); + } + + if (vFilter_ != nullptr) { + NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(vFilter_, propsUsed, qctx, spaceID)); + } + // if (eFilter_ != nullptr) { + // NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(eFilter_, propsUsed, qctx, + // spaceID)); + // } + + auto* vertexProps = this->vertexProps(); + if (vertexProps != nullptr) { + auto prunedVertexProps = std::make_unique>(); + prunedVertexProps->reserve(vertexProps->size()); + + auto it2 = propsUsed.vertexPropsMap.find(nodeAlias); + if (it2 == propsUsed.vertexPropsMap.end()) { // nodeAlias is not used + for (const auto& vProp : *vertexProps) { + VertexProp newVProp; + newVProp.tag_ref() = vProp.tag_ref().value(); + std::vector propNames{nebula::kTag}; + newVProp.props_ref() = std::move(propNames); + prunedVertexProps->emplace_back(std::move(newVProp)); + } + } else { + auto& usedVertexProps = it2->second; + for (const auto& vProp : *vertexProps) { + VertexProp newVProp; + auto tagID = vProp.tag_ref().value(); + newVProp.tag_ref() = tagID; + std::vector propNames{nebula::kTag}; + auto it3 = usedVertexProps.find(tagID); + if (it3 != usedVertexProps.end()) { + for (const auto& prop : it3->second) { + propNames.emplace_back(prop); + } + } + newVProp.props_ref() = std::move(propNames); + prunedVertexProps->emplace_back(std::move(newVProp)); + } + } + + setVertexProps(std::move(prunedVertexProps)); + } + + return depsPruneProperties(propsUsed, qctx, spaceID); +} + AppendVertices* AppendVertices::clone() const { auto newAV = AppendVertices::make(qctx_, nullptr, space_); newAV->cloneMembers(*this); @@ -778,8 +839,9 @@ std::unique_ptr AppendVertices::explain() const { Status AppendVertices::pruneProperties(PropertyTracker& propsUsed, graph::QueryContext* qctx, GraphSpaceID spaceID) { - DCHECK(!colNames().empty()); - auto& nodeAlias = colNames().back(); + auto& colNames = this->colNames(); + DCHECK(!colNames.empty()); + auto& nodeAlias = colNames.back(); auto it = propsUsed.colsSet.find(nodeAlias); if (it != propsUsed.colsSet.end()) { // All properties are used propsUsed.colsSet.erase(it); @@ -791,19 +853,23 @@ Status AppendVertices::pruneProperties(PropertyTracker& propsUsed, } auto* vertexProps = props(); if (vertexProps != nullptr) { + auto prunedVertexProps = std::make_unique>(); auto it2 = propsUsed.vertexPropsMap.find(nodeAlias); if (it2 != propsUsed.vertexPropsMap.end()) { - auto& tagPropsMap = it2->second; - auto prunedTagsProps = std::make_unique>(); - prunedTagsProps->reserve(tagPropsMap.size()); - for (auto& [tagId, tagProps] : tagPropsMap) { - VertexProp vProp; - vProp.tag_ref() = tagId; - vProp.props_ref() = std::vector(tagProps.begin(), tagProps.end()); - prunedTagsProps->emplace_back(std::move(vProp)); + auto& usedVertexProps = it2->second; + prunedVertexProps->reserve(usedVertexProps.size()); + for (auto& [tagId, vProps] : usedVertexProps) { + VertexProp newVProp; + newVProp.tag_ref() = tagId; + newVProp.props_ref() = std::vector(vProps.begin(), vProps.end()); + prunedVertexProps->emplace_back(std::move(newVProp)); } - setVertexProps(std::move(prunedTagsProps)); + } else { + // AppendVertices should be deleted + // Mark the node as toBeDeleted + // It should be done by ColumnPruner } + setVertexProps(std::move(prunedVertexProps)); } return depsPruneProperties(propsUsed, qctx, spaceID); diff --git a/src/graph/planner/plan/Query.h b/src/graph/planner/plan/Query.h index f353b47caed..d6ced759904 100644 --- a/src/graph/planner/plan/Query.h +++ b/src/graph/planner/plan/Query.h @@ -1507,6 +1507,10 @@ class Traverse final : public GetNeighbors { trackPrevPath_ = track; } + Status pruneProperties(PropertyTracker& propsUsed, + graph::QueryContext* qctx, + GraphSpaceID spaceID) override; + private: Traverse(QueryContext* qctx, PlanNode* input, GraphSpaceID space) : GetNeighbors(qctx, Kind::kTraverse, input, space) { From 4bbf08170bf9173c1b54b36a04f2324ea94576c2 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Tue, 18 Jan 2022 17:02:43 +0800 Subject: [PATCH 04/27] make vertexprops of travese be null when node alias is not used --- src/graph/planner/plan/Query.cpp | 36 +++++++------------------------- 1 file changed, 7 insertions(+), 29 deletions(-) diff --git a/src/graph/planner/plan/Query.cpp b/src/graph/planner/plan/Query.cpp index d3152e79947..d993c3930cd 100644 --- a/src/graph/planner/plan/Query.cpp +++ b/src/graph/planner/plan/Query.cpp @@ -755,8 +755,6 @@ Status Traverse::pruneProperties(PropertyTracker& propsUsed, auto& colNames = this->colNames(); DCHECK_GE(colNames.size(), 2); auto& nodeAlias = colNames[colNames.size() - 2]; - auto& edgeAlias = colNames.back(); - UNUSED(edgeAlias); auto it = propsUsed.colsSet.find(nodeAlias); if (it != propsUsed.colsSet.end()) { // All properties are used @@ -767,44 +765,24 @@ Status Traverse::pruneProperties(PropertyTracker& propsUsed, if (vFilter_ != nullptr) { NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(vFilter_, propsUsed, qctx, spaceID)); } - // if (eFilter_ != nullptr) { - // NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(eFilter_, propsUsed, qctx, - // spaceID)); - // } auto* vertexProps = this->vertexProps(); if (vertexProps != nullptr) { - auto prunedVertexProps = std::make_unique>(); - prunedVertexProps->reserve(vertexProps->size()); - auto it2 = propsUsed.vertexPropsMap.find(nodeAlias); if (it2 == propsUsed.vertexPropsMap.end()) { // nodeAlias is not used - for (const auto& vProp : *vertexProps) { - VertexProp newVProp; - newVProp.tag_ref() = vProp.tag_ref().value(); - std::vector propNames{nebula::kTag}; - newVProp.props_ref() = std::move(propNames); - prunedVertexProps->emplace_back(std::move(newVProp)); - } + setVertexProps(nullptr); } else { + auto prunedVertexProps = std::make_unique>(); auto& usedVertexProps = it2->second; - for (const auto& vProp : *vertexProps) { + prunedVertexProps->reserve(usedVertexProps.size()); + for (auto& [tagId, vProps] : usedVertexProps) { VertexProp newVProp; - auto tagID = vProp.tag_ref().value(); - newVProp.tag_ref() = tagID; - std::vector propNames{nebula::kTag}; - auto it3 = usedVertexProps.find(tagID); - if (it3 != usedVertexProps.end()) { - for (const auto& prop : it3->second) { - propNames.emplace_back(prop); - } - } - newVProp.props_ref() = std::move(propNames); + newVProp.tag_ref() = tagId; + newVProp.props_ref() = std::vector(vProps.begin(), vProps.end()); prunedVertexProps->emplace_back(std::move(newVProp)); } + setVertexProps(std::move(prunedVertexProps)); } - - setVertexProps(std::move(prunedVertexProps)); } return depsPruneProperties(propsUsed, qctx, spaceID); From 180bcbfe769a851ae249c9f2ee72c4ff19bbe0be Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Tue, 18 Jan 2022 18:08:58 +0800 Subject: [PATCH 05/27] fix extractPropsFromExps for tagPropExpr and edgePropexpr, vFilter and eFilter --- src/graph/planner/plan/Query.cpp | 6 +++-- src/graph/util/ExpressionUtils.cpp | 35 +++++++++++++++++++++++++----- src/graph/util/ExpressionUtils.h | 3 ++- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/graph/planner/plan/Query.cpp b/src/graph/planner/plan/Query.cpp index d993c3930cd..43293076782 100644 --- a/src/graph/planner/plan/Query.cpp +++ b/src/graph/planner/plan/Query.cpp @@ -763,7 +763,8 @@ Status Traverse::pruneProperties(PropertyTracker& propsUsed, } if (vFilter_ != nullptr) { - NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(vFilter_, propsUsed, qctx, spaceID)); + NG_RETURN_IF_ERROR( + ExpressionUtils::extractPropsFromExprs(vFilter_, propsUsed, qctx, spaceID, nodeAlias)); } auto* vertexProps = this->vertexProps(); @@ -827,7 +828,8 @@ Status AppendVertices::pruneProperties(PropertyTracker& propsUsed, } if (vFilter_ != nullptr) { - NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(vFilter_, propsUsed, qctx, spaceID)); + NG_RETURN_IF_ERROR( + ExpressionUtils::extractPropsFromExprs(vFilter_, propsUsed, qctx, spaceID, nodeAlias)); } auto* vertexProps = props(); if (vertexProps != nullptr) { diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp index 0e91f37828c..2b607227f20 100644 --- a/src/graph/util/ExpressionUtils.cpp +++ b/src/graph/util/ExpressionUtils.cpp @@ -1063,22 +1063,25 @@ bool ExpressionUtils::isGeoIndexAcceleratedPredicate(const Expression *expr) { Status ExpressionUtils::extractPropsFromExprs(const Expression *expr, PropertyTracker &propsUsed, const graph::QueryContext *qctx, - GraphSpaceID spaceID) { + GraphSpaceID spaceID, + const std::string &entityAlias) { auto exprs = ExpressionUtils::collectAll(expr, {Expression::Kind::kLabelTagProperty, Expression::Kind::kInputProperty, - Expression::Kind::kVarProperty}); + Expression::Kind::kVarProperty, + Expression::Kind::kTagProperty, + Expression::Kind::kEdgeProperty}); for (auto *e : exprs) { switch (e->kind()) { case Expression::Kind::kLabelTagProperty: { auto *labelTagPropExpr = static_cast(e); - auto &labelName = labelTagPropExpr->label(); + auto &nodeAlias = labelTagPropExpr->label(); auto &tagName = labelTagPropExpr->sym(); auto &propName = labelTagPropExpr->prop(); auto ret = qctx->schemaMng()->toTagID(spaceID, tagName); NG_RETURN_IF_ERROR(ret); auto tagId = ret.value(); - propsUsed.vertexPropsMap[labelName][tagId].emplace(propName); + propsUsed.vertexPropsMap[nodeAlias][tagId].emplace(propName); break; } case Expression::Kind::kInputProperty: { @@ -1093,8 +1096,28 @@ Status ExpressionUtils::extractPropsFromExprs(const Expression *expr, propsUsed.colsSet.emplace(colName); break; } - case Expression::Kind::kTagProperty: - // case Expression::Kind::kPathBuilding: + case Expression::Kind::kTagProperty: { + auto *tagPropExpr = static_cast(e); + auto &tagName = tagPropExpr->sym(); + auto &propName = tagPropExpr->prop(); + auto ret = qctx->schemaMng()->toTagID(spaceID, tagName); + NG_RETURN_IF_ERROR(ret); + auto tagId = ret.value(); + DCHECK(!entityAlias.empty()); + propsUsed.vertexPropsMap[entityAlias][tagId].emplace(propName); + break; + } + case Expression::Kind::kEdgeProperty: { + auto *edgePropExpr = static_cast(e); + auto &edgeName = edgePropExpr->sym(); + auto &propName = edgePropExpr->prop(); + auto ret = qctx->schemaMng()->toTagID(spaceID, edgeName); + NG_RETURN_IF_ERROR(ret); + auto edgeType = ret.value(); + DCHECK(!entityAlias.empty()); + propsUsed.vertexPropsMap[entityAlias][edgeType].emplace(propName); + break; + } default: break; } diff --git a/src/graph/util/ExpressionUtils.h b/src/graph/util/ExpressionUtils.h index ff7848246ba..25f13f7ee0d 100644 --- a/src/graph/util/ExpressionUtils.h +++ b/src/graph/util/ExpressionUtils.h @@ -173,7 +173,8 @@ class ExpressionUtils { static Status extractPropsFromExprs(const Expression* expr, PropertyTracker& propsUsed, const graph::QueryContext* qctx, - GraphSpaceID spaceID); + GraphSpaceID spaceID, + const std::string& entityAlias = ""); static constexpr int32_t kMaxDepth = 512; }; From e3e724833b40d1b782a07823302f2659f63a058e Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Wed, 19 Jan 2022 11:25:14 +0800 Subject: [PATCH 06/27] add PropertyTracker::update for project node --- src/common/expression/Expression.cpp | 27 +++++++++++++++++++++++++++ src/common/expression/Expression.h | 4 ++++ src/graph/optimizer/Optimizer.cpp | 12 +++++++++--- src/graph/optimizer/Optimizer.h | 2 ++ src/graph/planner/plan/Query.cpp | 16 ++++++++++++++-- 5 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/common/expression/Expression.cpp b/src/common/expression/Expression.cpp index f6cacf218ec..77cdfeba0b4 100644 --- a/src/common/expression/Expression.cpp +++ b/src/common/expression/Expression.cpp @@ -741,4 +741,31 @@ std::ostream& operator<<(std::ostream& os, Expression::Kind kind) { return os; } +Status PropertyTracker::update(const std::string& oldName, const std::string& newName) { + auto it1 = vertexPropsMap.find(oldName); + bool has1 = it1 != vertexPropsMap.end(); + auto it2 = edgePropsMap.find(oldName); + bool has2 = it2 != edgePropsMap.end(); + if (has1 && has2) { + return Status::Error("Duplicated property name: %s", oldName.c_str()); + } + if (has1) { + vertexPropsMap[newName] = vertexPropsMap[oldName]; + vertexPropsMap.erase(it1); + } + if (has2) { + edgePropsMap[newName] = edgePropsMap[oldName]; + edgePropsMap.erase(it2); + } + + auto it3 = colsSet.find(oldName); + bool has3 = it3 != colsSet.end(); + if (has3) { + colsSet.erase(it3); + colsSet.insert(newName); + } + + return Status::OK(); +} + } // namespace nebula diff --git a/src/common/expression/Expression.h b/src/common/expression/Expression.h index 3653817bae3..3f2de2c3b6f 100644 --- a/src/common/expression/Expression.h +++ b/src/common/expression/Expression.h @@ -8,7 +8,9 @@ #include "common/base/Base.h" #include "common/base/ObjectPool.h" +#include "common/base/Status.h" #include "common/context/ExpressionContext.h" +#include "common/datatypes/Edge.h" #include "common/datatypes/Value.h" namespace nebula { @@ -229,6 +231,8 @@ struct PropertyTracker { std::unordered_map>> edgePropsMap; std::unordered_set colsSet; + + Status update(const std::string& oldName, const std::string& newName); }; } // namespace nebula diff --git a/src/graph/optimizer/Optimizer.cpp b/src/graph/optimizer/Optimizer.cpp index 41d754f4938..b1e8e089580 100644 --- a/src/graph/optimizer/Optimizer.cpp +++ b/src/graph/optimizer/Optimizer.cpp @@ -41,12 +41,18 @@ StatusOr Optimizer::findBestPlan(QueryContext *qctx) { NG_RETURN_IF_ERROR(doExploration(optCtx.get(), rootGroup)); auto *newRoot = rootGroup->getPlan(); - auto status = preprocess(const_cast(newRoot), qctx, spaceID); - NG_RETURN_IF_ERROR(status); + + auto status2 = postprocess(const_cast(newRoot), qctx, spaceID); + NG_RETURN_IF_ERROR(status2); return newRoot; } -Status Optimizer::preprocess(PlanNode *root, graph::QueryContext *qctx, GraphSpaceID spaceID) { +// Status Optimizer::preprocess(PlanNode *root, graph::QueryContext *qctx, GraphSpaceID spaceID) { +// ColumnTracker colsUsed; +// return root->pruneCulumns(colsUsed, qctx, spaceID); +// } + +Status Optimizer::postprocess(PlanNode *root, graph::QueryContext *qctx, GraphSpaceID spaceID) { PropertyTracker propsUsed; return root->pruneProperties(propsUsed, qctx, spaceID); } diff --git a/src/graph/optimizer/Optimizer.h b/src/graph/optimizer/Optimizer.h index efc91db3f32..dc2425d672a 100644 --- a/src/graph/optimizer/Optimizer.h +++ b/src/graph/optimizer/Optimizer.h @@ -33,6 +33,8 @@ class Optimizer final { private: Status preprocess(graph::PlanNode *root, graph::QueryContext *qctx, GraphSpaceID spaceID); + Status postprocess(graph::PlanNode *root, graph::QueryContext *qctx, GraphSpaceID spaceID); + StatusOr prepare(OptContext *ctx, graph::PlanNode *root); Status doExploration(OptContext *octx, OptGroup *rootGroup); diff --git a/src/graph/planner/plan/Query.cpp b/src/graph/planner/plan/Query.cpp index 43293076782..62efc87bb22 100644 --- a/src/graph/planner/plan/Query.cpp +++ b/src/graph/planner/plan/Query.cpp @@ -356,9 +356,21 @@ Status Project::pruneProperties(PropertyTracker& propsUsed, graph::QueryContext* qctx, GraphSpaceID spaceID) { if (cols_) { - for (auto* col : cols_->columns()) { - DCHECK_NOTNULL(col); + const auto& columns = cols_->columns(); + auto& colNames = this->colNames(); + for (size_t i = 0; i < columns.size(); ++i) { + auto* col = DCHECK_NOTNULL(columns[i]); auto* expr = col->expr(); + auto& alias = colNames[i]; + if (expr->kind() == Expression::Kind::kInputProperty) { + auto* inputPropExpr = static_cast(expr); + auto& propName = inputPropExpr->prop(); + NG_RETURN_IF_ERROR(propsUsed.update(alias, propName)); + } else if (expr->kind() == Expression::Kind::kVarProperty) { + auto* varPropExpr = static_cast(expr); + auto& propName = varPropExpr->prop(); + NG_RETURN_IF_ERROR(propsUsed.update(alias, propName)); + } NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(expr, propsUsed, qctx, spaceID)); } } From 0f36c8d78579a7b3790e35f4aac4116b4f973fd1 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Thu, 20 Jan 2022 10:41:43 +0800 Subject: [PATCH 07/27] add DeduceMatchPropsVisitor and FLAGS_enable_opt_collapse_project_rule --- src/graph/optimizer/Optimizer.cpp | 8 +- .../optimizer/rule/CollapseProjectRule.cpp | 5 + .../optimizer/rule/CollapseProjectRule.h | 2 + src/graph/util/ExpressionUtils.cpp | 62 +------ src/graph/visitor/CMakeLists.txt | 1 + src/graph/visitor/DeduceMatchPropsVisitor.cpp | 173 ++++++++++++++++++ src/graph/visitor/DeduceMatchPropsVisitor.h | 73 ++++++++ 7 files changed, 266 insertions(+), 58 deletions(-) create mode 100644 src/graph/visitor/DeduceMatchPropsVisitor.cpp create mode 100644 src/graph/visitor/DeduceMatchPropsVisitor.h diff --git a/src/graph/optimizer/Optimizer.cpp b/src/graph/optimizer/Optimizer.cpp index b1e8e089580..fb41d873427 100644 --- a/src/graph/optimizer/Optimizer.cpp +++ b/src/graph/optimizer/Optimizer.cpp @@ -33,7 +33,9 @@ StatusOr Optimizer::findBestPlan(QueryContext *qctx) { auto spaceID = qctx->rctx()->session()->space().id; // auto status = preprocess(root, qctx, spaceID); - // NG_RETURN_IF_ERROR(status); + // if (!status.ok()) { + // LOG(ERROR) << "Failed to preprocess plan: " << status; + // } auto ret = prepare(optCtx.get(), root); NG_RETURN_IF_ERROR(ret); @@ -43,7 +45,9 @@ StatusOr Optimizer::findBestPlan(QueryContext *qctx) { auto *newRoot = rootGroup->getPlan(); auto status2 = postprocess(const_cast(newRoot), qctx, spaceID); - NG_RETURN_IF_ERROR(status2); + if (!status2.ok()) { + LOG(ERROR) << "Failed to postprocess plan: " << status2; + } return newRoot; } diff --git a/src/graph/optimizer/rule/CollapseProjectRule.cpp b/src/graph/optimizer/rule/CollapseProjectRule.cpp index 6a8ae3d552d..8bd59a852c0 100644 --- a/src/graph/optimizer/rule/CollapseProjectRule.cpp +++ b/src/graph/optimizer/rule/CollapseProjectRule.cpp @@ -11,6 +11,8 @@ #include "graph/planner/plan/Query.h" #include "graph/util/ExpressionUtils.h" +DEFINE_bool(enable_opt_collapse_project_rule, true, ""); + using nebula::graph::PlanNode; using nebula::graph::QueryContext; @@ -110,6 +112,9 @@ StatusOr CollapseProjectRule::transform( } bool CollapseProjectRule::match(OptContext* octx, const MatchedResult& matched) const { + if (!FLAGS_enable_opt_collapse_project_rule) { + return false; + } return OptRule::match(octx, matched); } diff --git a/src/graph/optimizer/rule/CollapseProjectRule.h b/src/graph/optimizer/rule/CollapseProjectRule.h index 3b57da7cfa0..a50687174cb 100644 --- a/src/graph/optimizer/rule/CollapseProjectRule.h +++ b/src/graph/optimizer/rule/CollapseProjectRule.h @@ -8,6 +8,8 @@ #include "graph/optimizer/OptRule.h" +DECLARE_bool(enable_opt_collapse_project_rule); + namespace nebula { namespace opt { diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp index 2b607227f20..ba9b59d2980 100644 --- a/src/graph/util/ExpressionUtils.cpp +++ b/src/graph/util/ExpressionUtils.cpp @@ -18,6 +18,7 @@ #include "common/thrift/ThriftTypes.h" #include "graph/context/QueryContext.h" #include "graph/context/QueryExpressionContext.h" +#include "graph/visitor/DeduceMatchPropsVisitor.h" #include "graph/visitor/FoldConstantExprVisitor.h" DEFINE_int32(max_expression_depth, 512, "Max depth of expression tree."); @@ -1065,63 +1066,12 @@ Status ExpressionUtils::extractPropsFromExprs(const Expression *expr, const graph::QueryContext *qctx, GraphSpaceID spaceID, const std::string &entityAlias) { - auto exprs = ExpressionUtils::collectAll(expr, - {Expression::Kind::kLabelTagProperty, - Expression::Kind::kInputProperty, - Expression::Kind::kVarProperty, - Expression::Kind::kTagProperty, - Expression::Kind::kEdgeProperty}); - for (auto *e : exprs) { - switch (e->kind()) { - case Expression::Kind::kLabelTagProperty: { - auto *labelTagPropExpr = static_cast(e); - auto &nodeAlias = labelTagPropExpr->label(); - auto &tagName = labelTagPropExpr->sym(); - auto &propName = labelTagPropExpr->prop(); - auto ret = qctx->schemaMng()->toTagID(spaceID, tagName); - NG_RETURN_IF_ERROR(ret); - auto tagId = ret.value(); - propsUsed.vertexPropsMap[nodeAlias][tagId].emplace(propName); - break; - } - case Expression::Kind::kInputProperty: { - auto *inputPropExpr = static_cast(e); - auto &colName = inputPropExpr->prop(); - propsUsed.colsSet.emplace(colName); - break; - } - case Expression::Kind::kVarProperty: { - auto *varPropExpr = static_cast(e); - auto &colName = varPropExpr->prop(); - propsUsed.colsSet.emplace(colName); - break; - } - case Expression::Kind::kTagProperty: { - auto *tagPropExpr = static_cast(e); - auto &tagName = tagPropExpr->sym(); - auto &propName = tagPropExpr->prop(); - auto ret = qctx->schemaMng()->toTagID(spaceID, tagName); - NG_RETURN_IF_ERROR(ret); - auto tagId = ret.value(); - DCHECK(!entityAlias.empty()); - propsUsed.vertexPropsMap[entityAlias][tagId].emplace(propName); - break; - } - case Expression::Kind::kEdgeProperty: { - auto *edgePropExpr = static_cast(e); - auto &edgeName = edgePropExpr->sym(); - auto &propName = edgePropExpr->prop(); - auto ret = qctx->schemaMng()->toTagID(spaceID, edgeName); - NG_RETURN_IF_ERROR(ret); - auto edgeType = ret.value(); - DCHECK(!entityAlias.empty()); - propsUsed.vertexPropsMap[entityAlias][edgeType].emplace(propName); - break; - } - default: - break; - } + DeduceMatchPropsVisitor visitor(qctx, spaceID, propsUsed, entityAlias); + const_cast(expr)->accept(&visitor); + if (!visitor.ok()) { + return visitor.status(); } + return Status::OK(); } diff --git a/src/graph/visitor/CMakeLists.txt b/src/graph/visitor/CMakeLists.txt index c02f9d91f6a..2477c8eee25 100644 --- a/src/graph/visitor/CMakeLists.txt +++ b/src/graph/visitor/CMakeLists.txt @@ -6,6 +6,7 @@ nebula_add_library( expr_visitor_obj OBJECT ExprVisitorImpl.cpp DeducePropsVisitor.cpp + DeduceMatchPropsVisitor.cpp DeduceTypeVisitor.cpp ExtractPropExprVisitor.cpp ExtractFilterExprVisitor.cpp diff --git a/src/graph/visitor/DeduceMatchPropsVisitor.cpp b/src/graph/visitor/DeduceMatchPropsVisitor.cpp new file mode 100644 index 00000000000..453eb4beab7 --- /dev/null +++ b/src/graph/visitor/DeduceMatchPropsVisitor.cpp @@ -0,0 +1,173 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License. + */ + +#include "graph/visitor/DeduceMatchPropsVisitor.h" + +#include + +#include "common/expression/Expression.h" +#include "graph/context/QueryContext.h" + +namespace nebula { +namespace graph { + +// visitor +DeduceMatchPropsVisitor::DeduceMatchPropsVisitor(const QueryContext *qctx, + GraphSpaceID space, + PropertyTracker &propsUsed, + const std::string &entityAlias) + : qctx_(qctx), space_(space), propsUsed_(propsUsed), entityAlias_(entityAlias) { + DCHECK(qctx != nullptr); +} + +void DeduceMatchPropsVisitor::visit(TagPropertyExpression *expr) { + auto &tagName = expr->sym(); + auto &propName = expr->prop(); + auto ret = qctx_->schemaMng()->toTagID(space_, tagName); + if (!ret.ok()) { + status_ = std::move(ret).status(); + return; + } + auto tagId = ret.value(); + propsUsed_.vertexPropsMap[entityAlias_][tagId].emplace(propName); +} + +void DeduceMatchPropsVisitor::visit(EdgePropertyExpression *expr) { + auto &edgeName = expr->sym(); + auto &propName = expr->prop(); + auto ret = qctx_->schemaMng()->toEdgeType(space_, edgeName); + if (!ret.ok()) { + status_ = std::move(ret).status(); + return; + } + auto edgeType = ret.value(); + propsUsed_.edgePropsMap[entityAlias_][edgeType].emplace(propName); +} + +void DeduceMatchPropsVisitor::visit(LabelTagPropertyExpression *expr) { + auto status = qctx_->schemaMng()->toTagID(space_, expr->sym()); + if (!status.ok()) { + status_ = std::move(status).status(); + return; + } + auto &nodeAlias = expr->label(); + auto &tagName = expr->sym(); + auto &propName = expr->prop(); + auto ret = qctx_->schemaMng()->toTagID(space_, tagName); + if (!ret.ok()) { + status_ = std::move(ret).status(); + return; + } + auto tagId = ret.value(); + propsUsed_.vertexPropsMap[nodeAlias][tagId].emplace(propName); +} + +void DeduceMatchPropsVisitor::visit(InputPropertyExpression *expr) { + auto &colName = expr->prop(); + propsUsed_.colsSet.emplace(colName); +} + +void DeduceMatchPropsVisitor::visit(VariablePropertyExpression *expr) { + auto &colName = expr->prop(); + propsUsed_.colsSet.emplace(colName); +} + +// void DeduceMatchPropsVisitor::visit(AttributeExpression *expr) { +// auto *lhs = expr->left(); +// auto *rhs = expr->right(); +// if (rhs->kind() != Expression::Kind::kConstant) { +// return; +// } +// auto *constExpr = static_cast(rhs); +// auto &propName = constExpr->value(); +// switch (lhs->kind()) { +// case Expression::Kind::kVarProperty: { // $e.name +// auto *varPropExpr = static_cast(lhs); +// auto &edgeAlias = varPropExpr->prop(); +// propsUsed_.edgePropsMap[edgeAlias][0].emplace(propName); +// break; +// } +// case Expression::Kind::kInputProperty: { +// auto *inputPropExpr = static_cast(lhs); +// auto &edgeAlias = inputPropExpr->prop(); +// propsUsed_.edgePropsMap[edgeAlias][0].emplace(propName); +// break; +// } +// case Expression::Kind::kSubscript: { // $-.e[0].name +// auto *subscriptExpr = static_cast(lhs); +// auto *subLeftExpr = subscriptExpr->left(); +// if (subLeftExpr->kind() == Expression::Kind::kInputProperty) { +// auto *inputPropExpr = static_cast(subLeftExpr); +// auto &edgeAlias = inputPropExpr->prop(); +// propsUsed_.edgePropsMap[edgeAlias][0].emplace(propName); +// } +// } +// default: +// break; +// } +// } + +void DeduceMatchPropsVisitor::visit(DestPropertyExpression *expr) { + UNUSED(expr); +} + +void DeduceMatchPropsVisitor::visit(SourcePropertyExpression *expr) { + UNUSED(expr); +} + +void DeduceMatchPropsVisitor::visit(EdgeSrcIdExpression *expr) { + UNUSED(expr); +} + +void DeduceMatchPropsVisitor::visit(EdgeTypeExpression *expr) { + UNUSED(expr); +} + +void DeduceMatchPropsVisitor::visit(EdgeRankExpression *expr) { + UNUSED(expr); +} + +void DeduceMatchPropsVisitor::visit(EdgeDstIdExpression *expr) { + UNUSED(expr); +} + +void DeduceMatchPropsVisitor::visit(UUIDExpression *expr) { + UNUSED(expr); +} + +void DeduceMatchPropsVisitor::visit(VariableExpression *expr) { + UNUSED(expr); +} + +void DeduceMatchPropsVisitor::visit(VersionedVariableExpression *expr) { + UNUSED(expr); +} + +void DeduceMatchPropsVisitor::visit(LabelExpression *expr) { + UNUSED(expr); +} + +void DeduceMatchPropsVisitor::visit(LabelAttributeExpression *expr) { + UNUSED(expr); +} + +void DeduceMatchPropsVisitor::visit(ConstantExpression *expr) { + UNUSED(expr); +} + +void DeduceMatchPropsVisitor::visit(ColumnExpression *expr) { + UNUSED(expr); +} + +void DeduceMatchPropsVisitor::visit(VertexExpression *expr) { + UNUSED(expr); +} + +void DeduceMatchPropsVisitor::visit(EdgeExpression *expr) { + UNUSED(expr); +} + +} // namespace graph +} // namespace nebula diff --git a/src/graph/visitor/DeduceMatchPropsVisitor.h b/src/graph/visitor/DeduceMatchPropsVisitor.h new file mode 100644 index 00000000000..3b676e11eb7 --- /dev/null +++ b/src/graph/visitor/DeduceMatchPropsVisitor.h @@ -0,0 +1,73 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License. + */ + +#ifndef GRAPH_VISITOR_DEDUCEMATCHPROPSVISITOR_H +#define GRAPH_VISITOR_DEDUCEMATCHPROPSVISITOR_H + +#include "common/base/Status.h" +#include "common/expression/ListComprehensionExpression.h" +#include "common/expression/SubscriptExpression.h" +#include "common/thrift/ThriftTypes.h" +#include "graph/visitor/ExprVisitorImpl.h" + +namespace nebula { + +class Expression; + +namespace graph { + +class QueryContext; + +class DeduceMatchPropsVisitor : public ExprVisitorImpl { + public: + DeduceMatchPropsVisitor(const QueryContext* qctx, + GraphSpaceID space, + PropertyTracker& propsUsed, + const std::string& entityAlias); + + bool ok() const override { + return status_.ok(); + } + + const Status& status() const { + return status_; + } + + private: + using ExprVisitorImpl::visit; + void visit(TagPropertyExpression* expr) override; + void visit(EdgePropertyExpression* expr) override; + void visit(LabelTagPropertyExpression* expr) override; + void visit(InputPropertyExpression* expr) override; + void visit(VariablePropertyExpression* expr) override; + // void visit(AttributeExpression* expr) override; + void visit(SourcePropertyExpression* expr) override; + void visit(DestPropertyExpression* expr) override; + void visit(EdgeSrcIdExpression* expr) override; + void visit(EdgeTypeExpression* expr) override; + void visit(EdgeRankExpression* expr) override; + void visit(EdgeDstIdExpression* expr) override; + void visit(UUIDExpression* expr) override; + void visit(VariableExpression* expr) override; + void visit(VersionedVariableExpression* expr) override; + void visit(LabelExpression* expr) override; + void visit(LabelAttributeExpression* expr) override; + void visit(ConstantExpression* expr) override; + void visit(VertexExpression* expr) override; + void visit(EdgeExpression* expr) override; + void visit(ColumnExpression* expr) override; + + const QueryContext* qctx_{nullptr}; + GraphSpaceID space_; + PropertyTracker& propsUsed_; + std::string entityAlias_; + + Status status_; +}; + +} // namespace graph +} // namespace nebula + +#endif // GRAPH_VISITOR_DEDUCEMATCHPROPSVISITOR_H From 481bcbb2197733b40698501d223bac0dc024e022 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Thu, 20 Jan 2022 14:00:05 +0800 Subject: [PATCH 08/27] add Filter::pruneProperties and only 3 tck cases not passed --- src/graph/planner/plan/Query.cpp | 61 +++++++++++++++++++++++++------- src/graph/planner/plan/Query.h | 4 +++ 2 files changed, 52 insertions(+), 13 deletions(-) diff --git a/src/graph/planner/plan/Query.cpp b/src/graph/planner/plan/Query.cpp index 62efc87bb22..4272ba80765 100644 --- a/src/graph/planner/plan/Query.cpp +++ b/src/graph/planner/plan/Query.cpp @@ -283,6 +283,17 @@ void Filter::cloneMembers(const Filter& f) { needStableFilter_ = f.needStableFilter(); } +Status Filter::pruneProperties(PropertyTracker& propsUsed, + graph::QueryContext* qctx, + GraphSpaceID spaceID) { + if (condition_ != nullptr) { + NG_RETURN_IF_ERROR( + ExpressionUtils::extractPropsFromExprs(condition_, propsUsed, qctx, spaceID)); + } + + return depsPruneProperties(propsUsed, qctx, spaceID); +} + void SetOp::cloneMembers(const SetOp& s) { BinaryInputNode::cloneMembers(s); } @@ -788,11 +799,23 @@ Status Traverse::pruneProperties(PropertyTracker& propsUsed, auto prunedVertexProps = std::make_unique>(); auto& usedVertexProps = it2->second; prunedVertexProps->reserve(usedVertexProps.size()); - for (auto& [tagId, vProps] : usedVertexProps) { - VertexProp newVProp; - newVProp.tag_ref() = tagId; - newVProp.props_ref() = std::vector(vProps.begin(), vProps.end()); - prunedVertexProps->emplace_back(std::move(newVProp)); + for (auto& vertexProp : *vertexProps) { + auto tagId = vertexProp.tag_ref().value(); + auto& props = vertexProp.props_ref().value(); + auto it3 = usedVertexProps.find(tagId); + if (it3 != usedVertexProps.end()) { + auto& usedProps = it3->second; + VertexProp newVProp; + newVProp.tag_ref() = tagId; + std::vector newProps; + for (auto& prop : props) { + if (usedProps.find(prop) != usedProps.end()) { + newProps.emplace_back(prop); + } + } + newVProp.props_ref() = std::move(newProps); + prunedVertexProps->emplace_back(std::move(newVProp)); + } } setVertexProps(std::move(prunedVertexProps)); } @@ -850,16 +873,28 @@ Status AppendVertices::pruneProperties(PropertyTracker& propsUsed, if (it2 != propsUsed.vertexPropsMap.end()) { auto& usedVertexProps = it2->second; prunedVertexProps->reserve(usedVertexProps.size()); - for (auto& [tagId, vProps] : usedVertexProps) { - VertexProp newVProp; - newVProp.tag_ref() = tagId; - newVProp.props_ref() = std::vector(vProps.begin(), vProps.end()); - prunedVertexProps->emplace_back(std::move(newVProp)); + for (auto& vertexProp : *vertexProps) { + auto tagId = vertexProp.tag_ref().value(); + auto& props = vertexProp.props_ref().value(); + auto it3 = usedVertexProps.find(tagId); + if (it3 != usedVertexProps.end()) { + auto& usedProps = it3->second; + VertexProp newVProp; + newVProp.tag_ref() = tagId; + std::vector newProps; + for (auto& prop : props) { + if (usedProps.find(prop) != usedProps.end()) { + newProps.emplace_back(prop); + } + } + newVProp.props_ref() = std::move(newProps); + prunedVertexProps->emplace_back(std::move(newVProp)); + } } } else { - // AppendVertices should be deleted - // Mark the node as toBeDeleted - // It should be done by ColumnPruner + // AppendVertices should be deleted when no props are used by the parent node + // markAsToBeDeleted(); + // It could be done by ColumnPruner } setVertexProps(std::move(prunedVertexProps)); } diff --git a/src/graph/planner/plan/Query.h b/src/graph/planner/plan/Query.h index d6ced759904..f8890740789 100644 --- a/src/graph/planner/plan/Query.h +++ b/src/graph/planner/plan/Query.h @@ -730,6 +730,10 @@ class Filter final : public SingleInputNode { PlanNode* clone() const override; std::unique_ptr explain() const override; + Status pruneProperties(PropertyTracker& propsUsed, + graph::QueryContext* qctx, + GraphSpaceID spaceID) override; + private: Filter(QueryContext* qctx, PlanNode* input, Expression* condition, bool needStableFilter); void cloneMembers(const Filter&); From 6173b7254e958848905d9476413ffc5f8520df03 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Thu, 20 Jan 2022 16:35:03 +0800 Subject: [PATCH 09/27] Do not do propsUsed.colsSet.erase(it) --- src/graph/planner/plan/Query.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/graph/planner/plan/Query.cpp b/src/graph/planner/plan/Query.cpp index 4272ba80765..633aa1357ae 100644 --- a/src/graph/planner/plan/Query.cpp +++ b/src/graph/planner/plan/Query.cpp @@ -781,7 +781,7 @@ Status Traverse::pruneProperties(PropertyTracker& propsUsed, auto it = propsUsed.colsSet.find(nodeAlias); if (it != propsUsed.colsSet.end()) { // All properties are used - propsUsed.colsSet.erase(it); + // propsUsed.colsSet.erase(it); return depsPruneProperties(propsUsed, qctx, spaceID); } @@ -858,7 +858,7 @@ Status AppendVertices::pruneProperties(PropertyTracker& propsUsed, auto& nodeAlias = colNames.back(); auto it = propsUsed.colsSet.find(nodeAlias); if (it != propsUsed.colsSet.end()) { // All properties are used - propsUsed.colsSet.erase(it); + // propsUsed.colsSet.erase(it); return depsPruneProperties(propsUsed, qctx, spaceID); } From 8c78e8c45a203a0ca40c36cb89caf82885d2d52c Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Thu, 20 Jan 2022 16:55:35 +0800 Subject: [PATCH 10/27] revert labeltagpropexpr and pass all tck --- src/graph/visitor/DeduceMatchPropsVisitor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/graph/visitor/DeduceMatchPropsVisitor.cpp b/src/graph/visitor/DeduceMatchPropsVisitor.cpp index 453eb4beab7..c600eee33d7 100644 --- a/src/graph/visitor/DeduceMatchPropsVisitor.cpp +++ b/src/graph/visitor/DeduceMatchPropsVisitor.cpp @@ -52,7 +52,7 @@ void DeduceMatchPropsVisitor::visit(LabelTagPropertyExpression *expr) { status_ = std::move(status).status(); return; } - auto &nodeAlias = expr->label(); + auto &nodeAlias = static_cast(expr->label())->prop(); auto &tagName = expr->sym(); auto &propName = expr->prop(); auto ret = qctx_->schemaMng()->toTagID(space_, tagName); From 792d0f18e036c6d9a3380feb7df9e8a4c60c5365 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Fri, 21 Jan 2022 16:20:06 +0800 Subject: [PATCH 11/27] add hasAlias, add deduce types for id, src, dst func --- src/common/expression/Expression.cpp | 9 ++++++ src/common/expression/Expression.h | 1 + src/graph/planner/plan/Query.cpp | 29 +++++++++++++------ src/graph/visitor/DeduceMatchPropsVisitor.cpp | 13 +++++++++ src/graph/visitor/DeduceMatchPropsVisitor.h | 3 ++ 5 files changed, 46 insertions(+), 9 deletions(-) diff --git a/src/common/expression/Expression.cpp b/src/common/expression/Expression.cpp index 77cdfeba0b4..ff7abf5b39c 100644 --- a/src/common/expression/Expression.cpp +++ b/src/common/expression/Expression.cpp @@ -742,6 +742,10 @@ std::ostream& operator<<(std::ostream& os, Expression::Kind kind) { } Status PropertyTracker::update(const std::string& oldName, const std::string& newName) { + if (oldName == newName) { + return Status::OK(); + } + auto it1 = vertexPropsMap.find(oldName); bool has1 = it1 != vertexPropsMap.end(); auto it2 = edgePropsMap.find(oldName); @@ -768,4 +772,9 @@ Status PropertyTracker::update(const std::string& oldName, const std::string& ne return Status::OK(); } +bool PropertyTracker::hasAlias(const std::string& name) const { + return vertexPropsMap.find(name) != vertexPropsMap.end() || + edgePropsMap.find(name) != edgePropsMap.end() || colsSet.find(name) != colsSet.end(); +} + } // namespace nebula diff --git a/src/common/expression/Expression.h b/src/common/expression/Expression.h index 3f2de2c3b6f..4de1d46517c 100644 --- a/src/common/expression/Expression.h +++ b/src/common/expression/Expression.h @@ -233,6 +233,7 @@ struct PropertyTracker { std::unordered_set colsSet; Status update(const std::string& oldName, const std::string& newName); + bool hasAlias(const std::string& name) const; }; } // namespace nebula diff --git a/src/graph/planner/plan/Query.cpp b/src/graph/planner/plan/Query.cpp index 633aa1357ae..1a4923e935d 100644 --- a/src/graph/planner/plan/Query.cpp +++ b/src/graph/planner/plan/Query.cpp @@ -369,20 +369,31 @@ Status Project::pruneProperties(PropertyTracker& propsUsed, if (cols_) { const auto& columns = cols_->columns(); auto& colNames = this->colNames(); + std::vector aliasExists(colNames.size(), false); + for (size_t i = 0; i < columns.size(); ++i) { + aliasExists[i] = propsUsed.hasAlias(colNames[i]); + } for (size_t i = 0; i < columns.size(); ++i) { auto* col = DCHECK_NOTNULL(columns[i]); auto* expr = col->expr(); auto& alias = colNames[i]; - if (expr->kind() == Expression::Kind::kInputProperty) { - auto* inputPropExpr = static_cast(expr); - auto& propName = inputPropExpr->prop(); - NG_RETURN_IF_ERROR(propsUsed.update(alias, propName)); - } else if (expr->kind() == Expression::Kind::kVarProperty) { - auto* varPropExpr = static_cast(expr); - auto& propName = varPropExpr->prop(); - NG_RETURN_IF_ERROR(propsUsed.update(alias, propName)); + // First, try to rename alias + if (aliasExists[i]) { + if (expr->kind() == Expression::Kind::kInputProperty) { + auto* inputPropExpr = static_cast(expr); + auto& newAlias = inputPropExpr->prop(); + NG_RETURN_IF_ERROR(propsUsed.update(alias, newAlias)); + } else if (expr->kind() == Expression::Kind::kVarProperty) { + auto* varPropExpr = static_cast(expr); + auto& newAlias = varPropExpr->prop(); + NG_RETURN_IF_ERROR(propsUsed.update(alias, newAlias)); + } else { + // How to handle this case? + } + } else { + // If not, try to extract properties from expression + NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(expr, propsUsed, qctx, spaceID)); } - NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(expr, propsUsed, qctx, spaceID)); } } diff --git a/src/graph/visitor/DeduceMatchPropsVisitor.cpp b/src/graph/visitor/DeduceMatchPropsVisitor.cpp index c600eee33d7..f91b19a1430 100644 --- a/src/graph/visitor/DeduceMatchPropsVisitor.cpp +++ b/src/graph/visitor/DeduceMatchPropsVisitor.cpp @@ -109,6 +109,19 @@ void DeduceMatchPropsVisitor::visit(VariablePropertyExpression *expr) { // } // } +void DeduceMatchPropsVisitor::visit(FunctionCallExpression *expr) { + auto funName = expr->name(); + if (funName == "id" || funName == "src" || funName == "dst") { + return; + } + for (const auto &arg : expr->args()->args()) { + arg->accept(this); + if (!ok()) { + break; + } + } +} + void DeduceMatchPropsVisitor::visit(DestPropertyExpression *expr) { UNUSED(expr); } diff --git a/src/graph/visitor/DeduceMatchPropsVisitor.h b/src/graph/visitor/DeduceMatchPropsVisitor.h index 3b676e11eb7..d77c4b90ea9 100644 --- a/src/graph/visitor/DeduceMatchPropsVisitor.h +++ b/src/graph/visitor/DeduceMatchPropsVisitor.h @@ -7,6 +7,7 @@ #define GRAPH_VISITOR_DEDUCEMATCHPROPSVISITOR_H #include "common/base/Status.h" +#include "common/expression/FunctionCallExpression.h" #include "common/expression/ListComprehensionExpression.h" #include "common/expression/SubscriptExpression.h" #include "common/thrift/ThriftTypes.h" @@ -43,6 +44,8 @@ class DeduceMatchPropsVisitor : public ExprVisitorImpl { void visit(InputPropertyExpression* expr) override; void visit(VariablePropertyExpression* expr) override; // void visit(AttributeExpression* expr) override; + void visit(FunctionCallExpression* expr) override; + void visit(SourcePropertyExpression* expr) override; void visit(DestPropertyExpression* expr) override; void visit(EdgeSrcIdExpression* expr) override; From 7d4f9dc5e7e795951a45ab18abca7ba29135b9e5 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Fri, 21 Jan 2022 16:30:12 +0800 Subject: [PATCH 12/27] rename visitor --- src/graph/util/ExpressionUtils.cpp | 4 +- src/graph/visitor/CMakeLists.txt | 2 +- ...Visitor.cpp => PropertyTrackerVisitor.cpp} | 54 +++++++++---------- ...ropsVisitor.h => PropertyTrackerVisitor.h} | 16 +++--- 4 files changed, 38 insertions(+), 38 deletions(-) rename src/graph/visitor/{DeduceMatchPropsVisitor.cpp => PropertyTrackerVisitor.cpp} (66%) rename src/graph/visitor/{DeduceMatchPropsVisitor.h => PropertyTrackerVisitor.h} (82%) diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp index ba9b59d2980..6efb38bd064 100644 --- a/src/graph/util/ExpressionUtils.cpp +++ b/src/graph/util/ExpressionUtils.cpp @@ -18,8 +18,8 @@ #include "common/thrift/ThriftTypes.h" #include "graph/context/QueryContext.h" #include "graph/context/QueryExpressionContext.h" -#include "graph/visitor/DeduceMatchPropsVisitor.h" #include "graph/visitor/FoldConstantExprVisitor.h" +#include "graph/visitor/PropertyTrackerVisitor.h" DEFINE_int32(max_expression_depth, 512, "Max depth of expression tree."); @@ -1066,7 +1066,7 @@ Status ExpressionUtils::extractPropsFromExprs(const Expression *expr, const graph::QueryContext *qctx, GraphSpaceID spaceID, const std::string &entityAlias) { - DeduceMatchPropsVisitor visitor(qctx, spaceID, propsUsed, entityAlias); + PropertyTrackerVisitor visitor(qctx, spaceID, propsUsed, entityAlias); const_cast(expr)->accept(&visitor); if (!visitor.ok()) { return visitor.status(); diff --git a/src/graph/visitor/CMakeLists.txt b/src/graph/visitor/CMakeLists.txt index 2477c8eee25..653d0c5e711 100644 --- a/src/graph/visitor/CMakeLists.txt +++ b/src/graph/visitor/CMakeLists.txt @@ -6,7 +6,7 @@ nebula_add_library( expr_visitor_obj OBJECT ExprVisitorImpl.cpp DeducePropsVisitor.cpp - DeduceMatchPropsVisitor.cpp + PropertyTrackerVisitor.cpp DeduceTypeVisitor.cpp ExtractPropExprVisitor.cpp ExtractFilterExprVisitor.cpp diff --git a/src/graph/visitor/DeduceMatchPropsVisitor.cpp b/src/graph/visitor/PropertyTrackerVisitor.cpp similarity index 66% rename from src/graph/visitor/DeduceMatchPropsVisitor.cpp rename to src/graph/visitor/PropertyTrackerVisitor.cpp index f91b19a1430..c36bea1ff9b 100644 --- a/src/graph/visitor/DeduceMatchPropsVisitor.cpp +++ b/src/graph/visitor/PropertyTrackerVisitor.cpp @@ -3,7 +3,7 @@ * This source code is licensed under Apache 2.0 License. */ -#include "graph/visitor/DeduceMatchPropsVisitor.h" +#include "graph/visitor/PropertyTrackerVisitor.h" #include @@ -14,15 +14,15 @@ namespace nebula { namespace graph { // visitor -DeduceMatchPropsVisitor::DeduceMatchPropsVisitor(const QueryContext *qctx, - GraphSpaceID space, - PropertyTracker &propsUsed, - const std::string &entityAlias) +PropertyTrackerVisitor::PropertyTrackerVisitor(const QueryContext *qctx, + GraphSpaceID space, + PropertyTracker &propsUsed, + const std::string &entityAlias) : qctx_(qctx), space_(space), propsUsed_(propsUsed), entityAlias_(entityAlias) { DCHECK(qctx != nullptr); } -void DeduceMatchPropsVisitor::visit(TagPropertyExpression *expr) { +void PropertyTrackerVisitor::visit(TagPropertyExpression *expr) { auto &tagName = expr->sym(); auto &propName = expr->prop(); auto ret = qctx_->schemaMng()->toTagID(space_, tagName); @@ -34,7 +34,7 @@ void DeduceMatchPropsVisitor::visit(TagPropertyExpression *expr) { propsUsed_.vertexPropsMap[entityAlias_][tagId].emplace(propName); } -void DeduceMatchPropsVisitor::visit(EdgePropertyExpression *expr) { +void PropertyTrackerVisitor::visit(EdgePropertyExpression *expr) { auto &edgeName = expr->sym(); auto &propName = expr->prop(); auto ret = qctx_->schemaMng()->toEdgeType(space_, edgeName); @@ -46,7 +46,7 @@ void DeduceMatchPropsVisitor::visit(EdgePropertyExpression *expr) { propsUsed_.edgePropsMap[entityAlias_][edgeType].emplace(propName); } -void DeduceMatchPropsVisitor::visit(LabelTagPropertyExpression *expr) { +void PropertyTrackerVisitor::visit(LabelTagPropertyExpression *expr) { auto status = qctx_->schemaMng()->toTagID(space_, expr->sym()); if (!status.ok()) { status_ = std::move(status).status(); @@ -64,17 +64,17 @@ void DeduceMatchPropsVisitor::visit(LabelTagPropertyExpression *expr) { propsUsed_.vertexPropsMap[nodeAlias][tagId].emplace(propName); } -void DeduceMatchPropsVisitor::visit(InputPropertyExpression *expr) { +void PropertyTrackerVisitor::visit(InputPropertyExpression *expr) { auto &colName = expr->prop(); propsUsed_.colsSet.emplace(colName); } -void DeduceMatchPropsVisitor::visit(VariablePropertyExpression *expr) { +void PropertyTrackerVisitor::visit(VariablePropertyExpression *expr) { auto &colName = expr->prop(); propsUsed_.colsSet.emplace(colName); } -// void DeduceMatchPropsVisitor::visit(AttributeExpression *expr) { +// void PropertyTrackerVisitor::visit(AttributeExpression *expr) { // auto *lhs = expr->left(); // auto *rhs = expr->right(); // if (rhs->kind() != Expression::Kind::kConstant) { @@ -109,7 +109,7 @@ void DeduceMatchPropsVisitor::visit(VariablePropertyExpression *expr) { // } // } -void DeduceMatchPropsVisitor::visit(FunctionCallExpression *expr) { +void PropertyTrackerVisitor::visit(FunctionCallExpression *expr) { auto funName = expr->name(); if (funName == "id" || funName == "src" || funName == "dst") { return; @@ -122,63 +122,63 @@ void DeduceMatchPropsVisitor::visit(FunctionCallExpression *expr) { } } -void DeduceMatchPropsVisitor::visit(DestPropertyExpression *expr) { +void PropertyTrackerVisitor::visit(DestPropertyExpression *expr) { UNUSED(expr); } -void DeduceMatchPropsVisitor::visit(SourcePropertyExpression *expr) { +void PropertyTrackerVisitor::visit(SourcePropertyExpression *expr) { UNUSED(expr); } -void DeduceMatchPropsVisitor::visit(EdgeSrcIdExpression *expr) { +void PropertyTrackerVisitor::visit(EdgeSrcIdExpression *expr) { UNUSED(expr); } -void DeduceMatchPropsVisitor::visit(EdgeTypeExpression *expr) { +void PropertyTrackerVisitor::visit(EdgeTypeExpression *expr) { UNUSED(expr); } -void DeduceMatchPropsVisitor::visit(EdgeRankExpression *expr) { +void PropertyTrackerVisitor::visit(EdgeRankExpression *expr) { UNUSED(expr); } -void DeduceMatchPropsVisitor::visit(EdgeDstIdExpression *expr) { +void PropertyTrackerVisitor::visit(EdgeDstIdExpression *expr) { UNUSED(expr); } -void DeduceMatchPropsVisitor::visit(UUIDExpression *expr) { +void PropertyTrackerVisitor::visit(UUIDExpression *expr) { UNUSED(expr); } -void DeduceMatchPropsVisitor::visit(VariableExpression *expr) { +void PropertyTrackerVisitor::visit(VariableExpression *expr) { UNUSED(expr); } -void DeduceMatchPropsVisitor::visit(VersionedVariableExpression *expr) { +void PropertyTrackerVisitor::visit(VersionedVariableExpression *expr) { UNUSED(expr); } -void DeduceMatchPropsVisitor::visit(LabelExpression *expr) { +void PropertyTrackerVisitor::visit(LabelExpression *expr) { UNUSED(expr); } -void DeduceMatchPropsVisitor::visit(LabelAttributeExpression *expr) { +void PropertyTrackerVisitor::visit(LabelAttributeExpression *expr) { UNUSED(expr); } -void DeduceMatchPropsVisitor::visit(ConstantExpression *expr) { +void PropertyTrackerVisitor::visit(ConstantExpression *expr) { UNUSED(expr); } -void DeduceMatchPropsVisitor::visit(ColumnExpression *expr) { +void PropertyTrackerVisitor::visit(ColumnExpression *expr) { UNUSED(expr); } -void DeduceMatchPropsVisitor::visit(VertexExpression *expr) { +void PropertyTrackerVisitor::visit(VertexExpression *expr) { UNUSED(expr); } -void DeduceMatchPropsVisitor::visit(EdgeExpression *expr) { +void PropertyTrackerVisitor::visit(EdgeExpression *expr) { UNUSED(expr); } diff --git a/src/graph/visitor/DeduceMatchPropsVisitor.h b/src/graph/visitor/PropertyTrackerVisitor.h similarity index 82% rename from src/graph/visitor/DeduceMatchPropsVisitor.h rename to src/graph/visitor/PropertyTrackerVisitor.h index d77c4b90ea9..6300215747e 100644 --- a/src/graph/visitor/DeduceMatchPropsVisitor.h +++ b/src/graph/visitor/PropertyTrackerVisitor.h @@ -3,8 +3,8 @@ * This source code is licensed under Apache 2.0 License. */ -#ifndef GRAPH_VISITOR_DEDUCEMATCHPROPSVISITOR_H -#define GRAPH_VISITOR_DEDUCEMATCHPROPSVISITOR_H +#ifndef GRAPH_VISITOR_PROPERTYTRACKERVISITOR_H +#define GRAPH_VISITOR_PROPERTYTRACKERVISITOR_H #include "common/base/Status.h" #include "common/expression/FunctionCallExpression.h" @@ -21,12 +21,12 @@ namespace graph { class QueryContext; -class DeduceMatchPropsVisitor : public ExprVisitorImpl { +class PropertyTrackerVisitor : public ExprVisitorImpl { public: - DeduceMatchPropsVisitor(const QueryContext* qctx, - GraphSpaceID space, - PropertyTracker& propsUsed, - const std::string& entityAlias); + PropertyTrackerVisitor(const QueryContext* qctx, + GraphSpaceID space, + PropertyTracker& propsUsed, + const std::string& entityAlias); bool ok() const override { return status_.ok(); @@ -73,4 +73,4 @@ class DeduceMatchPropsVisitor : public ExprVisitorImpl { } // namespace graph } // namespace nebula -#endif // GRAPH_VISITOR_DEDUCEMATCHPROPSVISITOR_H +#endif // GRAPH_VISITOR_PROPERTYTRACKERVISITOR_H From 93ec6c5b9811ebccff65077371906d9ddb3bc88a Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Sun, 23 Jan 2022 19:48:10 +0800 Subject: [PATCH 13/27] ingore func id, src, dst, type, typeid, rank, hash in property tracker visitor --- src/graph/visitor/PropertyTrackerVisitor.cpp | 6 ++- .../features/match/MultiQueryParts.feature | 50 +++++++++++++------ 2 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/graph/visitor/PropertyTrackerVisitor.cpp b/src/graph/visitor/PropertyTrackerVisitor.cpp index c36bea1ff9b..b947eed98fc 100644 --- a/src/graph/visitor/PropertyTrackerVisitor.cpp +++ b/src/graph/visitor/PropertyTrackerVisitor.cpp @@ -110,10 +110,14 @@ void PropertyTrackerVisitor::visit(VariablePropertyExpression *expr) { // } void PropertyTrackerVisitor::visit(FunctionCallExpression *expr) { + static const std::unordered_set kIgnoreFuncs = { + "id", "src", "dst", "type", "typeid", "rank", "hash"}; + auto funName = expr->name(); - if (funName == "id" || funName == "src" || funName == "dst") { + if (kIgnoreFuncs.find(funName) != kIgnoreFuncs.end()) { return; } + for (const auto &arg : expr->args()->args()) { arg->accept(this); if (!ok()) { diff --git a/tests/tck/features/match/MultiQueryParts.feature b/tests/tck/features/match/MultiQueryParts.feature index 2a5cda28864..735126fc3ca 100644 --- a/tests/tck/features/match/MultiQueryParts.feature +++ b/tests/tck/features/match/MultiQueryParts.feature @@ -1,13 +1,14 @@ # Copyright (c) 2021 vesoft inc. All rights reserved. # # This source code is licensed under Apache 2.0 License. +@jie Feature: Multi Query Parts Background: Given a graph with space named "nba" Scenario: Multi Path Patterns - When executing query: + When profiling query: """ MATCH (m)-[]-(n), (n)-[]-(l) WHERE id(m)=="Tim Duncan" RETURN m.player.name AS n1, n.player.name AS n2, @@ -26,7 +27,26 @@ Feature: Multi Query Parts | "Tim Duncan" | "Boris Diaw" | "Spurs" | | "Tim Duncan" | "Boris Diaw" | "Suns" | | "Tim Duncan" | "Boris Diaw" | "Tim Duncan" | - When executing query: + When profiling query: + """ + MATCH (m)-[]-(n), (l)-[]-(n) WHERE id(m)=="Tim Duncan" + RETURN m.player.name AS n1, n.player.name AS n2, + CASE WHEN l.team.name is not null THEN l.team.name + WHEN l.player.name is not null THEN l.player.name ELSE "null" END AS n3 ORDER BY n1, n2, n3 LIMIT 10 + """ + Then the result should be, in order: + | n1 | n2 | n3 | + | "Tim Duncan" | "Aron Baynes" | "Celtics" | + | "Tim Duncan" | "Aron Baynes" | "Pistons" | + | "Tim Duncan" | "Aron Baynes" | "Spurs" | + | "Tim Duncan" | "Aron Baynes" | "Tim Duncan" | + | "Tim Duncan" | "Boris Diaw" | "Hawks" | + | "Tim Duncan" | "Boris Diaw" | "Hornets" | + | "Tim Duncan" | "Boris Diaw" | "Jazz" | + | "Tim Duncan" | "Boris Diaw" | "Spurs" | + | "Tim Duncan" | "Boris Diaw" | "Suns" | + | "Tim Duncan" | "Boris Diaw" | "Tim Duncan" | + When profiling query: """ MATCH (m)-[]-(n), (n)-[]-(l) WHERE id(n)=="Tim Duncan" RETURN m.player.name AS n1, n.player.name AS n2, l.player.name AS n3 ORDER BY n1, n2, n3 LIMIT 10 @@ -43,7 +63,7 @@ Feature: Multi Query Parts | "Aron Baynes" | "Tim Duncan" | "Manu Ginobili" | | "Aron Baynes" | "Tim Duncan" | "Manu Ginobili" | | "Aron Baynes" | "Tim Duncan" | "Manu Ginobili" | - When executing query: + When profiling query: """ MATCH (m)-[]-(n), (n)-[]-(l), (l)-[]-(h) WHERE id(m)=="Tim Duncan" RETURN m.player.name AS n1, n.player.name AS n2, l.team.name AS n3, h.player.name AS n4 @@ -70,7 +90,7 @@ Feature: Multi Query Parts Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down. Scenario: Multi Match - When executing query: + When profiling query: """ MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" MATCH (n)-[]-(l) @@ -90,7 +110,7 @@ Feature: Multi Query Parts | "Tim Duncan" | "Boris Diaw" | "Spurs" | | "Tim Duncan" | "Boris Diaw" | "Suns" | | "Tim Duncan" | "Boris Diaw" | "Tim Duncan" | - When executing query: + When profiling query: """ MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" MATCH (n)-[]-(l), (l)-[]-(h) @@ -109,7 +129,7 @@ Feature: Multi Query Parts | "Tim Duncan" | "Aron Baynes" | "Pistons" | "Grant Hill" | | "Tim Duncan" | "Aron Baynes" | "Spurs" | "Aron Baynes" | | "Tim Duncan" | "Aron Baynes" | "Spurs" | "Boris Diaw" | - When executing query: + When profiling query: """ MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" MATCH (n)-[]-(l) @@ -129,7 +149,7 @@ Feature: Multi Query Parts | "Tim Duncan" | "Aron Baynes" | "Pistons" | "Grant Hill" | | "Tim Duncan" | "Aron Baynes" | "Spurs" | "Aron Baynes" | | "Tim Duncan" | "Aron Baynes" | "Spurs" | "Boris Diaw" | - When executing query: + When profiling query: """ MATCH (v:player{name:"Tony Parker"}) WITH v AS a @@ -142,7 +162,7 @@ Feature: Multi Query Parts | "Tim Duncan" | Scenario: Optional Match - When executing query: + When profiling query: """ MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" OPTIONAL MATCH (n)<-[:serve]-(l) @@ -160,8 +180,8 @@ Feature: Multi Query Parts | "Tim Duncan" | "Manu Ginobili" | NULL | | "Tim Duncan" | "Manu Ginobili" | NULL | | "Tim Duncan" | "Manu Ginobili" | NULL | - # Below scenario is not supported for the execution plan has a scan. - When executing query: + # Below scenario is not suppoted for the execution plan has a scan. + When profiling query: """ MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" OPTIONAL MATCH (a)<-[]-(b) @@ -170,7 +190,7 @@ Feature: Multi Query Parts Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down. Scenario: Multi Query Parts - When executing query: + When profiling query: """ MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" WITH n, n.player.name AS n1 ORDER BY n1 LIMIT 10 @@ -191,7 +211,7 @@ Feature: Multi Query Parts | "Boris Diaw" | "Spurs" | | "Boris Diaw" | "Suns" | | "Boris Diaw" | "Tim Duncan" | - When executing query: + When profiling query: """ MATCH (m:player{name:"Tim Duncan"})-[:like]-(n)--() WITH m,count(*) AS lcount @@ -201,7 +221,7 @@ Feature: Multi Query Parts Then the result should be, in order: | scount | lcount | | 19 | 110 | - When executing query: + When profiling query: """ MATCH (m:player{name:"Tim Duncan"})-[:like]-(n)--() WITH m,n @@ -222,14 +242,14 @@ Feature: Multi Query Parts Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down. Scenario: Some Erros - When executing query: + When profiling query: """ MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" WITH n, n.player.name AS n1 ORDER BY n1 LIMIT 10 RETURN m """ Then a SemanticError should be raised at runtime: Alias used but not defined: `m' - When executing query: + When profiling query: """ MATCH (v:player)-[e]-(v:team) RETURN v, e """ From bcae49b1f4fe59ddc908082c5b28482a41ad8932 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Mon, 24 Jan 2022 00:17:54 +0800 Subject: [PATCH 14/27] add Aggregate::pruneProperties and store the alias of id/src/dst... to propsused --- src/graph/planner/plan/Query.cpp | 32 ++++++++++++++ src/graph/planner/plan/Query.h | 8 ++++ src/graph/visitor/PropertyTrackerVisitor.cpp | 42 +++++++++++++++++-- src/graph/visitor/PropertyTrackerVisitor.h | 2 + .../features/match/MultiQueryParts.feature | 16 +++++++ 5 files changed, 96 insertions(+), 4 deletions(-) diff --git a/src/graph/planner/plan/Query.cpp b/src/graph/planner/plan/Query.cpp index 1a4923e935d..de90a9aa5f0 100644 --- a/src/graph/planner/plan/Query.cpp +++ b/src/graph/planner/plan/Query.cpp @@ -545,6 +545,18 @@ PlanNode* Aggregate::clone() const { return newAggregate; } +Status Aggregate::pruneProperties(PropertyTracker& propsUsed, + graph::QueryContext* qctx, + GraphSpaceID spaceID) { + for (auto* groupKey : groupKeys_) { + NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(groupKey, propsUsed, qctx, spaceID)); + } + for (auto* groupItem : groupItems_) { + NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(groupItem, propsUsed, qctx, spaceID)); + } + return depsPruneProperties(propsUsed, qctx, spaceID); +} + void Aggregate::cloneMembers(const Aggregate& agg) { SingleInputNode::cloneMembers(agg); @@ -809,6 +821,10 @@ Status Traverse::pruneProperties(PropertyTracker& propsUsed, } else { auto prunedVertexProps = std::make_unique>(); auto& usedVertexProps = it2->second; + if (usedVertexProps.empty()) { + setVertexProps(nullptr); + return depsPruneProperties(propsUsed, qctx, spaceID); + } prunedVertexProps->reserve(usedVertexProps.size()); for (auto& vertexProp : *vertexProps) { auto tagId = vertexProp.tag_ref().value(); @@ -883,6 +899,10 @@ Status AppendVertices::pruneProperties(PropertyTracker& propsUsed, auto it2 = propsUsed.vertexPropsMap.find(nodeAlias); if (it2 != propsUsed.vertexPropsMap.end()) { auto& usedVertexProps = it2->second; + if (usedVertexProps.empty()) { + // markAsToBeDeleted(); + // return depsPruneProperties(propsUsed, qctx, spaceID); + } prunedVertexProps->reserve(usedVertexProps.size()); for (auto& vertexProp : *vertexProps) { auto tagId = vertexProp.tag_ref().value(); @@ -920,6 +940,18 @@ std::unique_ptr BiJoin::explain() const { return desc; } +Status BiJoin::pruneProperties(PropertyTracker& propsUsed, + graph::QueryContext* qctx, + GraphSpaceID spaceID) { + for (auto* hashKey : hashKeys_) { + NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(hashKey, propsUsed, qctx, spaceID)); + } + for (auto* probeKey : probeKeys_) { + NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(probeKey, propsUsed, qctx, spaceID)); + } + return depsPruneProperties(propsUsed, qctx, spaceID); +} + void BiJoin::cloneMembers(const BiJoin& j) { BinaryInputNode::cloneMembers(j); diff --git a/src/graph/planner/plan/Query.h b/src/graph/planner/plan/Query.h index f8890740789..d40b5593366 100644 --- a/src/graph/planner/plan/Query.h +++ b/src/graph/planner/plan/Query.h @@ -1122,6 +1122,10 @@ class Aggregate final : public SingleInputNode { PlanNode* clone() const override; std::unique_ptr explain() const override; + Status pruneProperties(PropertyTracker& propsUsed, + graph::QueryContext* qctx, + GraphSpaceID spaceID) override; + private: Aggregate(QueryContext* qctx, PlanNode* input, @@ -1601,6 +1605,10 @@ class BiJoin : public BinaryInputNode { std::unique_ptr explain() const override; + Status pruneProperties(PropertyTracker& propsUsed, + graph::QueryContext* qctx, + GraphSpaceID spaceID) override; + protected: BiJoin(QueryContext* qctx, Kind kind, diff --git a/src/graph/visitor/PropertyTrackerVisitor.cpp b/src/graph/visitor/PropertyTrackerVisitor.cpp index b947eed98fc..57687c38148 100644 --- a/src/graph/visitor/PropertyTrackerVisitor.cpp +++ b/src/graph/visitor/PropertyTrackerVisitor.cpp @@ -6,6 +6,7 @@ #include "graph/visitor/PropertyTrackerVisitor.h" #include +#include #include "common/expression/Expression.h" #include "graph/context/QueryContext.h" @@ -110,15 +111,36 @@ void PropertyTrackerVisitor::visit(VariablePropertyExpression *expr) { // } void PropertyTrackerVisitor::visit(FunctionCallExpression *expr) { - static const std::unordered_set kIgnoreFuncs = { - "id", "src", "dst", "type", "typeid", "rank", "hash"}; + static const std::unordered_set kVertexIgnoreFuncs = {"id"}; + static const std::unordered_set kEdgeIgnoreFuncs = { + "src", "dst", "type", "typeid", "rank"}; auto funName = expr->name(); - if (kIgnoreFuncs.find(funName) != kIgnoreFuncs.end()) { + if (kVertexIgnoreFuncs.find(funName) != kVertexIgnoreFuncs.end()) { + DCHECK_EQ(expr->args()->numArgs(), 1); + auto argExpr = expr->args()->args()[0]; + auto nodeAlias = extractColNameFromInputPropOrVarPropExpr(argExpr); + if (!nodeAlias.empty()) { + auto it = propsUsed_.vertexPropsMap.find(nodeAlias); + if (it == propsUsed_.vertexPropsMap.end()) { + propsUsed_.vertexPropsMap[nodeAlias] = {}; + } + } + return; + } else if (kEdgeIgnoreFuncs.find(funName) != kEdgeIgnoreFuncs.end()) { + DCHECK_EQ(expr->args()->numArgs(), 1); + auto argExpr = expr->args()->args()[0]; + auto edgeAlias = extractColNameFromInputPropOrVarPropExpr(argExpr); + if (!edgeAlias.empty()) { + auto it = propsUsed_.edgePropsMap.find(edgeAlias); + if (it == propsUsed_.edgePropsMap.end()) { + propsUsed_.edgePropsMap[edgeAlias] = {}; + } + } return; } - for (const auto &arg : expr->args()->args()) { + for (auto *arg : expr->args()->args()) { arg->accept(this); if (!ok()) { break; @@ -186,5 +208,17 @@ void PropertyTrackerVisitor::visit(EdgeExpression *expr) { UNUSED(expr); } +std::string PropertyTrackerVisitor::extractColNameFromInputPropOrVarPropExpr( + const Expression *expr) { + if (expr->kind() == Expression::Kind::kInputProperty) { + auto *inputPropExpr = static_cast(expr); + return inputPropExpr->prop(); + } else if (expr->kind() == Expression::Kind::kVarProperty) { + auto *varPropExpr = static_cast(expr); + return varPropExpr->prop(); + } + return ""; +} + } // namespace graph } // namespace nebula diff --git a/src/graph/visitor/PropertyTrackerVisitor.h b/src/graph/visitor/PropertyTrackerVisitor.h index 6300215747e..5cc47e20189 100644 --- a/src/graph/visitor/PropertyTrackerVisitor.h +++ b/src/graph/visitor/PropertyTrackerVisitor.h @@ -62,6 +62,8 @@ class PropertyTrackerVisitor : public ExprVisitorImpl { void visit(EdgeExpression* expr) override; void visit(ColumnExpression* expr) override; + std::string extractColNameFromInputPropOrVarPropExpr(const Expression* expr); + const QueryContext* qctx_{nullptr}; GraphSpaceID space_; PropertyTracker& propsUsed_; diff --git a/tests/tck/features/match/MultiQueryParts.feature b/tests/tck/features/match/MultiQueryParts.feature index 735126fc3ca..4321bbdd714 100644 --- a/tests/tck/features/match/MultiQueryParts.feature +++ b/tests/tck/features/match/MultiQueryParts.feature @@ -27,6 +27,22 @@ Feature: Multi Query Parts | "Tim Duncan" | "Boris Diaw" | "Spurs" | | "Tim Duncan" | "Boris Diaw" | "Suns" | | "Tim Duncan" | "Boris Diaw" | "Tim Duncan" | + # And the execution plan should be: + # | id | name | dependencies | operator info | + # | 15 | DataCollect | 16 | | + # | 16 | TopN | 12 | | + # | 12 | Project | 18 | | + # | 18 | Project | 17 | | + # | 17 | Filter | 9 | | + # | 9 | BiInnerJoin | 5, 8 | | + # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 4 | Traverse | 2 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 2 | Dedup | 1 | | + # | 1 | PassThrough | 3 | | + # | 3 | Start | | | + # | 8 | AppendVertices | 7 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | + # | 7 | Traverse | 6 | { "vertexProps": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | + # | 6 | Argument | | | When profiling query: """ MATCH (m)-[]-(n), (l)-[]-(n) WHERE id(m)=="Tim Duncan" From 97071d67662c49a5a34b84df499acb36bbb21ec0 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Mon, 24 Jan 2022 00:31:20 +0800 Subject: [PATCH 15/27] format tck --- .../features/match/MultiQueryParts.feature | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/tests/tck/features/match/MultiQueryParts.feature b/tests/tck/features/match/MultiQueryParts.feature index 4321bbdd714..f7947a89e84 100644 --- a/tests/tck/features/match/MultiQueryParts.feature +++ b/tests/tck/features/match/MultiQueryParts.feature @@ -28,21 +28,21 @@ Feature: Multi Query Parts | "Tim Duncan" | "Boris Diaw" | "Suns" | | "Tim Duncan" | "Boris Diaw" | "Tim Duncan" | # And the execution plan should be: - # | id | name | dependencies | operator info | - # | 15 | DataCollect | 16 | | - # | 16 | TopN | 12 | | - # | 12 | Project | 18 | | - # | 18 | Project | 17 | | - # | 17 | Filter | 9 | | - # | 9 | BiInnerJoin | 5, 8 | | - # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | - # | 4 | Traverse | 2 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | - # | 2 | Dedup | 1 | | - # | 1 | PassThrough | 3 | | - # | 3 | Start | | | - # | 8 | AppendVertices | 7 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | - # | 7 | Traverse | 6 | { "vertexProps": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | - # | 6 | Argument | | | + # | id | name | dependencies | operator info | + # | 15 | DataCollect | 16 | | + # | 16 | TopN | 12 | | + # | 12 | Project | 18 | | + # | 18 | Project | 17 | | + # | 17 | Filter | 9 | | + # | 9 | BiInnerJoin | 5, 8 | | + # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 4 | Traverse | 2 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 2 | Dedup | 1 | | + # | 1 | PassThrough | 3 | | + # | 3 | Start | | | + # | 8 | AppendVertices | 7 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | + # | 7 | Traverse | 6 | { "vertexProps": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | + # | 6 | Argument | | | When profiling query: """ MATCH (m)-[]-(n), (l)-[]-(n) WHERE id(m)=="Tim Duncan" From c41e0563ed6206c745f8010a9c57b7c50c18853a Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Mon, 24 Jan 2022 00:39:02 +0800 Subject: [PATCH 16/27] remove some unusedless headers --- src/common/expression/Expression.h | 1 - src/graph/planner/plan/Query.cpp | 1 - src/graph/util/ExpressionUtils.cpp | 2 -- src/graph/visitor/PropertyTrackerVisitor.cpp | 1 - 4 files changed, 5 deletions(-) diff --git a/src/common/expression/Expression.h b/src/common/expression/Expression.h index 4de1d46517c..1fc81c59f9e 100644 --- a/src/common/expression/Expression.h +++ b/src/common/expression/Expression.h @@ -10,7 +10,6 @@ #include "common/base/ObjectPool.h" #include "common/base/Status.h" #include "common/context/ExpressionContext.h" -#include "common/datatypes/Edge.h" #include "common/datatypes/Value.h" namespace nebula { diff --git a/src/graph/planner/plan/Query.cpp b/src/graph/planner/plan/Query.cpp index de90a9aa5f0..94feda83bd0 100644 --- a/src/graph/planner/plan/Query.cpp +++ b/src/graph/planner/plan/Query.cpp @@ -8,7 +8,6 @@ #include #include #include -#include #include #include "graph/util/ExpressionUtils.h" diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp index 6efb38bd064..b69beff1db6 100644 --- a/src/graph/util/ExpressionUtils.cpp +++ b/src/graph/util/ExpressionUtils.cpp @@ -10,12 +10,10 @@ #include #include "common/base/ObjectPool.h" -#include "common/base/Status.h" #include "common/expression/ArithmeticExpression.h" #include "common/expression/Expression.h" #include "common/expression/PropertyExpression.h" #include "common/function/AggFunctionManager.h" -#include "common/thrift/ThriftTypes.h" #include "graph/context/QueryContext.h" #include "graph/context/QueryExpressionContext.h" #include "graph/visitor/FoldConstantExprVisitor.h" diff --git a/src/graph/visitor/PropertyTrackerVisitor.cpp b/src/graph/visitor/PropertyTrackerVisitor.cpp index 57687c38148..4b6bada0d2d 100644 --- a/src/graph/visitor/PropertyTrackerVisitor.cpp +++ b/src/graph/visitor/PropertyTrackerVisitor.cpp @@ -14,7 +14,6 @@ namespace nebula { namespace graph { -// visitor PropertyTrackerVisitor::PropertyTrackerVisitor(const QueryContext *qctx, GraphSpaceID space, PropertyTracker &propsUsed, From b76199604ffa5a48ca6c08eaf209da3c4c20c891 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Mon, 24 Jan 2022 14:19:43 +0800 Subject: [PATCH 17/27] add tck for plan --- .../features/match/MultiQueryParts.feature | 134 +++++++++++++++--- 1 file changed, 117 insertions(+), 17 deletions(-) diff --git a/tests/tck/features/match/MultiQueryParts.feature b/tests/tck/features/match/MultiQueryParts.feature index f7947a89e84..44462124a44 100644 --- a/tests/tck/features/match/MultiQueryParts.feature +++ b/tests/tck/features/match/MultiQueryParts.feature @@ -28,21 +28,21 @@ Feature: Multi Query Parts | "Tim Duncan" | "Boris Diaw" | "Suns" | | "Tim Duncan" | "Boris Diaw" | "Tim Duncan" | # And the execution plan should be: - # | id | name | dependencies | operator info | - # | 15 | DataCollect | 16 | | - # | 16 | TopN | 12 | | - # | 12 | Project | 18 | | - # | 18 | Project | 17 | | - # | 17 | Filter | 9 | | - # | 9 | BiInnerJoin | 5, 8 | | - # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | - # | 4 | Traverse | 2 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | - # | 2 | Dedup | 1 | | - # | 1 | PassThrough | 3 | | - # | 3 | Start | | | - # | 8 | AppendVertices | 7 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | - # | 7 | Traverse | 6 | { "vertexProps": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | - # | 6 | Argument | | | + # | id | name | dependencies | operator info | + # | 15 | DataCollect | 16 | | + # | 16 | TopN | 12 | | + # | 12 | Project | 18 | | + # | 18 | Project | 17 | | + # | 17 | Filter | 9 | | + # | 9 | BiInnerJoin | 5, 8 | | + # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 4 | Traverse | 2 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 2 | Dedup | 1 | | + # | 1 | PassThrough | 3 | | + # | 3 | Start | | | + # | 8 | AppendVertices | 7 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}, {\"tagId\":3,\"props\":[\"name\"]}]" } | + # | 7 | Traverse | 6 | { "vertexProps": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | + # | 6 | Argument | | | When profiling query: """ MATCH (m)-[]-(n), (l)-[]-(n) WHERE id(m)=="Tim Duncan" @@ -97,7 +97,26 @@ Feature: Multi Query Parts | "Tim Duncan" | "Aron Baynes" | "Pistons" | "Grant Hill" | | "Tim Duncan" | "Aron Baynes" | "Spurs" | "Aron Baynes" | | "Tim Duncan" | "Aron Baynes" | "Spurs" | "Boris Diaw" | - # Below scenario is not supported for the execution plan has a scan. + # And the execution plan should be: + # | id | name | dependencies | operator info | + # | 19 | DataCollect | 20 | | + # | 20 | TopN | 23 | | + # | 23 | Project | 21 | | + # | 21 | Filter | 13 | | + # | 13 | BiInnerJoin | 9, 12 | | + # | 9 | BiInnerJoin | 5, 8 | | + # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 4 | Traverse | 2 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 2 | Dedup | 1 | | + # | 1 | PassThrough | 3 | | + # | 3 | Start | | | + # | 8 | AppendVertices | 7 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}, {\"tagId\":3,\"props\":[\"name\"]}]" } | + # | 7 | Traverse | 6 | { "vertexProps": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | + # | 6 | Argument | | | + # | 12 | AppendVertices | 11 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 11 | Traverse | 10 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":3}]" } | + # | 10 | Argument | | | + # Below scenario is not suppoted for the execution plan has a scan. When executing query: """ MATCH (m)-[]-(n), (a)-[]-(c) WHERE id(m)=="Tim Duncan" @@ -126,6 +145,23 @@ Feature: Multi Query Parts | "Tim Duncan" | "Boris Diaw" | "Spurs" | | "Tim Duncan" | "Boris Diaw" | "Suns" | | "Tim Duncan" | "Boris Diaw" | "Tim Duncan" | + # And the execution plan should be: + # | id | name | dependencies | operator info | + # | 16 | DataCollect | 17 | | + # | 17 | TopN | 13 | | + # | 13 | Project | 12 | | + # | 12 | BiInnerJoin | 19, 11 | | + # | 19 | Project | 18 | | + # | 18 | Filter | 5 | | + # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 4 | Traverse | 2 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 2 | Dedup | 1 | | + # | 1 | PassThrough | 3 | | + # | 3 | Start | | | + # | 11 | Project | 10 | | + # | 10 | AppendVertices | 9 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}, {\"tagId\":3,\"props\":[\"name\"]}]" } | + # | 9 | Traverse | 8 | { "vertexProps": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | + # | 8 | Argument | | | When profiling query: """ MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" @@ -145,6 +181,27 @@ Feature: Multi Query Parts | "Tim Duncan" | "Aron Baynes" | "Pistons" | "Grant Hill" | | "Tim Duncan" | "Aron Baynes" | "Spurs" | "Aron Baynes" | | "Tim Duncan" | "Aron Baynes" | "Spurs" | "Boris Diaw" | + # And the execution plan should be: + # | id | name | dependencies | operator info | + # | 20 | DataCollect | 21 | | + # | 21 | TopN | 17 | | + # | 17 | Project | 16 | | + # | 16 | BiInnerJoin | 23, 15 | | + # | 23 | Project | 22 | | + # | 22 | Filter | 5 | | + # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 4 | Traverse | 2 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 2 | Dedup | 1 | | + # | 1 | PassThrough | 3 | | + # | 3 | Start | | | + # | 15 | Project | 14 | | + # | 14 | BiInnerJoin | 10, 13 | | + # | 10 | AppendVertices | 9 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}, {\"tagId\":3,\"props\":[\"name\"]}]" } | + # | 9 | Traverse | 8 | { "vertexProps": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | + # | 8 | Argument | | | + # | 13 | AppendVertices | 12 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}, {\"tagId\":3,\"props\":[\"name\"]}]" } | + # | 12 | Traverse | 11 | { "vertexProps": "[{\"tagId\":3,\"props\":[\"name\"]}]" } | + # | 11 | Argument | | | When profiling query: """ MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" @@ -177,6 +234,18 @@ Feature: Multi Query Parts | "Tim Duncan" | | "Tim Duncan" | + # And the execution plan should be: + # | id | name | dependencies | operator info | + # | 10 | Project | 11 | | + # | 11 | BiInnerJoin | 14, 9 | | + # | 14 | Project | 3 | | + # | 3 | AppendVertices | 12 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 12 | IndexScan | 2 | | + # | 2 | Start | | | + # | 9 | Project | 8 | | + # | 8 | AppendVertices | 7 | { "props": "[{\"props\":[\"name\", \"_tag\"],\"tagId\":3}, {\"props\":[\"name\", \"age\", \"_tag\"],\"tagId\":2}, {\"props\":[\"name\", , \"speciality\", \"_tag\"],\"tagId\":4}, {\"props\":[\"id\", \"ts\", \"_tag\"],\"tagId\":6}]" } | + # | 7 | Traverse | 6 | { "vertexProps": "[{\"props\":[\"name\", \"_tag\"],\"tagId\":3}, {\"props\":[\"name\", \"age\", \"_tag\"],\"tagId\":2}, {\"props\":[\"name\", , \"speciality\", \"_tag\"],\"tagId\":4}, {\"props\":[\"id\", \"ts\", \"_tag\"],\"tagId\":6}]" } | + # | 6 | Argument | | | Scenario: Optional Match When profiling query: """ @@ -196,6 +265,23 @@ Feature: Multi Query Parts | "Tim Duncan" | "Manu Ginobili" | NULL | | "Tim Duncan" | "Manu Ginobili" | NULL | | "Tim Duncan" | "Manu Ginobili" | NULL | + # And the execution plan should be: + # | id | name | dependencies | operator info | + # | 16 | DataCollect | 17 | | + # | 17 | TopN | 13 | | + # | 13 | Project | 12 | | + # | 12 | BiLeftJoin | 19, 11 | | + # | 19 | Project | 18 | | + # | 18 | Filter | 5 | | + # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 4 | Traverse | 2 | | + # | 2 | Dedup | 1 | | + # | 1 | PassThrough | 3 | | + # | 3 | Start | | | + # | 11 | Project | 10 | | + # | 10 | AppendVertices | 9 | { "props": "[{\"props\":[\"name\", \"_tag\"],\"tagId\":3}, {\"props\":[\"name\", \"age\", \"_tag\"],\"tagId\":2}, {\"props\":[\"name\", , \"speciality\", \"_tag\"],\"tagId\":4}, {\"props\":[\"id\", \"ts\", \"_tag\"],\"tagId\":6}]" } | + # | 9 | Traverse | 8 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 8 | Argument | | | # Below scenario is not suppoted for the execution plan has a scan. When profiling query: """ @@ -247,7 +333,21 @@ Feature: Multi Query Parts Then the result should be, in order: | scount | | 270 | - # Below scenario is not supported for the execution plan has a scan. + # And the execution plan should be: + # | id | name | dependencies | operator info | + # | 12 | Aggregate | 13 | | + # | 13 | BiInnerJoin | 15, 11 | | + # | 15 | Project | 5 | | + # | 5 | AppendVertices | 4 | { "props": "[]" } | + # | 4 | Traverse | 3 | { "vertexProps": "" } | + # | 3 | Traverse | 14 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 14 | IndexScan | 2 | | + # | 2 | Start | | | + # | 11 | Project | 10 | | + # | 10 | AppendVertices | 9 | { "props": "[]" } | + # | 9 | Traverse | 8 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 8 | Argument | | | + # Below scenario is not suppoted for the execution plan has a scan. When executing query: """ MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" From ab9f684852dcbbad068a9c29408a774000ac5172 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Mon, 24 Jan 2022 14:47:54 +0800 Subject: [PATCH 18/27] rename has1, has2, has3 --- src/common/expression/Expression.cpp | 17 ++++++++--------- tests/tck/features/yield/NoSpaceChosen.feature | 1 + 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/common/expression/Expression.cpp b/src/common/expression/Expression.cpp index ff7abf5b39c..44414012f19 100644 --- a/src/common/expression/Expression.cpp +++ b/src/common/expression/Expression.cpp @@ -747,24 +747,23 @@ Status PropertyTracker::update(const std::string& oldName, const std::string& ne } auto it1 = vertexPropsMap.find(oldName); - bool has1 = it1 != vertexPropsMap.end(); + bool hasNodeAlias = it1 != vertexPropsMap.end(); auto it2 = edgePropsMap.find(oldName); - bool has2 = it2 != edgePropsMap.end(); - if (has1 && has2) { + bool hasEdgeAlias = it2 != edgePropsMap.end(); + if (hasNodeAlias && hasEdgeAlias) { return Status::Error("Duplicated property name: %s", oldName.c_str()); } - if (has1) { - vertexPropsMap[newName] = vertexPropsMap[oldName]; + if (hasNodeAlias) { + vertexPropsMap[newName] = std::move(it1->second); vertexPropsMap.erase(it1); } - if (has2) { - edgePropsMap[newName] = edgePropsMap[oldName]; + if (hasEdgeAlias) { + edgePropsMap[newName] = std::move(it2->second); edgePropsMap.erase(it2); } auto it3 = colsSet.find(oldName); - bool has3 = it3 != colsSet.end(); - if (has3) { + if (it3 != colsSet.end()) { colsSet.erase(it3); colsSet.insert(newName); } diff --git a/tests/tck/features/yield/NoSpaceChosen.feature b/tests/tck/features/yield/NoSpaceChosen.feature index 3c79382d44f..5a30e0a6c32 100644 --- a/tests/tck/features/yield/NoSpaceChosen.feature +++ b/tests/tck/features/yield/NoSpaceChosen.feature @@ -1,3 +1,4 @@ +@wang Feature: Yield Background: From cde0eb3ac1f0566d09beb6f077f4add37dad004d Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Mon, 7 Feb 2022 16:16:38 +0800 Subject: [PATCH 19/27] add flag_enable_optimizer_property_pruner_rule and support prune edge props... --- src/graph/optimizer/Optimizer.cpp | 9 ++- src/graph/optimizer/Optimizer.h | 2 + .../optimizer/rule/CollapseProjectRule.cpp | 4 +- .../optimizer/rule/CollapseProjectRule.h | 2 +- src/graph/planner/plan/Query.cpp | 75 ++++++++++++++----- src/graph/util/ExpressionUtils.cpp | 10 +-- src/graph/util/ExpressionUtils.h | 10 +-- src/graph/visitor/PropertyTrackerVisitor.cpp | 72 +++++++++--------- src/graph/visitor/PropertyTrackerVisitor.h | 2 +- 9 files changed, 118 insertions(+), 68 deletions(-) diff --git a/src/graph/optimizer/Optimizer.cpp b/src/graph/optimizer/Optimizer.cpp index fb41d873427..fc8288ecf05 100644 --- a/src/graph/optimizer/Optimizer.cpp +++ b/src/graph/optimizer/Optimizer.cpp @@ -20,6 +20,8 @@ using nebula::graph::QueryContext; using nebula::graph::Select; using nebula::graph::SingleDependencyNode; +DEFINE_bool(enable_optimizer_property_pruner_rule, true, ""); + namespace nebula { namespace opt { @@ -57,8 +59,11 @@ StatusOr Optimizer::findBestPlan(QueryContext *qctx) { // } Status Optimizer::postprocess(PlanNode *root, graph::QueryContext *qctx, GraphSpaceID spaceID) { - PropertyTracker propsUsed; - return root->pruneProperties(propsUsed, qctx, spaceID); + if (FLAGS_enable_optimizer_property_pruner_rule) { + PropertyTracker propsUsed; + return root->pruneProperties(propsUsed, qctx, spaceID); + } + return Status::OK(); } StatusOr Optimizer::prepare(OptContext *ctx, PlanNode *root) { diff --git a/src/graph/optimizer/Optimizer.h b/src/graph/optimizer/Optimizer.h index dc2425d672a..46ba4006079 100644 --- a/src/graph/optimizer/Optimizer.h +++ b/src/graph/optimizer/Optimizer.h @@ -10,6 +10,8 @@ #include "common/base/StatusOr.h" #include "common/thrift/ThriftTypes.h" +DECLARE_bool(enable_optimizer_property_pruner_rule); + namespace nebula { namespace graph { class PlanNode; diff --git a/src/graph/optimizer/rule/CollapseProjectRule.cpp b/src/graph/optimizer/rule/CollapseProjectRule.cpp index 8bd59a852c0..209f8eb5195 100644 --- a/src/graph/optimizer/rule/CollapseProjectRule.cpp +++ b/src/graph/optimizer/rule/CollapseProjectRule.cpp @@ -11,7 +11,7 @@ #include "graph/planner/plan/Query.h" #include "graph/util/ExpressionUtils.h" -DEFINE_bool(enable_opt_collapse_project_rule, true, ""); +DEFINE_bool(enable_optimizer_collapse_project_rule, true, ""); using nebula::graph::PlanNode; using nebula::graph::QueryContext; @@ -112,7 +112,7 @@ StatusOr CollapseProjectRule::transform( } bool CollapseProjectRule::match(OptContext* octx, const MatchedResult& matched) const { - if (!FLAGS_enable_opt_collapse_project_rule) { + if (!FLAGS_enable_optimizer_collapse_project_rule) { return false; } return OptRule::match(octx, matched); diff --git a/src/graph/optimizer/rule/CollapseProjectRule.h b/src/graph/optimizer/rule/CollapseProjectRule.h index a50687174cb..50c6bd5e77c 100644 --- a/src/graph/optimizer/rule/CollapseProjectRule.h +++ b/src/graph/optimizer/rule/CollapseProjectRule.h @@ -8,7 +8,7 @@ #include "graph/optimizer/OptRule.h" -DECLARE_bool(enable_opt_collapse_project_rule); +DECLARE_bool(enable_optimizer_collapse_project_rule); namespace nebula { namespace opt { diff --git a/src/graph/planner/plan/Query.cpp b/src/graph/planner/plan/Query.cpp index 94feda83bd0..7c721ee1a5f 100644 --- a/src/graph/planner/plan/Query.cpp +++ b/src/graph/planner/plan/Query.cpp @@ -286,8 +286,7 @@ Status Filter::pruneProperties(PropertyTracker& propsUsed, graph::QueryContext* qctx, GraphSpaceID spaceID) { if (condition_ != nullptr) { - NG_RETURN_IF_ERROR( - ExpressionUtils::extractPropsFromExprs(condition_, propsUsed, qctx, spaceID)); + NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExpr(condition_, propsUsed, qctx, spaceID)); } return depsPruneProperties(propsUsed, qctx, spaceID); @@ -376,7 +375,7 @@ Status Project::pruneProperties(PropertyTracker& propsUsed, auto* col = DCHECK_NOTNULL(columns[i]); auto* expr = col->expr(); auto& alias = colNames[i]; - // First, try to rename alias + // If the alias exists, try to rename alias if (aliasExists[i]) { if (expr->kind() == Expression::Kind::kInputProperty) { auto* inputPropExpr = static_cast(expr); @@ -386,12 +385,14 @@ Status Project::pruneProperties(PropertyTracker& propsUsed, auto* varPropExpr = static_cast(expr); auto& newAlias = varPropExpr->prop(); NG_RETURN_IF_ERROR(propsUsed.update(alias, newAlias)); - } else { + } else { // eg. "PathBuild[$-.x,$-.__VAR_0,$-.y] AS p" // How to handle this case? + propsUsed.colsSet.erase(alias); + NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExpr(expr, propsUsed, qctx, spaceID)); } } else { - // If not, try to extract properties from expression - NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(expr, propsUsed, qctx, spaceID)); + // Otherwise, extract properties from the column expression + NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExpr(expr, propsUsed, qctx, spaceID)); } } } @@ -548,10 +549,10 @@ Status Aggregate::pruneProperties(PropertyTracker& propsUsed, graph::QueryContext* qctx, GraphSpaceID spaceID) { for (auto* groupKey : groupKeys_) { - NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(groupKey, propsUsed, qctx, spaceID)); + NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExpr(groupKey, propsUsed, qctx, spaceID)); } for (auto* groupItem : groupItems_) { - NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(groupItem, propsUsed, qctx, spaceID)); + NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExpr(groupItem, propsUsed, qctx, spaceID)); } return depsPruneProperties(propsUsed, qctx, spaceID); } @@ -800,20 +801,20 @@ Status Traverse::pruneProperties(PropertyTracker& propsUsed, auto& colNames = this->colNames(); DCHECK_GE(colNames.size(), 2); auto& nodeAlias = colNames[colNames.size() - 2]; + auto& edgeAlias = colNames.back(); - auto it = propsUsed.colsSet.find(nodeAlias); - if (it != propsUsed.colsSet.end()) { // All properties are used - // propsUsed.colsSet.erase(it); - return depsPruneProperties(propsUsed, qctx, spaceID); + if (vFilter_ != nullptr) { + NG_RETURN_IF_ERROR( + ExpressionUtils::extractPropsFromExpr(vFilter_, propsUsed, qctx, spaceID, nodeAlias)); } - if (vFilter_ != nullptr) { + if (eFilter_ != nullptr) { NG_RETURN_IF_ERROR( - ExpressionUtils::extractPropsFromExprs(vFilter_, propsUsed, qctx, spaceID, nodeAlias)); + ExpressionUtils::extractPropsFromExpr(eFilter_, propsUsed, qctx, spaceID, edgeAlias)); } auto* vertexProps = this->vertexProps(); - if (vertexProps != nullptr) { + if (propsUsed.colsSet.find(nodeAlias) == propsUsed.colsSet.end() && vertexProps != nullptr) { auto it2 = propsUsed.vertexPropsMap.find(nodeAlias); if (it2 == propsUsed.vertexPropsMap.end()) { // nodeAlias is not used setVertexProps(nullptr); @@ -847,6 +848,44 @@ Status Traverse::pruneProperties(PropertyTracker& propsUsed, } } + static const std::unordered_set reservedEdgeProps = { + nebula::kSrc, nebula::kType, nebula::kRank, nebula::kDst}; + auto* edgeProps = this->edgeProps(); + if (propsUsed.colsSet.find(edgeAlias) == propsUsed.colsSet.end() && edgeProps != nullptr) { + auto prunedEdgeProps = std::make_unique>(); + prunedEdgeProps->reserve(edgeProps->size()); + auto it2 = propsUsed.edgePropsMap.find(edgeAlias); + + for (auto& edgeProp : *edgeProps) { + auto edgeType = edgeProp.type_ref().value(); + auto& props = edgeProp.props_ref().value(); + EdgeProp newEProp; + newEProp.type_ref() = edgeType; + std::vector newProps{reservedEdgeProps.begin(), reservedEdgeProps.end()}; + std::unordered_set usedProps; + if (it2 != propsUsed.edgePropsMap.end()) { + auto& usedEdgeProps = it2->second; + auto it3 = usedEdgeProps.find(std::abs(edgeType)); + if (it3 != usedEdgeProps.end()) { + usedProps = {it3->second.begin(), it3->second.end()}; + } + auto it4 = usedEdgeProps.find(0); + if (it4 != usedEdgeProps.end()) { + usedProps.insert(it4->second.begin(), it4->second.end()); + } + } + for (auto& prop : props) { + if (reservedEdgeProps.find(prop) == reservedEdgeProps.end() && + usedProps.find(prop) != usedProps.end()) { + newProps.emplace_back(prop); + } + } + newEProp.props_ref() = std::move(newProps); + prunedEdgeProps->emplace_back(std::move(newEProp)); + } + setEdgeProps(std::move(prunedEdgeProps)); + } + return depsPruneProperties(propsUsed, qctx, spaceID); } @@ -890,7 +929,7 @@ Status AppendVertices::pruneProperties(PropertyTracker& propsUsed, if (vFilter_ != nullptr) { NG_RETURN_IF_ERROR( - ExpressionUtils::extractPropsFromExprs(vFilter_, propsUsed, qctx, spaceID, nodeAlias)); + ExpressionUtils::extractPropsFromExpr(vFilter_, propsUsed, qctx, spaceID, nodeAlias)); } auto* vertexProps = props(); if (vertexProps != nullptr) { @@ -943,10 +982,10 @@ Status BiJoin::pruneProperties(PropertyTracker& propsUsed, graph::QueryContext* qctx, GraphSpaceID spaceID) { for (auto* hashKey : hashKeys_) { - NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(hashKey, propsUsed, qctx, spaceID)); + NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExpr(hashKey, propsUsed, qctx, spaceID)); } for (auto* probeKey : probeKeys_) { - NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExprs(probeKey, propsUsed, qctx, spaceID)); + NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExpr(probeKey, propsUsed, qctx, spaceID)); } return depsPruneProperties(propsUsed, qctx, spaceID); } diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp index b69beff1db6..c800fd2b323 100644 --- a/src/graph/util/ExpressionUtils.cpp +++ b/src/graph/util/ExpressionUtils.cpp @@ -1059,11 +1059,11 @@ bool ExpressionUtils::isGeoIndexAcceleratedPredicate(const Expression *expr) { return false; } -Status ExpressionUtils::extractPropsFromExprs(const Expression *expr, - PropertyTracker &propsUsed, - const graph::QueryContext *qctx, - GraphSpaceID spaceID, - const std::string &entityAlias) { +Status ExpressionUtils::extractPropsFromExpr(const Expression *expr, + PropertyTracker &propsUsed, + const graph::QueryContext *qctx, + GraphSpaceID spaceID, + const std::string &entityAlias) { PropertyTrackerVisitor visitor(qctx, spaceID, propsUsed, entityAlias); const_cast(expr)->accept(&visitor); if (!visitor.ok()) { diff --git a/src/graph/util/ExpressionUtils.h b/src/graph/util/ExpressionUtils.h index 25f13f7ee0d..b009afe5086 100644 --- a/src/graph/util/ExpressionUtils.h +++ b/src/graph/util/ExpressionUtils.h @@ -170,11 +170,11 @@ class ExpressionUtils { static bool checkExprDepth(const Expression* expr); - static Status extractPropsFromExprs(const Expression* expr, - PropertyTracker& propsUsed, - const graph::QueryContext* qctx, - GraphSpaceID spaceID, - const std::string& entityAlias = ""); + static Status extractPropsFromExpr(const Expression* expr, + PropertyTracker& propsUsed, + const graph::QueryContext* qctx, + GraphSpaceID spaceID, + const std::string& entityAlias = ""); static constexpr int32_t kMaxDepth = 512; }; diff --git a/src/graph/visitor/PropertyTrackerVisitor.cpp b/src/graph/visitor/PropertyTrackerVisitor.cpp index 4b6bada0d2d..d2d9eeb083e 100644 --- a/src/graph/visitor/PropertyTrackerVisitor.cpp +++ b/src/graph/visitor/PropertyTrackerVisitor.cpp @@ -74,40 +74,44 @@ void PropertyTrackerVisitor::visit(VariablePropertyExpression *expr) { propsUsed_.colsSet.emplace(colName); } -// void PropertyTrackerVisitor::visit(AttributeExpression *expr) { -// auto *lhs = expr->left(); -// auto *rhs = expr->right(); -// if (rhs->kind() != Expression::Kind::kConstant) { -// return; -// } -// auto *constExpr = static_cast(rhs); -// auto &propName = constExpr->value(); -// switch (lhs->kind()) { -// case Expression::Kind::kVarProperty: { // $e.name -// auto *varPropExpr = static_cast(lhs); -// auto &edgeAlias = varPropExpr->prop(); -// propsUsed_.edgePropsMap[edgeAlias][0].emplace(propName); -// break; -// } -// case Expression::Kind::kInputProperty: { -// auto *inputPropExpr = static_cast(lhs); -// auto &edgeAlias = inputPropExpr->prop(); -// propsUsed_.edgePropsMap[edgeAlias][0].emplace(propName); -// break; -// } -// case Expression::Kind::kSubscript: { // $-.e[0].name -// auto *subscriptExpr = static_cast(lhs); -// auto *subLeftExpr = subscriptExpr->left(); -// if (subLeftExpr->kind() == Expression::Kind::kInputProperty) { -// auto *inputPropExpr = static_cast(subLeftExpr); -// auto &edgeAlias = inputPropExpr->prop(); -// propsUsed_.edgePropsMap[edgeAlias][0].emplace(propName); -// } -// } -// default: -// break; -// } -// } +void PropertyTrackerVisitor::visit(AttributeExpression *expr) { + auto *lhs = expr->left(); + auto *rhs = expr->right(); + if (rhs->kind() != Expression::Kind::kConstant) { + return; + } + auto *constExpr = static_cast(rhs); + auto &constVal = constExpr->value(); + if (constVal.type() != Value::Type::STRING) { + return; + } + auto &propName = constVal.getStr(); + switch (lhs->kind()) { + case Expression::Kind::kVarProperty: { // $e.name + auto *varPropExpr = static_cast(lhs); + auto &edgeAlias = varPropExpr->prop(); + propsUsed_.edgePropsMap[edgeAlias][0].emplace(propName); + break; + } + case Expression::Kind::kInputProperty: { + auto *inputPropExpr = static_cast(lhs); + auto &edgeAlias = inputPropExpr->prop(); + propsUsed_.edgePropsMap[edgeAlias][0].emplace(propName); + break; + } + case Expression::Kind::kSubscript: { // $-.e[0].name + auto *subscriptExpr = static_cast(lhs); + auto *subLeftExpr = subscriptExpr->left(); + if (subLeftExpr->kind() == Expression::Kind::kInputProperty) { + auto *inputPropExpr = static_cast(subLeftExpr); + auto &edgeAlias = inputPropExpr->prop(); + propsUsed_.edgePropsMap[edgeAlias][0].emplace(propName); + } + } + default: + break; + } +} void PropertyTrackerVisitor::visit(FunctionCallExpression *expr) { static const std::unordered_set kVertexIgnoreFuncs = {"id"}; diff --git a/src/graph/visitor/PropertyTrackerVisitor.h b/src/graph/visitor/PropertyTrackerVisitor.h index 5cc47e20189..43733b8df8c 100644 --- a/src/graph/visitor/PropertyTrackerVisitor.h +++ b/src/graph/visitor/PropertyTrackerVisitor.h @@ -43,7 +43,7 @@ class PropertyTrackerVisitor : public ExprVisitorImpl { void visit(LabelTagPropertyExpression* expr) override; void visit(InputPropertyExpression* expr) override; void visit(VariablePropertyExpression* expr) override; - // void visit(AttributeExpression* expr) override; + void visit(AttributeExpression* expr) override; void visit(FunctionCallExpression* expr) override; void visit(SourcePropertyExpression* expr) override; From cfc0275b60182af28c0143b093c3daa7b3fb7c15 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Thu, 10 Feb 2022 10:01:26 +0800 Subject: [PATCH 20/27] add kUnknownEdgeType --- src/graph/planner/plan/Query.cpp | 3 ++- src/graph/visitor/PropertyTrackerVisitor.cpp | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/graph/planner/plan/Query.cpp b/src/graph/planner/plan/Query.cpp index 7c721ee1a5f..e5f37958982 100644 --- a/src/graph/planner/plan/Query.cpp +++ b/src/graph/planner/plan/Query.cpp @@ -869,7 +869,8 @@ Status Traverse::pruneProperties(PropertyTracker& propsUsed, if (it3 != usedEdgeProps.end()) { usedProps = {it3->second.begin(), it3->second.end()}; } - auto it4 = usedEdgeProps.find(0); + static const int kUnknownEdgeType = 0; + auto it4 = usedEdgeProps.find(kUnknownEdgeType); if (it4 != usedEdgeProps.end()) { usedProps.insert(it4->second.begin(), it4->second.end()); } diff --git a/src/graph/visitor/PropertyTrackerVisitor.cpp b/src/graph/visitor/PropertyTrackerVisitor.cpp index d2d9eeb083e..d0d1126a71a 100644 --- a/src/graph/visitor/PropertyTrackerVisitor.cpp +++ b/src/graph/visitor/PropertyTrackerVisitor.cpp @@ -86,17 +86,18 @@ void PropertyTrackerVisitor::visit(AttributeExpression *expr) { return; } auto &propName = constVal.getStr(); + static const int kUnknownEdgeType = 0; switch (lhs->kind()) { case Expression::Kind::kVarProperty: { // $e.name auto *varPropExpr = static_cast(lhs); auto &edgeAlias = varPropExpr->prop(); - propsUsed_.edgePropsMap[edgeAlias][0].emplace(propName); + propsUsed_.edgePropsMap[edgeAlias][kUnknownEdgeType].emplace(propName); break; } case Expression::Kind::kInputProperty: { auto *inputPropExpr = static_cast(lhs); auto &edgeAlias = inputPropExpr->prop(); - propsUsed_.edgePropsMap[edgeAlias][0].emplace(propName); + propsUsed_.edgePropsMap[edgeAlias][kUnknownEdgeType].emplace(propName); break; } case Expression::Kind::kSubscript: { // $-.e[0].name @@ -105,7 +106,7 @@ void PropertyTrackerVisitor::visit(AttributeExpression *expr) { if (subLeftExpr->kind() == Expression::Kind::kInputProperty) { auto *inputPropExpr = static_cast(subLeftExpr); auto &edgeAlias = inputPropExpr->prop(); - propsUsed_.edgePropsMap[edgeAlias][0].emplace(propName); + propsUsed_.edgePropsMap[edgeAlias][kUnknownEdgeType].emplace(propName); } } default: From 72b0c228c668dd28557c08ea0b5948c27dc47311 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Thu, 10 Feb 2022 16:03:32 +0800 Subject: [PATCH 21/27] add PrunePropertiesVisitor --- src/daemons/CMakeLists.txt | 2 + src/graph/optimizer/Optimizer.cpp | 7 +- src/graph/optimizer/test/CMakeLists.txt | 1 + src/graph/planner/plan/PlanNode.cpp | 5 + src/graph/planner/plan/PlanNode.h | 4 + src/graph/planner/plan/PlanNodeVisitor.h | 31 ++ src/graph/planner/plan/Query.cpp | 25 ++ src/graph/planner/plan/Query.h | 12 + src/graph/visitor/CMakeLists.txt | 5 + src/graph/visitor/PrunePropertiesVisitor.cpp | 288 +++++++++++++++++++ src/graph/visitor/PrunePropertiesVisitor.h | 51 ++++ 11 files changed, 430 insertions(+), 1 deletion(-) create mode 100644 src/graph/planner/plan/PlanNodeVisitor.h create mode 100644 src/graph/visitor/PrunePropertiesVisitor.cpp create mode 100644 src/graph/visitor/PrunePropertiesVisitor.h diff --git a/src/daemons/CMakeLists.txt b/src/daemons/CMakeLists.txt index 2600e7dd3d5..04f6cafd646 100644 --- a/src/daemons/CMakeLists.txt +++ b/src/daemons/CMakeLists.txt @@ -128,6 +128,7 @@ nebula_add_executable( $ $ $ + $ $ $ $ @@ -211,6 +212,7 @@ nebula_add_executable( $ $ $ + $ $ $ $ diff --git a/src/graph/optimizer/Optimizer.cpp b/src/graph/optimizer/Optimizer.cpp index fc8288ecf05..79abdac90d1 100644 --- a/src/graph/optimizer/Optimizer.cpp +++ b/src/graph/optimizer/Optimizer.cpp @@ -12,6 +12,7 @@ #include "graph/planner/plan/ExecutionPlan.h" #include "graph/planner/plan/Logic.h" #include "graph/planner/plan/PlanNode.h" +#include "graph/visitor/PrunePropertiesVisitor.h" using nebula::graph::BinaryInputNode; using nebula::graph::Loop; @@ -61,7 +62,11 @@ StatusOr Optimizer::findBestPlan(QueryContext *qctx) { Status Optimizer::postprocess(PlanNode *root, graph::QueryContext *qctx, GraphSpaceID spaceID) { if (FLAGS_enable_optimizer_property_pruner_rule) { PropertyTracker propsUsed; - return root->pruneProperties(propsUsed, qctx, spaceID); + graph::PrunePropertiesVisitor visitor(propsUsed, qctx, spaceID); + root->accept(&visitor); + if (!visitor.ok()) { + return visitor.status(); + } } return Status::OK(); } diff --git a/src/graph/optimizer/test/CMakeLists.txt b/src/graph/optimizer/test/CMakeLists.txt index f4fd79ff81f..f53898b4b9f 100644 --- a/src/graph/optimizer/test/CMakeLists.txt +++ b/src/graph/optimizer/test/CMakeLists.txt @@ -43,6 +43,7 @@ set(OPTIMIZER_TEST_LIB $ $ $ + $ $ $ $ diff --git a/src/graph/planner/plan/PlanNode.cpp b/src/graph/planner/plan/PlanNode.cpp index ac1657235db..42b2838d391 100644 --- a/src/graph/planner/plan/PlanNode.cpp +++ b/src/graph/planner/plan/PlanNode.cpp @@ -13,6 +13,7 @@ #include "common/graph/Response.h" #include "graph/context/QueryContext.h" +#include "graph/planner/plan/PlanNodeVisitor.h" #include "graph/util/ToJson.h" namespace nebula { @@ -360,6 +361,10 @@ std::unique_ptr PlanNode::explain() const { return desc; } +void PlanNode::accept(PlanNodeVisitor* visitor) { + visitor->visit(this); +} + void PlanNode::releaseSymbols() { auto symTbl = qctx_->symTable(); for (auto in : inputVars_) { diff --git a/src/graph/planner/plan/PlanNode.h b/src/graph/planner/plan/PlanNode.h index 6a61415e7a6..e8a757a6672 100644 --- a/src/graph/planner/plan/PlanNode.h +++ b/src/graph/planner/plan/PlanNode.h @@ -14,6 +14,8 @@ namespace nebula { namespace graph { +class PlanNodeVisitor; + /** * PlanNode is an abstraction of nodes in an execution plan which * is a kind of directed cyclic graph. @@ -189,6 +191,8 @@ class PlanNode { // Describe plan node virtual std::unique_ptr explain() const; + virtual void accept(PlanNodeVisitor* visitor); + virtual PlanNode* clone() const = 0; virtual void calcCost(); diff --git a/src/graph/planner/plan/PlanNodeVisitor.h b/src/graph/planner/plan/PlanNodeVisitor.h new file mode 100644 index 00000000000..bd3e9f9ed2e --- /dev/null +++ b/src/graph/planner/plan/PlanNodeVisitor.h @@ -0,0 +1,31 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License. + */ + +#ifndef PLAN_PLANVISITOR_H_ +#define PLAN_PLANVISITOR_H_ + +#include "graph/planner/plan/PlanNode.h" +#include "graph/planner/plan/Query.h" + +namespace nebula { +namespace graph { + +class PlanNodeVisitor { + public: + virtual ~PlanNodeVisitor() = default; + + virtual void visit(PlanNode *node) = 0; + virtual void visit(Filter *node) = 0; + virtual void visit(Project *node) = 0; + virtual void visit(Aggregate *node) = 0; + virtual void visit(Traverse *node) = 0; + virtual void visit(AppendVertices *node) = 0; + virtual void visit(BiJoin *node) = 0; +}; + +} // namespace graph +} // namespace nebula + +#endif // PLAN_PLANVISITOR_H_ diff --git a/src/graph/planner/plan/Query.cpp b/src/graph/planner/plan/Query.cpp index e5f37958982..ae56848ab32 100644 --- a/src/graph/planner/plan/Query.cpp +++ b/src/graph/planner/plan/Query.cpp @@ -10,6 +10,7 @@ #include #include +#include "graph/planner/plan/PlanNodeVisitor.h" #include "graph/util/ExpressionUtils.h" #include "graph/util/ToJson.h" @@ -269,6 +270,10 @@ std::unique_ptr Filter::explain() const { return desc; } +void Filter::accept(PlanNodeVisitor* visitor) { + visitor->visit(this); +} + PlanNode* Filter::clone() const { auto* newFilter = Filter::make(qctx_, nullptr); newFilter->cloneMembers(*this); @@ -346,6 +351,10 @@ std::unique_ptr Project::explain() const { return desc; } +void Project::accept(PlanNodeVisitor* visitor) { + visitor->visit(this); +} + PlanNode* Project::clone() const { auto* newProj = Project::make(qctx_, nullptr); newProj->cloneMembers(*this); @@ -539,6 +548,10 @@ std::unique_ptr Aggregate::explain() const { return desc; } +void Aggregate::accept(PlanNodeVisitor* visitor) { + visitor->visit(this); +} + PlanNode* Aggregate::clone() const { auto* newAggregate = Aggregate::make(qctx_, nullptr); newAggregate->cloneMembers(*this); @@ -795,6 +808,10 @@ std::unique_ptr Traverse::explain() const { return desc; } +void Traverse::accept(PlanNodeVisitor* visitor) { + visitor->visit(this); +} + Status Traverse::pruneProperties(PropertyTracker& propsUsed, graph::QueryContext* qctx, GraphSpaceID spaceID) { @@ -916,6 +933,10 @@ std::unique_ptr AppendVertices::explain() const { return desc; } +void AppendVertices::accept(PlanNodeVisitor* visitor) { + visitor->visit(this); +} + Status AppendVertices::pruneProperties(PropertyTracker& propsUsed, graph::QueryContext* qctx, GraphSpaceID spaceID) { @@ -979,6 +1000,10 @@ std::unique_ptr BiJoin::explain() const { return desc; } +void BiJoin::accept(PlanNodeVisitor* visitor) { + visitor->visit(this); +} + Status BiJoin::pruneProperties(PropertyTracker& propsUsed, graph::QueryContext* qctx, GraphSpaceID spaceID) { diff --git a/src/graph/planner/plan/Query.h b/src/graph/planner/plan/Query.h index d40b5593366..7fb77dccdef 100644 --- a/src/graph/planner/plan/Query.h +++ b/src/graph/planner/plan/Query.h @@ -730,6 +730,8 @@ class Filter final : public SingleInputNode { PlanNode* clone() const override; std::unique_ptr explain() const override; + void accept(PlanNodeVisitor* visitor) override; + Status pruneProperties(PropertyTracker& propsUsed, graph::QueryContext* qctx, GraphSpaceID spaceID) override; @@ -830,6 +832,8 @@ class Project final : public SingleInputNode { PlanNode* clone() const override; std::unique_ptr explain() const override; + void accept(PlanNodeVisitor* visitor) override; + Status pruneProperties(PropertyTracker& propsUsed, graph::QueryContext* qctx, GraphSpaceID spaceID) override; @@ -1122,6 +1126,8 @@ class Aggregate final : public SingleInputNode { PlanNode* clone() const override; std::unique_ptr explain() const override; + void accept(PlanNodeVisitor* visitor) override; + Status pruneProperties(PropertyTracker& propsUsed, graph::QueryContext* qctx, GraphSpaceID spaceID) override; @@ -1481,6 +1487,8 @@ class Traverse final : public GetNeighbors { std::unique_ptr explain() const override; + void accept(PlanNodeVisitor* visitor) override; + Traverse* clone() const override; MatchStepRange* stepRange() const { @@ -1542,6 +1550,8 @@ class AppendVertices final : public GetVertices { std::unique_ptr explain() const override; + void accept(PlanNodeVisitor* visitor) override; + AppendVertices* clone() const override; Expression* vFilter() const { @@ -1605,6 +1615,8 @@ class BiJoin : public BinaryInputNode { std::unique_ptr explain() const override; + void accept(PlanNodeVisitor* visitor) override; + Status pruneProperties(PropertyTracker& propsUsed, graph::QueryContext* qctx, GraphSpaceID spaceID) override; diff --git a/src/graph/visitor/CMakeLists.txt b/src/graph/visitor/CMakeLists.txt index 653d0c5e711..77c38ebf19a 100644 --- a/src/graph/visitor/CMakeLists.txt +++ b/src/graph/visitor/CMakeLists.txt @@ -18,4 +18,9 @@ nebula_add_library( EvaluableExprVisitor.cpp ) +nebula_add_library( + plan_node_visitor_obj OBJECT + PrunePropertiesVisitor.cpp +) + nebula_add_subdirectory(test) diff --git a/src/graph/visitor/PrunePropertiesVisitor.cpp b/src/graph/visitor/PrunePropertiesVisitor.cpp new file mode 100644 index 00000000000..a223ef17ef4 --- /dev/null +++ b/src/graph/visitor/PrunePropertiesVisitor.cpp @@ -0,0 +1,288 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License. + */ + +#include "graph/visitor/PrunePropertiesVisitor.h" + +#include "graph/visitor/PropertyTrackerVisitor.h" + +namespace nebula { +namespace graph { + +PrunePropertiesVisitor::PrunePropertiesVisitor(PropertyTracker &propsUsed, + graph::QueryContext *qctx, + GraphSpaceID spaceID) + : propsUsed_(propsUsed), qctx_(qctx), spaceID_(spaceID) { + DCHECK(qctx_ != nullptr); +} + +void PrunePropertiesVisitor::visit(PlanNode *node) { + status_ = depsPruneProperties(node->dependencies()); +} + +void PrunePropertiesVisitor::visit(Filter *node) { + if (node->condition() != nullptr) { + status_ = extractPropsFromExpr(node->condition()); + if (!status_.ok()) { + return; + } + } + + status_ = depsPruneProperties(node->dependencies()); +} + +void PrunePropertiesVisitor::visit(Project *node) { + if (node->columns()) { + const auto &columns = node->columns()->columns(); + auto &colNames = node->colNames(); + std::vector aliasExists(colNames.size(), false); + for (size_t i = 0; i < columns.size(); ++i) { + aliasExists[i] = propsUsed_.hasAlias(colNames[i]); + } + for (size_t i = 0; i < columns.size(); ++i) { + auto *col = DCHECK_NOTNULL(columns[i]); + auto *expr = col->expr(); + auto &alias = colNames[i]; + // If the alias exists, try to rename alias + if (aliasExists[i]) { + if (expr->kind() == Expression::Kind::kInputProperty) { + auto *inputPropExpr = static_cast(expr); + auto &newAlias = inputPropExpr->prop(); + status_ = propsUsed_.update(alias, newAlias); + if (!status_.ok()) { + return; + } + } else if (expr->kind() == Expression::Kind::kVarProperty) { + auto *varPropExpr = static_cast(expr); + auto &newAlias = varPropExpr->prop(); + status_ = propsUsed_.update(alias, newAlias); + if (!status_.ok()) { + return; + } + } else { // eg. "PathBuild[$-.x,$-.__VAR_0,$-.y] AS p" + // How to handle this case? + propsUsed_.colsSet.erase(alias); + status_ = extractPropsFromExpr(expr); + if (!status_.ok()) { + return; + } + } + } else { + // Otherwise, extract properties from the column expression + status_ = extractPropsFromExpr(expr); + if (!status_.ok()) { + return; + } + } + } + } + + status_ = depsPruneProperties(node->dependencies()); +} + +void PrunePropertiesVisitor::visit(Aggregate *node) { + for (auto *groupKey : node->groupKeys()) { + status_ = extractPropsFromExpr(groupKey); + if (!status_.ok()) { + return; + } + } + for (auto *groupItem : node->groupItems()) { + status_ = extractPropsFromExpr(groupItem); + if (!status_.ok()) { + return; + } + } + status_ = depsPruneProperties(node->dependencies()); +} + +void PrunePropertiesVisitor::visit(Traverse *node) { + auto &colNames = node->colNames(); + DCHECK_GE(colNames.size(), 2); + auto &nodeAlias = colNames[colNames.size() - 2]; + auto &edgeAlias = colNames.back(); + + if (node->vFilter() != nullptr) { + status_ = extractPropsFromExpr(node->vFilter(), nodeAlias); + if (!status_.ok()) { + return; + } + } + + if (node->eFilter() != nullptr) { + status_ = extractPropsFromExpr(node->eFilter(), edgeAlias); + if (!status_.ok()) { + return; + } + } + + auto *vertexProps = node->vertexProps(); + if (propsUsed_.colsSet.find(nodeAlias) == propsUsed_.colsSet.end() && vertexProps != nullptr) { + auto it2 = propsUsed_.vertexPropsMap.find(nodeAlias); + if (it2 == propsUsed_.vertexPropsMap.end()) { // nodeAlias is not used + node->setVertexProps(nullptr); + } else { + auto prunedVertexProps = std::make_unique>(); + auto &usedVertexProps = it2->second; + if (usedVertexProps.empty()) { + node->setVertexProps(nullptr); + status_ = depsPruneProperties(node->dependencies()); + return; + } + prunedVertexProps->reserve(usedVertexProps.size()); + for (auto &vertexProp : *vertexProps) { + auto tagId = vertexProp.tag_ref().value(); + auto &props = vertexProp.props_ref().value(); + auto it3 = usedVertexProps.find(tagId); + if (it3 != usedVertexProps.end()) { + auto &usedProps = it3->second; + VertexProp newVProp; + newVProp.tag_ref() = tagId; + std::vector newProps; + for (auto &prop : props) { + if (usedProps.find(prop) != usedProps.end()) { + newProps.emplace_back(prop); + } + } + newVProp.props_ref() = std::move(newProps); + prunedVertexProps->emplace_back(std::move(newVProp)); + } + } + node->setVertexProps(std::move(prunedVertexProps)); + } + } + + static const std::unordered_set reservedEdgeProps = { + nebula::kSrc, nebula::kType, nebula::kRank, nebula::kDst}; + auto *edgeProps = node->edgeProps(); + if (propsUsed_.colsSet.find(edgeAlias) == propsUsed_.colsSet.end() && edgeProps != nullptr) { + auto prunedEdgeProps = std::make_unique>(); + prunedEdgeProps->reserve(edgeProps->size()); + auto it2 = propsUsed_.edgePropsMap.find(edgeAlias); + + for (auto &edgeProp : *edgeProps) { + auto edgeType = edgeProp.type_ref().value(); + auto &props = edgeProp.props_ref().value(); + EdgeProp newEProp; + newEProp.type_ref() = edgeType; + std::vector newProps{reservedEdgeProps.begin(), reservedEdgeProps.end()}; + std::unordered_set usedProps; + if (it2 != propsUsed_.edgePropsMap.end()) { + auto &usedEdgeProps = it2->second; + auto it3 = usedEdgeProps.find(std::abs(edgeType)); + if (it3 != usedEdgeProps.end()) { + usedProps = {it3->second.begin(), it3->second.end()}; + } + static const int kUnknownEdgeType = 0; + auto it4 = usedEdgeProps.find(kUnknownEdgeType); + if (it4 != usedEdgeProps.end()) { + usedProps.insert(it4->second.begin(), it4->second.end()); + } + } + for (auto &prop : props) { + if (reservedEdgeProps.find(prop) == reservedEdgeProps.end() && + usedProps.find(prop) != usedProps.end()) { + newProps.emplace_back(prop); + } + } + newEProp.props_ref() = std::move(newProps); + prunedEdgeProps->emplace_back(std::move(newEProp)); + } + node->setEdgeProps(std::move(prunedEdgeProps)); + } + + status_ = depsPruneProperties(node->dependencies()); +} + +void PrunePropertiesVisitor::visit(AppendVertices *node) { + auto &colNames = node->colNames(); + DCHECK(!colNames.empty()); + auto &nodeAlias = colNames.back(); + auto it = propsUsed_.colsSet.find(nodeAlias); + if (it != propsUsed_.colsSet.end()) { // All properties are used + // propsUsed_.colsSet.erase(it); + status_ = depsPruneProperties(node->dependencies()); + return; + } + + if (node->vFilter() != nullptr) { + status_ = extractPropsFromExpr(node->vFilter(), nodeAlias); + return; + } + auto *vertexProps = node->props(); + if (vertexProps != nullptr) { + auto prunedVertexProps = std::make_unique>(); + auto it2 = propsUsed_.vertexPropsMap.find(nodeAlias); + if (it2 != propsUsed_.vertexPropsMap.end()) { + auto &usedVertexProps = it2->second; + if (usedVertexProps.empty()) { + // markAsToBeDeleted(); + // return depsPruneProperties(propsUsed_, qctx_, spaceID_); + } + prunedVertexProps->reserve(usedVertexProps.size()); + for (auto &vertexProp : *vertexProps) { + auto tagId = vertexProp.tag_ref().value(); + auto &props = vertexProp.props_ref().value(); + auto it3 = usedVertexProps.find(tagId); + if (it3 != usedVertexProps.end()) { + auto &usedProps = it3->second; + VertexProp newVProp; + newVProp.tag_ref() = tagId; + std::vector newProps; + for (auto &prop : props) { + if (usedProps.find(prop) != usedProps.end()) { + newProps.emplace_back(prop); + } + } + newVProp.props_ref() = std::move(newProps); + prunedVertexProps->emplace_back(std::move(newVProp)); + } + } + } else { + // AppendVertices should be deleted when no props are used by the parent node + // markAsToBeDeleted(); + // It could be done by ColumnPruner + } + node->setVertexProps(std::move(prunedVertexProps)); + } + + status_ = depsPruneProperties(node->dependencies()); +} + +void PrunePropertiesVisitor::visit(BiJoin *node) { + for (auto *hashKey : node->hashKeys()) { + status_ = extractPropsFromExpr(hashKey); + if (!status_.ok()) { + return; + } + } + for (auto *probeKey : node->probeKeys()) { + status_ = extractPropsFromExpr(probeKey); + if (!status_.ok()) { + return; + } + } + status_ = depsPruneProperties(node->dependencies()); +} + +Status PrunePropertiesVisitor::depsPruneProperties(std::vector &dependencies) { + for (const auto *dep : dependencies) { + const_cast(dep)->accept(this); + } + return Status::OK(); +} + +Status PrunePropertiesVisitor::extractPropsFromExpr(const Expression *expr, + const std::string &entityAlias) { + PropertyTrackerVisitor visitor(qctx_, spaceID_, propsUsed_, entityAlias); + const_cast(expr)->accept(&visitor); + if (!visitor.ok()) { + return visitor.status(); + } + + return Status::OK(); +} + +} // namespace graph +} // namespace nebula diff --git a/src/graph/visitor/PrunePropertiesVisitor.h b/src/graph/visitor/PrunePropertiesVisitor.h new file mode 100644 index 00000000000..9edfc52bae2 --- /dev/null +++ b/src/graph/visitor/PrunePropertiesVisitor.h @@ -0,0 +1,51 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License. + */ + +#ifndef GRAPH_VISITOR_PRUNEPROPERTIES_VISITOR_H_ +#define GRAPH_VISITOR_PRUNEPROPERTIES_VISITOR_H_ + +#include "graph/planner/plan/PlanNode.h" +#include "graph/planner/plan/PlanNodeVisitor.h" +#include "graph/planner/plan/Query.h" + +namespace nebula { +namespace graph { + +class PrunePropertiesVisitor final : public PlanNodeVisitor { + public: + PrunePropertiesVisitor(PropertyTracker &propsUsed, + graph::QueryContext *qctx, + GraphSpaceID spaceID); + + bool ok() const { + return status_.ok(); + } + + const Status &status() const { + return status_; + } + + void visit(PlanNode *node) override; + void visit(Filter *node) override; + void visit(Project *node) override; + void visit(Aggregate *node) override; + void visit(Traverse *node) override; + void visit(AppendVertices *node) override; + void visit(BiJoin *node) override; + + private: + Status depsPruneProperties(std::vector &dependencies); + Status extractPropsFromExpr(const Expression *expr, const std::string &entityAlias = ""); + + PropertyTracker &propsUsed_; + QueryContext *qctx_; + GraphSpaceID spaceID_; + Status status_; +}; + +} // namespace graph +} // namespace nebula + +#endif // GRAPH_VISITOR_PRUNEPROPERTIES_VISITOR_H_ From f60ca32f7f408e701c8e5e4917979f84198c037a Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Thu, 10 Feb 2022 16:19:00 +0800 Subject: [PATCH 22/27] remove PlanNode::pruneProperties, and move PropertyTracker to PropertyTrackerVisitor --- src/common/expression/Expression.cpp | 35 --- src/common/expression/Expression.h | 11 - src/graph/optimizer/Optimizer.cpp | 2 +- src/graph/planner/plan/PlanNode.cpp | 16 -- src/graph/planner/plan/PlanNode.h | 8 - src/graph/planner/plan/Query.cpp | 224 ------------------- src/graph/planner/plan/Query.h | 24 -- src/graph/util/ExpressionUtils.cpp | 14 -- src/graph/util/ExpressionUtils.h | 6 - src/graph/visitor/PropertyTrackerVisitor.cpp | 35 +++ src/graph/visitor/PropertyTrackerVisitor.h | 14 +- src/graph/visitor/PrunePropertiesVisitor.cpp | 2 - src/graph/visitor/PrunePropertiesVisitor.h | 1 + 13 files changed, 48 insertions(+), 344 deletions(-) diff --git a/src/common/expression/Expression.cpp b/src/common/expression/Expression.cpp index 44414012f19..f6cacf218ec 100644 --- a/src/common/expression/Expression.cpp +++ b/src/common/expression/Expression.cpp @@ -741,39 +741,4 @@ std::ostream& operator<<(std::ostream& os, Expression::Kind kind) { return os; } -Status PropertyTracker::update(const std::string& oldName, const std::string& newName) { - if (oldName == newName) { - return Status::OK(); - } - - auto it1 = vertexPropsMap.find(oldName); - bool hasNodeAlias = it1 != vertexPropsMap.end(); - auto it2 = edgePropsMap.find(oldName); - bool hasEdgeAlias = it2 != edgePropsMap.end(); - if (hasNodeAlias && hasEdgeAlias) { - return Status::Error("Duplicated property name: %s", oldName.c_str()); - } - if (hasNodeAlias) { - vertexPropsMap[newName] = std::move(it1->second); - vertexPropsMap.erase(it1); - } - if (hasEdgeAlias) { - edgePropsMap[newName] = std::move(it2->second); - edgePropsMap.erase(it2); - } - - auto it3 = colsSet.find(oldName); - if (it3 != colsSet.end()) { - colsSet.erase(it3); - colsSet.insert(newName); - } - - return Status::OK(); -} - -bool PropertyTracker::hasAlias(const std::string& name) const { - return vertexPropsMap.find(name) != vertexPropsMap.end() || - edgePropsMap.find(name) != edgePropsMap.end() || colsSet.find(name) != colsSet.end(); -} - } // namespace nebula diff --git a/src/common/expression/Expression.h b/src/common/expression/Expression.h index 1fc81c59f9e..23dd93ffe29 100644 --- a/src/common/expression/Expression.h +++ b/src/common/expression/Expression.h @@ -224,17 +224,6 @@ class Expression { std::ostream& operator<<(std::ostream& os, Expression::Kind kind); -struct PropertyTracker { - std::unordered_map>> - vertexPropsMap; - std::unordered_map>> - edgePropsMap; - std::unordered_set colsSet; - - Status update(const std::string& oldName, const std::string& newName); - bool hasAlias(const std::string& name) const; -}; - } // namespace nebula #endif // COMMON_EXPRESSION_EXPRESSION_H_ diff --git a/src/graph/optimizer/Optimizer.cpp b/src/graph/optimizer/Optimizer.cpp index 79abdac90d1..3dfee9a598f 100644 --- a/src/graph/optimizer/Optimizer.cpp +++ b/src/graph/optimizer/Optimizer.cpp @@ -61,7 +61,7 @@ StatusOr Optimizer::findBestPlan(QueryContext *qctx) { Status Optimizer::postprocess(PlanNode *root, graph::QueryContext *qctx, GraphSpaceID spaceID) { if (FLAGS_enable_optimizer_property_pruner_rule) { - PropertyTracker propsUsed; + graph::PropertyTracker propsUsed; graph::PrunePropertiesVisitor visitor(propsUsed, qctx, spaceID); root->accept(&visitor); if (!visitor.ok()) { diff --git a/src/graph/planner/plan/PlanNode.cpp b/src/graph/planner/plan/PlanNode.cpp index 42b2838d391..22fb40e7433 100644 --- a/src/graph/planner/plan/PlanNode.cpp +++ b/src/graph/planner/plan/PlanNode.cpp @@ -384,22 +384,6 @@ void PlanNode::updateSymbols() { } } -Status PlanNode::pruneProperties(PropertyTracker& propsUsed, - graph::QueryContext* qctx, - GraphSpaceID spaceID) { - NG_RETURN_IF_ERROR(depsPruneProperties(propsUsed, qctx, spaceID)); - return Status::OK(); -} - -Status PlanNode::depsPruneProperties(PropertyTracker& propsUsed, - graph::QueryContext* qctx, - GraphSpaceID spaceID) { - for (const auto* dep : dependencies_) { - NG_RETURN_IF_ERROR(const_cast(dep)->pruneProperties(propsUsed, qctx, spaceID)); - } - return Status::OK(); -} - std::ostream& operator<<(std::ostream& os, PlanNode::Kind kind) { os << PlanNode::toString(kind); return os; diff --git a/src/graph/planner/plan/PlanNode.h b/src/graph/planner/plan/PlanNode.h index e8a757a6672..d4bea302938 100644 --- a/src/graph/planner/plan/PlanNode.h +++ b/src/graph/planner/plan/PlanNode.h @@ -293,19 +293,11 @@ class PlanNode { return static_cast(this); } - virtual Status pruneProperties(PropertyTracker& propsUsed, - graph::QueryContext* qctx, - GraphSpaceID spaceID); - protected: PlanNode(QueryContext* qctx, Kind kind); virtual ~PlanNode() = default; - Status depsPruneProperties(PropertyTracker& propsUsed, - graph::QueryContext* qctx, - GraphSpaceID spaceID); - static void addDescription(std::string key, std::string value, PlanNodeDescription* desc); void readVariable(const std::string& varname); diff --git a/src/graph/planner/plan/Query.cpp b/src/graph/planner/plan/Query.cpp index ae56848ab32..0c1177047f2 100644 --- a/src/graph/planner/plan/Query.cpp +++ b/src/graph/planner/plan/Query.cpp @@ -287,16 +287,6 @@ void Filter::cloneMembers(const Filter& f) { needStableFilter_ = f.needStableFilter(); } -Status Filter::pruneProperties(PropertyTracker& propsUsed, - graph::QueryContext* qctx, - GraphSpaceID spaceID) { - if (condition_ != nullptr) { - NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExpr(condition_, propsUsed, qctx, spaceID)); - } - - return depsPruneProperties(propsUsed, qctx, spaceID); -} - void SetOp::cloneMembers(const SetOp& s) { BinaryInputNode::cloneMembers(s); } @@ -370,45 +360,6 @@ void Project::cloneMembers(const Project& p) { } } -Status Project::pruneProperties(PropertyTracker& propsUsed, - graph::QueryContext* qctx, - GraphSpaceID spaceID) { - if (cols_) { - const auto& columns = cols_->columns(); - auto& colNames = this->colNames(); - std::vector aliasExists(colNames.size(), false); - for (size_t i = 0; i < columns.size(); ++i) { - aliasExists[i] = propsUsed.hasAlias(colNames[i]); - } - for (size_t i = 0; i < columns.size(); ++i) { - auto* col = DCHECK_NOTNULL(columns[i]); - auto* expr = col->expr(); - auto& alias = colNames[i]; - // If the alias exists, try to rename alias - if (aliasExists[i]) { - if (expr->kind() == Expression::Kind::kInputProperty) { - auto* inputPropExpr = static_cast(expr); - auto& newAlias = inputPropExpr->prop(); - NG_RETURN_IF_ERROR(propsUsed.update(alias, newAlias)); - } else if (expr->kind() == Expression::Kind::kVarProperty) { - auto* varPropExpr = static_cast(expr); - auto& newAlias = varPropExpr->prop(); - NG_RETURN_IF_ERROR(propsUsed.update(alias, newAlias)); - } else { // eg. "PathBuild[$-.x,$-.__VAR_0,$-.y] AS p" - // How to handle this case? - propsUsed.colsSet.erase(alias); - NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExpr(expr, propsUsed, qctx, spaceID)); - } - } else { - // Otherwise, extract properties from the column expression - NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExpr(expr, propsUsed, qctx, spaceID)); - } - } - } - - return depsPruneProperties(propsUsed, qctx, spaceID); -} - std::unique_ptr Unwind::explain() const { auto desc = SingleInputNode::explain(); addDescription("alias", alias(), desc.get()); @@ -558,18 +509,6 @@ PlanNode* Aggregate::clone() const { return newAggregate; } -Status Aggregate::pruneProperties(PropertyTracker& propsUsed, - graph::QueryContext* qctx, - GraphSpaceID spaceID) { - for (auto* groupKey : groupKeys_) { - NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExpr(groupKey, propsUsed, qctx, spaceID)); - } - for (auto* groupItem : groupItems_) { - NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExpr(groupItem, propsUsed, qctx, spaceID)); - } - return depsPruneProperties(propsUsed, qctx, spaceID); -} - void Aggregate::cloneMembers(const Aggregate& agg) { SingleInputNode::cloneMembers(agg); @@ -812,101 +751,6 @@ void Traverse::accept(PlanNodeVisitor* visitor) { visitor->visit(this); } -Status Traverse::pruneProperties(PropertyTracker& propsUsed, - graph::QueryContext* qctx, - GraphSpaceID spaceID) { - auto& colNames = this->colNames(); - DCHECK_GE(colNames.size(), 2); - auto& nodeAlias = colNames[colNames.size() - 2]; - auto& edgeAlias = colNames.back(); - - if (vFilter_ != nullptr) { - NG_RETURN_IF_ERROR( - ExpressionUtils::extractPropsFromExpr(vFilter_, propsUsed, qctx, spaceID, nodeAlias)); - } - - if (eFilter_ != nullptr) { - NG_RETURN_IF_ERROR( - ExpressionUtils::extractPropsFromExpr(eFilter_, propsUsed, qctx, spaceID, edgeAlias)); - } - - auto* vertexProps = this->vertexProps(); - if (propsUsed.colsSet.find(nodeAlias) == propsUsed.colsSet.end() && vertexProps != nullptr) { - auto it2 = propsUsed.vertexPropsMap.find(nodeAlias); - if (it2 == propsUsed.vertexPropsMap.end()) { // nodeAlias is not used - setVertexProps(nullptr); - } else { - auto prunedVertexProps = std::make_unique>(); - auto& usedVertexProps = it2->second; - if (usedVertexProps.empty()) { - setVertexProps(nullptr); - return depsPruneProperties(propsUsed, qctx, spaceID); - } - prunedVertexProps->reserve(usedVertexProps.size()); - for (auto& vertexProp : *vertexProps) { - auto tagId = vertexProp.tag_ref().value(); - auto& props = vertexProp.props_ref().value(); - auto it3 = usedVertexProps.find(tagId); - if (it3 != usedVertexProps.end()) { - auto& usedProps = it3->second; - VertexProp newVProp; - newVProp.tag_ref() = tagId; - std::vector newProps; - for (auto& prop : props) { - if (usedProps.find(prop) != usedProps.end()) { - newProps.emplace_back(prop); - } - } - newVProp.props_ref() = std::move(newProps); - prunedVertexProps->emplace_back(std::move(newVProp)); - } - } - setVertexProps(std::move(prunedVertexProps)); - } - } - - static const std::unordered_set reservedEdgeProps = { - nebula::kSrc, nebula::kType, nebula::kRank, nebula::kDst}; - auto* edgeProps = this->edgeProps(); - if (propsUsed.colsSet.find(edgeAlias) == propsUsed.colsSet.end() && edgeProps != nullptr) { - auto prunedEdgeProps = std::make_unique>(); - prunedEdgeProps->reserve(edgeProps->size()); - auto it2 = propsUsed.edgePropsMap.find(edgeAlias); - - for (auto& edgeProp : *edgeProps) { - auto edgeType = edgeProp.type_ref().value(); - auto& props = edgeProp.props_ref().value(); - EdgeProp newEProp; - newEProp.type_ref() = edgeType; - std::vector newProps{reservedEdgeProps.begin(), reservedEdgeProps.end()}; - std::unordered_set usedProps; - if (it2 != propsUsed.edgePropsMap.end()) { - auto& usedEdgeProps = it2->second; - auto it3 = usedEdgeProps.find(std::abs(edgeType)); - if (it3 != usedEdgeProps.end()) { - usedProps = {it3->second.begin(), it3->second.end()}; - } - static const int kUnknownEdgeType = 0; - auto it4 = usedEdgeProps.find(kUnknownEdgeType); - if (it4 != usedEdgeProps.end()) { - usedProps.insert(it4->second.begin(), it4->second.end()); - } - } - for (auto& prop : props) { - if (reservedEdgeProps.find(prop) == reservedEdgeProps.end() && - usedProps.find(prop) != usedProps.end()) { - newProps.emplace_back(prop); - } - } - newEProp.props_ref() = std::move(newProps); - prunedEdgeProps->emplace_back(std::move(newEProp)); - } - setEdgeProps(std::move(prunedEdgeProps)); - } - - return depsPruneProperties(propsUsed, qctx, spaceID); -} - AppendVertices* AppendVertices::clone() const { auto newAV = AppendVertices::make(qctx_, nullptr, space_); newAV->cloneMembers(*this); @@ -937,62 +781,6 @@ void AppendVertices::accept(PlanNodeVisitor* visitor) { visitor->visit(this); } -Status AppendVertices::pruneProperties(PropertyTracker& propsUsed, - graph::QueryContext* qctx, - GraphSpaceID spaceID) { - auto& colNames = this->colNames(); - DCHECK(!colNames.empty()); - auto& nodeAlias = colNames.back(); - auto it = propsUsed.colsSet.find(nodeAlias); - if (it != propsUsed.colsSet.end()) { // All properties are used - // propsUsed.colsSet.erase(it); - return depsPruneProperties(propsUsed, qctx, spaceID); - } - - if (vFilter_ != nullptr) { - NG_RETURN_IF_ERROR( - ExpressionUtils::extractPropsFromExpr(vFilter_, propsUsed, qctx, spaceID, nodeAlias)); - } - auto* vertexProps = props(); - if (vertexProps != nullptr) { - auto prunedVertexProps = std::make_unique>(); - auto it2 = propsUsed.vertexPropsMap.find(nodeAlias); - if (it2 != propsUsed.vertexPropsMap.end()) { - auto& usedVertexProps = it2->second; - if (usedVertexProps.empty()) { - // markAsToBeDeleted(); - // return depsPruneProperties(propsUsed, qctx, spaceID); - } - prunedVertexProps->reserve(usedVertexProps.size()); - for (auto& vertexProp : *vertexProps) { - auto tagId = vertexProp.tag_ref().value(); - auto& props = vertexProp.props_ref().value(); - auto it3 = usedVertexProps.find(tagId); - if (it3 != usedVertexProps.end()) { - auto& usedProps = it3->second; - VertexProp newVProp; - newVProp.tag_ref() = tagId; - std::vector newProps; - for (auto& prop : props) { - if (usedProps.find(prop) != usedProps.end()) { - newProps.emplace_back(prop); - } - } - newVProp.props_ref() = std::move(newProps); - prunedVertexProps->emplace_back(std::move(newVProp)); - } - } - } else { - // AppendVertices should be deleted when no props are used by the parent node - // markAsToBeDeleted(); - // It could be done by ColumnPruner - } - setVertexProps(std::move(prunedVertexProps)); - } - - return depsPruneProperties(propsUsed, qctx, spaceID); -} - std::unique_ptr BiJoin::explain() const { auto desc = BinaryInputNode::explain(); addDescription("hashKeys", folly::toJson(util::toJson(hashKeys_)), desc.get()); @@ -1004,18 +792,6 @@ void BiJoin::accept(PlanNodeVisitor* visitor) { visitor->visit(this); } -Status BiJoin::pruneProperties(PropertyTracker& propsUsed, - graph::QueryContext* qctx, - GraphSpaceID spaceID) { - for (auto* hashKey : hashKeys_) { - NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExpr(hashKey, propsUsed, qctx, spaceID)); - } - for (auto* probeKey : probeKeys_) { - NG_RETURN_IF_ERROR(ExpressionUtils::extractPropsFromExpr(probeKey, propsUsed, qctx, spaceID)); - } - return depsPruneProperties(propsUsed, qctx, spaceID); -} - void BiJoin::cloneMembers(const BiJoin& j) { BinaryInputNode::cloneMembers(j); diff --git a/src/graph/planner/plan/Query.h b/src/graph/planner/plan/Query.h index 7fb77dccdef..a9e8a99d145 100644 --- a/src/graph/planner/plan/Query.h +++ b/src/graph/planner/plan/Query.h @@ -732,10 +732,6 @@ class Filter final : public SingleInputNode { void accept(PlanNodeVisitor* visitor) override; - Status pruneProperties(PropertyTracker& propsUsed, - graph::QueryContext* qctx, - GraphSpaceID spaceID) override; - private: Filter(QueryContext* qctx, PlanNode* input, Expression* condition, bool needStableFilter); void cloneMembers(const Filter&); @@ -834,10 +830,6 @@ class Project final : public SingleInputNode { void accept(PlanNodeVisitor* visitor) override; - Status pruneProperties(PropertyTracker& propsUsed, - graph::QueryContext* qctx, - GraphSpaceID spaceID) override; - private: Project(QueryContext* qctx, PlanNode* input, YieldColumns* cols); @@ -1128,10 +1120,6 @@ class Aggregate final : public SingleInputNode { void accept(PlanNodeVisitor* visitor) override; - Status pruneProperties(PropertyTracker& propsUsed, - graph::QueryContext* qctx, - GraphSpaceID spaceID) override; - private: Aggregate(QueryContext* qctx, PlanNode* input, @@ -1523,10 +1511,6 @@ class Traverse final : public GetNeighbors { trackPrevPath_ = track; } - Status pruneProperties(PropertyTracker& propsUsed, - graph::QueryContext* qctx, - GraphSpaceID spaceID) override; - private: Traverse(QueryContext* qctx, PlanNode* input, GraphSpaceID space) : GetNeighbors(qctx, Kind::kTraverse, input, space) { @@ -1570,10 +1554,6 @@ class AppendVertices final : public GetVertices { trackPrevPath_ = track; } - Status pruneProperties(PropertyTracker& propsUsed, - graph::QueryContext* qctx, - GraphSpaceID spaceID) override; - private: AppendVertices(QueryContext* qctx, PlanNode* input, GraphSpaceID space) : GetVertices(qctx, @@ -1617,10 +1597,6 @@ class BiJoin : public BinaryInputNode { void accept(PlanNodeVisitor* visitor) override; - Status pruneProperties(PropertyTracker& propsUsed, - graph::QueryContext* qctx, - GraphSpaceID spaceID) override; - protected: BiJoin(QueryContext* qctx, Kind kind, diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp index c800fd2b323..42d57d2d3b2 100644 --- a/src/graph/util/ExpressionUtils.cpp +++ b/src/graph/util/ExpressionUtils.cpp @@ -1059,20 +1059,6 @@ bool ExpressionUtils::isGeoIndexAcceleratedPredicate(const Expression *expr) { return false; } -Status ExpressionUtils::extractPropsFromExpr(const Expression *expr, - PropertyTracker &propsUsed, - const graph::QueryContext *qctx, - GraphSpaceID spaceID, - const std::string &entityAlias) { - PropertyTrackerVisitor visitor(qctx, spaceID, propsUsed, entityAlias); - const_cast(expr)->accept(&visitor); - if (!visitor.ok()) { - return visitor.status(); - } - - return Status::OK(); -} - bool ExpressionUtils::checkExprDepth(const Expression *expr) { std::queue exprQueue; exprQueue.emplace(expr); diff --git a/src/graph/util/ExpressionUtils.h b/src/graph/util/ExpressionUtils.h index b009afe5086..ceba8e99633 100644 --- a/src/graph/util/ExpressionUtils.h +++ b/src/graph/util/ExpressionUtils.h @@ -170,12 +170,6 @@ class ExpressionUtils { static bool checkExprDepth(const Expression* expr); - static Status extractPropsFromExpr(const Expression* expr, - PropertyTracker& propsUsed, - const graph::QueryContext* qctx, - GraphSpaceID spaceID, - const std::string& entityAlias = ""); - static constexpr int32_t kMaxDepth = 512; }; diff --git a/src/graph/visitor/PropertyTrackerVisitor.cpp b/src/graph/visitor/PropertyTrackerVisitor.cpp index d0d1126a71a..99e62836579 100644 --- a/src/graph/visitor/PropertyTrackerVisitor.cpp +++ b/src/graph/visitor/PropertyTrackerVisitor.cpp @@ -14,6 +14,41 @@ namespace nebula { namespace graph { +Status PropertyTracker::update(const std::string &oldName, const std::string &newName) { + if (oldName == newName) { + return Status::OK(); + } + + auto it1 = vertexPropsMap.find(oldName); + bool hasNodeAlias = it1 != vertexPropsMap.end(); + auto it2 = edgePropsMap.find(oldName); + bool hasEdgeAlias = it2 != edgePropsMap.end(); + if (hasNodeAlias && hasEdgeAlias) { + return Status::Error("Duplicated property name: %s", oldName.c_str()); + } + if (hasNodeAlias) { + vertexPropsMap[newName] = std::move(it1->second); + vertexPropsMap.erase(it1); + } + if (hasEdgeAlias) { + edgePropsMap[newName] = std::move(it2->second); + edgePropsMap.erase(it2); + } + + auto it3 = colsSet.find(oldName); + if (it3 != colsSet.end()) { + colsSet.erase(it3); + colsSet.insert(newName); + } + + return Status::OK(); +} + +bool PropertyTracker::hasAlias(const std::string &name) const { + return vertexPropsMap.find(name) != vertexPropsMap.end() || + edgePropsMap.find(name) != edgePropsMap.end() || colsSet.find(name) != colsSet.end(); +} + PropertyTrackerVisitor::PropertyTrackerVisitor(const QueryContext *qctx, GraphSpaceID space, PropertyTracker &propsUsed, diff --git a/src/graph/visitor/PropertyTrackerVisitor.h b/src/graph/visitor/PropertyTrackerVisitor.h index 43733b8df8c..e8750391656 100644 --- a/src/graph/visitor/PropertyTrackerVisitor.h +++ b/src/graph/visitor/PropertyTrackerVisitor.h @@ -7,9 +7,6 @@ #define GRAPH_VISITOR_PROPERTYTRACKERVISITOR_H #include "common/base/Status.h" -#include "common/expression/FunctionCallExpression.h" -#include "common/expression/ListComprehensionExpression.h" -#include "common/expression/SubscriptExpression.h" #include "common/thrift/ThriftTypes.h" #include "graph/visitor/ExprVisitorImpl.h" @@ -21,6 +18,17 @@ namespace graph { class QueryContext; +struct PropertyTracker { + std::unordered_map>> + vertexPropsMap; + std::unordered_map>> + edgePropsMap; + std::unordered_set colsSet; + + Status update(const std::string& oldName, const std::string& newName); + bool hasAlias(const std::string& name) const; +}; + class PropertyTrackerVisitor : public ExprVisitorImpl { public: PropertyTrackerVisitor(const QueryContext* qctx, diff --git a/src/graph/visitor/PrunePropertiesVisitor.cpp b/src/graph/visitor/PrunePropertiesVisitor.cpp index a223ef17ef4..e02c4e81f5c 100644 --- a/src/graph/visitor/PrunePropertiesVisitor.cpp +++ b/src/graph/visitor/PrunePropertiesVisitor.cpp @@ -5,8 +5,6 @@ #include "graph/visitor/PrunePropertiesVisitor.h" -#include "graph/visitor/PropertyTrackerVisitor.h" - namespace nebula { namespace graph { diff --git a/src/graph/visitor/PrunePropertiesVisitor.h b/src/graph/visitor/PrunePropertiesVisitor.h index 9edfc52bae2..a0a04c8ee50 100644 --- a/src/graph/visitor/PrunePropertiesVisitor.h +++ b/src/graph/visitor/PrunePropertiesVisitor.h @@ -9,6 +9,7 @@ #include "graph/planner/plan/PlanNode.h" #include "graph/planner/plan/PlanNodeVisitor.h" #include "graph/planner/plan/Query.h" +#include "graph/visitor/PropertyTrackerVisitor.h" namespace nebula { namespace graph { From 4f80bc0b562141b7c18cb2f93b3b0fea99025418 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Thu, 10 Feb 2022 17:14:20 +0800 Subject: [PATCH 23/27] add PrunePropertiesRule.feature --- .../features/match/MultiQueryParts.feature | 1 - .../optimizer/PrunePropertiesRule.feature | 349 ++++++++++++++++++ 2 files changed, 349 insertions(+), 1 deletion(-) create mode 100644 tests/tck/features/optimizer/PrunePropertiesRule.feature diff --git a/tests/tck/features/match/MultiQueryParts.feature b/tests/tck/features/match/MultiQueryParts.feature index 44462124a44..10654ce2c05 100644 --- a/tests/tck/features/match/MultiQueryParts.feature +++ b/tests/tck/features/match/MultiQueryParts.feature @@ -1,7 +1,6 @@ # Copyright (c) 2021 vesoft inc. All rights reserved. # # This source code is licensed under Apache 2.0 License. -@jie Feature: Multi Query Parts Background: diff --git a/tests/tck/features/optimizer/PrunePropertiesRule.feature b/tests/tck/features/optimizer/PrunePropertiesRule.feature new file mode 100644 index 00000000000..b1ef947521e --- /dev/null +++ b/tests/tck/features/optimizer/PrunePropertiesRule.feature @@ -0,0 +1,349 @@ +# Copyright (c) 2021 vesoft inc. All rights reserved. +# +# This source code is licensed under Apache 2.0 License. +Feature: Prune Properties rule + + Background: + Given a graph with space named "nba" + + Scenario: Single Match + When profiling query: + """ + MATCH p = (v:player{name: "Tony Parker"})-[e:like]-(v2) + RETURN v2.player.age + """ + Then the result should be, in order: + | v2.player.age | + | 29 | + | 41 | + | 36 | + | 33 | + | 33 | + | 42 | + | 42 | + | 32 | + When profiling query: + """ + MATCH p = (v:player{name: "Tony Parker"})-[e:like]-(v2) + RETURN v.player.age + """ + Then the result should be, in order: + | v.player.age | + | 36 | + | 36 | + | 36 | + | 36 | + | 36 | + | 36 | + | 36 | + | 36 | + When profiling query: + """ + MATCH p = (v:player{name: "Tony Parker"})-[e:like]-(v2) + RETURN v + """ + Then the result should be, in order: + | v | + | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | + | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | + | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | + | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | + | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | + | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | + | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | + | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | + When profiling query: + """ + MATCH p = (v:player{name: "Tony Parker"})-[e:like]-(v2) + RETURN v2 + """ + Then the result should be, in order: + | v2 | + | ("Dejounte Murray" :player{age: 29, name: "Dejounte Murray"}) | + | ("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"}) | + | ("Boris Diaw" :player{age: 36, name: "Boris Diaw"}) | + | ("LaMarcus Aldridge" :player{age: 33, name: "LaMarcus Aldridge"}) | + | ("LaMarcus Aldridge" :player{age: 33, name: "LaMarcus Aldridge"}) | + | ("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"}) | + | ("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"}) | + | ("Marco Belinelli" :player{age: 32, name: "Marco Belinelli"}) | + # The rule will not take affect in this case + When profiling query: + """ + MATCH p = (v:player{name: "Tony Parker"})-[e:like]-(v2) + RETURN p + """ + Then the result should be, in order: + | p | + | <("Tony Parker" :player{age: 36, name: "Tony Parker"})<-[:like@0 {likeness: 99}]-("Dejounte Murray" :player{age: 29, name: "Dejounte Murray"})> | + | <("Tony Parker" :player{age: 36, name: "Tony Parker"})-[:like@0 {likeness: 95}]->("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"})> | + | <("Tony Parker" :player{age: 36, name: "Tony Parker"})<-[:like@0 {likeness: 80}]-("Boris Diaw" :player{age: 36, name: "Boris Diaw"})> | + | <("Tony Parker" :player{age: 36, name: "Tony Parker"})<-[:like@0 {likeness: 75}]-("LaMarcus Aldridge" :player{age: 33, name: "LaMarcus Aldridge"})> | + | <("Tony Parker" :player{age: 36, name: "Tony Parker"})-[:like@0 {likeness: 90}]->("LaMarcus Aldridge" :player{age: 33, name: "LaMarcus Aldridge"})> | + | <("Tony Parker" :player{age: 36, name: "Tony Parker"})<-[:like@0 {likeness: 95}]-("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"})> | + | <("Tony Parker" :player{age: 36, name: "Tony Parker"})-[:like@0 {likeness: 95}]->("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"})> | + | <("Tony Parker" :player{age: 36, name: "Tony Parker"})<-[:like@0 {likeness: 50}]-("Marco Belinelli" :player{age: 32, name: "Marco Belinelli"})> | + When profiling query: + """ + MATCH p = (v:player{name: "Tony Parker"})-[e:like]-(v2) + RETURN type(e) + """ + Then the result should be, in order: + | type(e) | + | "like" | + | "like" | + | "like" | + | "like" | + | "like" | + | "like" | + | "like" | + | "like" | + When profiling query: + """ + MATCH (v:player{name: "Tony Parker"})-[:like]-(v2)--(v3) + WITH v3, v3.player.age AS age + RETURN v3, age ORDER BY age LIMIT 3 + """ + Then the result should be, in order: + | v3 | age | + | ("Kyle Anderson" :player{age: 25, name: "Kyle Anderson"}) | 25 | + | ("Damian Lillard" :player{age: 28, name: "Damian Lillard"}) | 28 | + | ("Damian Lillard" :player{age: 28, name: "Damian Lillard"}) | 28 | + + Scenario: Multi Path Patterns + When profiling query: + """ + MATCH (m)-[]-(n), (n)-[]-(l) WHERE id(m)=="Tim Duncan" + RETURN m.player.name AS n1, n.player.name AS n2, + CASE WHEN l.team.name is not null THEN l.team.name + WHEN l.player.name is not null THEN l.player.name ELSE "null" END AS n3 ORDER BY n1, n2, n3 LIMIT 10 + """ + Then the result should be, in order: + | n1 | n2 | n3 | + | "Tim Duncan" | "Aron Baynes" | "Celtics" | + | "Tim Duncan" | "Aron Baynes" | "Pistons" | + | "Tim Duncan" | "Aron Baynes" | "Spurs" | + | "Tim Duncan" | "Aron Baynes" | "Tim Duncan" | + | "Tim Duncan" | "Boris Diaw" | "Hawks" | + | "Tim Duncan" | "Boris Diaw" | "Hornets" | + | "Tim Duncan" | "Boris Diaw" | "Jazz" | + | "Tim Duncan" | "Boris Diaw" | "Spurs" | + | "Tim Duncan" | "Boris Diaw" | "Suns" | + | "Tim Duncan" | "Boris Diaw" | "Tim Duncan" | + # And the execution plan should be: + # | id | name | dependencies | operator info | + # | 15 | DataCollect | 16 | | + # | 16 | TopN | 12 | | + # | 12 | Project | 18 | | + # | 18 | Project | 17 | | + # | 17 | Filter | 9 | | + # | 9 | BiInnerJoin | 5, 8 | | + # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 4 | Traverse | 2 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 2 | Dedup | 1 | | + # | 1 | PassThrough | 3 | | + # | 3 | Start | | | + # | 8 | AppendVertices | 7 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}, {\"tagId\":3,\"props\":[\"name\"]}]" } | + # | 7 | Traverse | 6 | { "vertexProps": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | + # | 6 | Argument | | | + When profiling query: + """ + MATCH (m)-[]-(n), (n)-[]-(l), (l)-[]-(h) WHERE id(m)=="Tim Duncan" + RETURN m.player.name AS n1, n.player.name AS n2, l.team.name AS n3, h.player.name AS n4 + ORDER BY n1, n2, n3, n4 LIMIT 10 + """ + Then the result should be, in order: + | n1 | n2 | n3 | n4 | + | "Tim Duncan" | "Aron Baynes" | "Celtics" | "Aron Baynes" | + | "Tim Duncan" | "Aron Baynes" | "Celtics" | "Kyrie Irving" | + | "Tim Duncan" | "Aron Baynes" | "Celtics" | "Rajon Rondo" | + | "Tim Duncan" | "Aron Baynes" | "Celtics" | "Ray Allen" | + | "Tim Duncan" | "Aron Baynes" | "Celtics" | "Shaquile O'Neal" | + | "Tim Duncan" | "Aron Baynes" | "Pistons" | "Aron Baynes" | + | "Tim Duncan" | "Aron Baynes" | "Pistons" | "Blake Griffin" | + | "Tim Duncan" | "Aron Baynes" | "Pistons" | "Grant Hill" | + | "Tim Duncan" | "Aron Baynes" | "Spurs" | "Aron Baynes" | + | "Tim Duncan" | "Aron Baynes" | "Spurs" | "Boris Diaw" | + + # And the execution plan should be: + # | id | name | dependencies | operator info | + # | 19 | DataCollect | 20 | | + # | 20 | TopN | 23 | | + # | 23 | Project | 21 | | + # | 21 | Filter | 13 | | + # | 13 | BiInnerJoin | 9, 12 | | + # | 9 | BiInnerJoin | 5, 8 | | + # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 4 | Traverse | 2 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 2 | Dedup | 1 | | + # | 1 | PassThrough | 3 | | + # | 3 | Start | | | + # | 8 | AppendVertices | 7 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}, {\"tagId\":3,\"props\":[\"name\"]}]" } | + # | 7 | Traverse | 6 | { "vertexProps": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | + # | 6 | Argument | | | + # | 12 | AppendVertices | 11 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 11 | Traverse | 10 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":3}]" } | + # | 10 | Argument | | | + Scenario: Multi Match + When profiling query: + """ + MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" + MATCH (n)-[]-(l) + RETURN m.player.name AS n1, n.player.name AS n2, + CASE WHEN l.player.name is not null THEN l.player.name + WHEN l.team.name is not null THEN l.team.name ELSE "null" END AS n3 ORDER BY n1, n2, n3 LIMIT 10 + """ + Then the result should be, in order: + | n1 | n2 | n3 | + | "Tim Duncan" | "Aron Baynes" | "Celtics" | + | "Tim Duncan" | "Aron Baynes" | "Pistons" | + | "Tim Duncan" | "Aron Baynes" | "Spurs" | + | "Tim Duncan" | "Aron Baynes" | "Tim Duncan" | + | "Tim Duncan" | "Boris Diaw" | "Hawks" | + | "Tim Duncan" | "Boris Diaw" | "Hornets" | + | "Tim Duncan" | "Boris Diaw" | "Jazz" | + | "Tim Duncan" | "Boris Diaw" | "Spurs" | + | "Tim Duncan" | "Boris Diaw" | "Suns" | + | "Tim Duncan" | "Boris Diaw" | "Tim Duncan" | + # And the execution plan should be: + # | id | name | dependencies | operator info | + # | 16 | DataCollect | 17 | | + # | 17 | TopN | 13 | | + # | 13 | Project | 12 | | + # | 12 | BiInnerJoin | 19, 11 | | + # | 19 | Project | 18 | | + # | 18 | Filter | 5 | | + # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 4 | Traverse | 2 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 2 | Dedup | 1 | | + # | 1 | PassThrough | 3 | | + # | 3 | Start | | | + # | 11 | Project | 10 | | + # | 10 | AppendVertices | 9 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}, {\"tagId\":3,\"props\":[\"name\"]}]" } | + # | 9 | Traverse | 8 | { "vertexProps": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | + # | 8 | Argument | | | + When profiling query: + """ + MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" + MATCH (n)-[]-(l), (l)-[]-(h) + RETURN m.player.name AS n1, n.player.name AS n2, l.team.name AS n3, h.player.name AS n4 + ORDER BY n1, n2, n3, n4 LIMIT 10 + """ + Then the result should be, in order: + | n1 | n2 | n3 | n4 | + | "Tim Duncan" | "Aron Baynes" | "Celtics" | "Aron Baynes" | + | "Tim Duncan" | "Aron Baynes" | "Celtics" | "Kyrie Irving" | + | "Tim Duncan" | "Aron Baynes" | "Celtics" | "Rajon Rondo" | + | "Tim Duncan" | "Aron Baynes" | "Celtics" | "Ray Allen" | + | "Tim Duncan" | "Aron Baynes" | "Celtics" | "Shaquile O'Neal" | + | "Tim Duncan" | "Aron Baynes" | "Pistons" | "Aron Baynes" | + | "Tim Duncan" | "Aron Baynes" | "Pistons" | "Blake Griffin" | + | "Tim Duncan" | "Aron Baynes" | "Pistons" | "Grant Hill" | + | "Tim Duncan" | "Aron Baynes" | "Spurs" | "Aron Baynes" | + | "Tim Duncan" | "Aron Baynes" | "Spurs" | "Boris Diaw" | + # And the execution plan should be: + # | id | name | dependencies | operator info | + # | 20 | DataCollect | 21 | | + # | 21 | TopN | 17 | | + # | 17 | Project | 16 | | + # | 16 | BiInnerJoin | 23, 15 | | + # | 23 | Project | 22 | | + # | 22 | Filter | 5 | | + # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 4 | Traverse | 2 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 2 | Dedup | 1 | | + # | 1 | PassThrough | 3 | | + # | 3 | Start | | | + # | 15 | Project | 14 | | + # | 14 | BiInnerJoin | 10, 13 | | + # | 10 | AppendVertices | 9 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}, {\"tagId\":3,\"props\":[\"name\"]}]" } | + # | 9 | Traverse | 8 | { "vertexProps": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | + # | 8 | Argument | | | + # | 13 | AppendVertices | 12 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}, {\"tagId\":3,\"props\":[\"name\"]}]" } | + # | 12 | Traverse | 11 | { "vertexProps": "[{\"tagId\":3,\"props\":[\"name\"]}]" } | + # | 11 | Argument | | | + When profiling query: + """ + MATCH (v:player{name:"Tony Parker"}) + WITH v AS a + MATCH p=(o:player{name:"Tim Duncan"})-[]->(a) + RETURN o.player.name + """ + Then the result should be, in order: + | o.player.name | + | "Tim Duncan" | + | "Tim Duncan" | + + # And the execution plan should be: + # | id | name | dependencies | operator info | + # | 10 | Project | 11 | | + # | 11 | BiInnerJoin | 14, 9 | | + # | 14 | Project | 3 | | + # | 3 | AppendVertices | 12 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 12 | IndexScan | 2 | | + # | 2 | Start | | | + # | 9 | Project | 8 | | + # | 8 | AppendVertices | 7 | { "props": "[{\"props\":[\"name\", \"_tag\"],\"tagId\":3}, {\"props\":[\"name\", \"age\", \"_tag\"],\"tagId\":2}, {\"props\":[\"name\", , \"speciality\", \"_tag\"],\"tagId\":4}, {\"props\":[\"id\", \"ts\", \"_tag\"],\"tagId\":6}]" } | + # | 7 | Traverse | 6 | { "vertexProps": "[{\"props\":[\"name\", \"_tag\"],\"tagId\":3}, {\"props\":[\"name\", \"age\", \"_tag\"],\"tagId\":2}, {\"props\":[\"name\", , \"speciality\", \"_tag\"],\"tagId\":4}, {\"props\":[\"id\", \"ts\", \"_tag\"],\"tagId\":6}]" } | + # | 6 | Argument | | | + Scenario: Optional Match + When profiling query: + """ + MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" + OPTIONAL MATCH (n)<-[:serve]-(l) + RETURN m.player.name AS n1, n.player.name AS n2, l AS n3 ORDER BY n1, n2, n3 LIMIT 10 + """ + Then the result should be, in order: + | n1 | n2 | n3 | + | "Tim Duncan" | "Aron Baynes" | NULL | + | "Tim Duncan" | "Boris Diaw" | NULL | + | "Tim Duncan" | "Danny Green" | NULL | + | "Tim Duncan" | "Danny Green" | NULL | + | "Tim Duncan" | "Dejounte Murray" | NULL | + | "Tim Duncan" | "LaMarcus Aldridge" | NULL | + | "Tim Duncan" | "LaMarcus Aldridge" | NULL | + | "Tim Duncan" | "Manu Ginobili" | NULL | + | "Tim Duncan" | "Manu Ginobili" | NULL | + | "Tim Duncan" | "Manu Ginobili" | NULL | + # And the execution plan should be: + # | id | name | dependencies | operator info | + # | 16 | DataCollect | 17 | | + # | 17 | TopN | 13 | | + # | 13 | Project | 12 | | + # | 12 | BiLeftJoin | 19, 11 | | + # | 19 | Project | 18 | | + # | 18 | Filter | 5 | | + # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 4 | Traverse | 2 | | + # | 2 | Dedup | 1 | | + # | 1 | PassThrough | 3 | | + # | 3 | Start | | | + # | 11 | Project | 10 | | + # | 10 | AppendVertices | 9 | { "props": "[{\"props\":[\"name\", \"_tag\"],\"tagId\":3}, {\"props\":[\"name\", \"age\", \"_tag\"],\"tagId\":2}, {\"props\":[\"name\", , \"speciality\", \"_tag\"],\"tagId\":4}, {\"props\":[\"id\", \"ts\", \"_tag\"],\"tagId\":6}]" } | + # | 9 | Traverse | 8 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | + # | 8 | Argument | | | + When profiling query: + """ + MATCH (m:player{name:"Tim Duncan"})-[:like]-(n)--() + WITH m,n + MATCH (m)--(n) + RETURN count(*) AS scount + """ + Then the result should be, in order: + | scount | + | 270 | + +# And the execution plan should be: +# | id | name | dependencies | operator info | +# | 12 | Aggregate | 13 | | +# | 13 | BiInnerJoin | 15, 11 | | +# | 15 | Project | 5 | | +# | 5 | AppendVertices | 4 | { "props": "[]" } | +# | 4 | Traverse | 3 | { "vertexProps": "" } | +# | 3 | Traverse | 14 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | +# | 14 | IndexScan | 2 | | +# | 2 | Start | | | +# | 11 | Project | 10 | | +# | 10 | AppendVertices | 9 | { "props": "[]" } | +# | 9 | Traverse | 8 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | +# | 8 | Argument | | | From b8a96af568f61898295bacb34115e3ac7e1bfc56 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Thu, 10 Feb 2022 17:56:50 +0800 Subject: [PATCH 24/27] add markDeleted interface --- src/graph/optimizer/Optimizer.cpp | 10 ---------- src/graph/planner/plan/PlanNode.h | 5 +++++ src/graph/visitor/PrunePropertiesVisitor.cpp | 12 +++++++----- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/graph/optimizer/Optimizer.cpp b/src/graph/optimizer/Optimizer.cpp index 3dfee9a598f..de570d6bac6 100644 --- a/src/graph/optimizer/Optimizer.cpp +++ b/src/graph/optimizer/Optimizer.cpp @@ -35,11 +35,6 @@ StatusOr Optimizer::findBestPlan(QueryContext *qctx) { auto root = qctx->plan()->root(); auto spaceID = qctx->rctx()->session()->space().id; - // auto status = preprocess(root, qctx, spaceID); - // if (!status.ok()) { - // LOG(ERROR) << "Failed to preprocess plan: " << status; - // } - auto ret = prepare(optCtx.get(), root); NG_RETURN_IF_ERROR(ret); auto rootGroup = std::move(ret).value(); @@ -54,11 +49,6 @@ StatusOr Optimizer::findBestPlan(QueryContext *qctx) { return newRoot; } -// Status Optimizer::preprocess(PlanNode *root, graph::QueryContext *qctx, GraphSpaceID spaceID) { -// ColumnTracker colsUsed; -// return root->pruneCulumns(colsUsed, qctx, spaceID); -// } - Status Optimizer::postprocess(PlanNode *root, graph::QueryContext *qctx, GraphSpaceID spaceID) { if (FLAGS_enable_optimizer_property_pruner_rule) { graph::PropertyTracker propsUsed; diff --git a/src/graph/planner/plan/PlanNode.h b/src/graph/planner/plan/PlanNode.h index d4bea302938..35e19b70b87 100644 --- a/src/graph/planner/plan/PlanNode.h +++ b/src/graph/planner/plan/PlanNode.h @@ -193,6 +193,10 @@ class PlanNode { virtual void accept(PlanNodeVisitor* visitor); + void markDeleted() { + deleted_ = true; + } + virtual PlanNode* clone() const = 0; virtual void calcCost(); @@ -315,6 +319,7 @@ class PlanNode { std::vector dependencies_; std::vector inputVars_; std::vector outputVars_; + bool deleted_{false}; }; std::ostream& operator<<(std::ostream& os, PlanNode::Kind kind); diff --git a/src/graph/visitor/PrunePropertiesVisitor.cpp b/src/graph/visitor/PrunePropertiesVisitor.cpp index e02c4e81f5c..6207b107c22 100644 --- a/src/graph/visitor/PrunePropertiesVisitor.cpp +++ b/src/graph/visitor/PrunePropertiesVisitor.cpp @@ -193,6 +193,7 @@ void PrunePropertiesVisitor::visit(Traverse *node) { status_ = depsPruneProperties(node->dependencies()); } +// AppendVertices should be deleted when no properties it pulls are used by the parent node. void PrunePropertiesVisitor::visit(AppendVertices *node) { auto &colNames = node->colNames(); DCHECK(!colNames.empty()); @@ -215,8 +216,9 @@ void PrunePropertiesVisitor::visit(AppendVertices *node) { if (it2 != propsUsed_.vertexPropsMap.end()) { auto &usedVertexProps = it2->second; if (usedVertexProps.empty()) { - // markAsToBeDeleted(); - // return depsPruneProperties(propsUsed_, qctx_, spaceID_); + node->markDeleted(); + status_ = depsPruneProperties(node->dependencies()); + return; } prunedVertexProps->reserve(usedVertexProps.size()); for (auto &vertexProp : *vertexProps) { @@ -238,9 +240,9 @@ void PrunePropertiesVisitor::visit(AppendVertices *node) { } } } else { - // AppendVertices should be deleted when no props are used by the parent node - // markAsToBeDeleted(); - // It could be done by ColumnPruner + node->markDeleted(); + status_ = depsPruneProperties(node->dependencies()); + return; } node->setVertexProps(std::move(prunedVertexProps)); } From cb33f65ecc527f77f58a0c1014734f5ad2ad0131 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Thu, 10 Feb 2022 18:20:20 +0800 Subject: [PATCH 25/27] remove unused code --- src/common/expression/Expression.h | 1 - src/graph/optimizer/Optimizer.h | 2 - src/graph/util/ExpressionUtils.h | 2 - .../features/match/MultiQueryParts.feature | 169 ++---------------- .../tck/features/yield/NoSpaceChosen.feature | 1 - 5 files changed, 17 insertions(+), 158 deletions(-) diff --git a/src/common/expression/Expression.h b/src/common/expression/Expression.h index 23dd93ffe29..96bb68451d0 100644 --- a/src/common/expression/Expression.h +++ b/src/common/expression/Expression.h @@ -8,7 +8,6 @@ #include "common/base/Base.h" #include "common/base/ObjectPool.h" -#include "common/base/Status.h" #include "common/context/ExpressionContext.h" #include "common/datatypes/Value.h" diff --git a/src/graph/optimizer/Optimizer.h b/src/graph/optimizer/Optimizer.h index 46ba4006079..b09a48ef4a8 100644 --- a/src/graph/optimizer/Optimizer.h +++ b/src/graph/optimizer/Optimizer.h @@ -33,8 +33,6 @@ class Optimizer final { StatusOr findBestPlan(graph::QueryContext *qctx); private: - Status preprocess(graph::PlanNode *root, graph::QueryContext *qctx, GraphSpaceID spaceID); - Status postprocess(graph::PlanNode *root, graph::QueryContext *qctx, GraphSpaceID spaceID); StatusOr prepare(OptContext *ctx, graph::PlanNode *root); diff --git a/src/graph/util/ExpressionUtils.h b/src/graph/util/ExpressionUtils.h index ceba8e99633..b5d3f642fc3 100644 --- a/src/graph/util/ExpressionUtils.h +++ b/src/graph/util/ExpressionUtils.h @@ -169,8 +169,6 @@ class ExpressionUtils { static bool isGeoIndexAcceleratedPredicate(const Expression* expr); static bool checkExprDepth(const Expression* expr); - - static constexpr int32_t kMaxDepth = 512; }; } // namespace graph diff --git a/tests/tck/features/match/MultiQueryParts.feature b/tests/tck/features/match/MultiQueryParts.feature index 10654ce2c05..2a5cda28864 100644 --- a/tests/tck/features/match/MultiQueryParts.feature +++ b/tests/tck/features/match/MultiQueryParts.feature @@ -7,7 +7,7 @@ Feature: Multi Query Parts Given a graph with space named "nba" Scenario: Multi Path Patterns - When profiling query: + When executing query: """ MATCH (m)-[]-(n), (n)-[]-(l) WHERE id(m)=="Tim Duncan" RETURN m.player.name AS n1, n.player.name AS n2, @@ -26,42 +26,7 @@ Feature: Multi Query Parts | "Tim Duncan" | "Boris Diaw" | "Spurs" | | "Tim Duncan" | "Boris Diaw" | "Suns" | | "Tim Duncan" | "Boris Diaw" | "Tim Duncan" | - # And the execution plan should be: - # | id | name | dependencies | operator info | - # | 15 | DataCollect | 16 | | - # | 16 | TopN | 12 | | - # | 12 | Project | 18 | | - # | 18 | Project | 17 | | - # | 17 | Filter | 9 | | - # | 9 | BiInnerJoin | 5, 8 | | - # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | - # | 4 | Traverse | 2 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | - # | 2 | Dedup | 1 | | - # | 1 | PassThrough | 3 | | - # | 3 | Start | | | - # | 8 | AppendVertices | 7 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}, {\"tagId\":3,\"props\":[\"name\"]}]" } | - # | 7 | Traverse | 6 | { "vertexProps": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | - # | 6 | Argument | | | - When profiling query: - """ - MATCH (m)-[]-(n), (l)-[]-(n) WHERE id(m)=="Tim Duncan" - RETURN m.player.name AS n1, n.player.name AS n2, - CASE WHEN l.team.name is not null THEN l.team.name - WHEN l.player.name is not null THEN l.player.name ELSE "null" END AS n3 ORDER BY n1, n2, n3 LIMIT 10 - """ - Then the result should be, in order: - | n1 | n2 | n3 | - | "Tim Duncan" | "Aron Baynes" | "Celtics" | - | "Tim Duncan" | "Aron Baynes" | "Pistons" | - | "Tim Duncan" | "Aron Baynes" | "Spurs" | - | "Tim Duncan" | "Aron Baynes" | "Tim Duncan" | - | "Tim Duncan" | "Boris Diaw" | "Hawks" | - | "Tim Duncan" | "Boris Diaw" | "Hornets" | - | "Tim Duncan" | "Boris Diaw" | "Jazz" | - | "Tim Duncan" | "Boris Diaw" | "Spurs" | - | "Tim Duncan" | "Boris Diaw" | "Suns" | - | "Tim Duncan" | "Boris Diaw" | "Tim Duncan" | - When profiling query: + When executing query: """ MATCH (m)-[]-(n), (n)-[]-(l) WHERE id(n)=="Tim Duncan" RETURN m.player.name AS n1, n.player.name AS n2, l.player.name AS n3 ORDER BY n1, n2, n3 LIMIT 10 @@ -78,7 +43,7 @@ Feature: Multi Query Parts | "Aron Baynes" | "Tim Duncan" | "Manu Ginobili" | | "Aron Baynes" | "Tim Duncan" | "Manu Ginobili" | | "Aron Baynes" | "Tim Duncan" | "Manu Ginobili" | - When profiling query: + When executing query: """ MATCH (m)-[]-(n), (n)-[]-(l), (l)-[]-(h) WHERE id(m)=="Tim Duncan" RETURN m.player.name AS n1, n.player.name AS n2, l.team.name AS n3, h.player.name AS n4 @@ -96,26 +61,7 @@ Feature: Multi Query Parts | "Tim Duncan" | "Aron Baynes" | "Pistons" | "Grant Hill" | | "Tim Duncan" | "Aron Baynes" | "Spurs" | "Aron Baynes" | | "Tim Duncan" | "Aron Baynes" | "Spurs" | "Boris Diaw" | - # And the execution plan should be: - # | id | name | dependencies | operator info | - # | 19 | DataCollect | 20 | | - # | 20 | TopN | 23 | | - # | 23 | Project | 21 | | - # | 21 | Filter | 13 | | - # | 13 | BiInnerJoin | 9, 12 | | - # | 9 | BiInnerJoin | 5, 8 | | - # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | - # | 4 | Traverse | 2 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | - # | 2 | Dedup | 1 | | - # | 1 | PassThrough | 3 | | - # | 3 | Start | | | - # | 8 | AppendVertices | 7 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}, {\"tagId\":3,\"props\":[\"name\"]}]" } | - # | 7 | Traverse | 6 | { "vertexProps": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | - # | 6 | Argument | | | - # | 12 | AppendVertices | 11 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | - # | 11 | Traverse | 10 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":3}]" } | - # | 10 | Argument | | | - # Below scenario is not suppoted for the execution plan has a scan. + # Below scenario is not supported for the execution plan has a scan. When executing query: """ MATCH (m)-[]-(n), (a)-[]-(c) WHERE id(m)=="Tim Duncan" @@ -124,7 +70,7 @@ Feature: Multi Query Parts Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down. Scenario: Multi Match - When profiling query: + When executing query: """ MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" MATCH (n)-[]-(l) @@ -144,24 +90,7 @@ Feature: Multi Query Parts | "Tim Duncan" | "Boris Diaw" | "Spurs" | | "Tim Duncan" | "Boris Diaw" | "Suns" | | "Tim Duncan" | "Boris Diaw" | "Tim Duncan" | - # And the execution plan should be: - # | id | name | dependencies | operator info | - # | 16 | DataCollect | 17 | | - # | 17 | TopN | 13 | | - # | 13 | Project | 12 | | - # | 12 | BiInnerJoin | 19, 11 | | - # | 19 | Project | 18 | | - # | 18 | Filter | 5 | | - # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | - # | 4 | Traverse | 2 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | - # | 2 | Dedup | 1 | | - # | 1 | PassThrough | 3 | | - # | 3 | Start | | | - # | 11 | Project | 10 | | - # | 10 | AppendVertices | 9 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}, {\"tagId\":3,\"props\":[\"name\"]}]" } | - # | 9 | Traverse | 8 | { "vertexProps": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | - # | 8 | Argument | | | - When profiling query: + When executing query: """ MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" MATCH (n)-[]-(l), (l)-[]-(h) @@ -180,28 +109,7 @@ Feature: Multi Query Parts | "Tim Duncan" | "Aron Baynes" | "Pistons" | "Grant Hill" | | "Tim Duncan" | "Aron Baynes" | "Spurs" | "Aron Baynes" | | "Tim Duncan" | "Aron Baynes" | "Spurs" | "Boris Diaw" | - # And the execution plan should be: - # | id | name | dependencies | operator info | - # | 20 | DataCollect | 21 | | - # | 21 | TopN | 17 | | - # | 17 | Project | 16 | | - # | 16 | BiInnerJoin | 23, 15 | | - # | 23 | Project | 22 | | - # | 22 | Filter | 5 | | - # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | - # | 4 | Traverse | 2 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | - # | 2 | Dedup | 1 | | - # | 1 | PassThrough | 3 | | - # | 3 | Start | | | - # | 15 | Project | 14 | | - # | 14 | BiInnerJoin | 10, 13 | | - # | 10 | AppendVertices | 9 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}, {\"tagId\":3,\"props\":[\"name\"]}]" } | - # | 9 | Traverse | 8 | { "vertexProps": "[{\"tagId\":2,\"props\":[\"name\"]}]" } | - # | 8 | Argument | | | - # | 13 | AppendVertices | 12 | { "props": "[{\"tagId\":2,\"props\":[\"name\"]}, {\"tagId\":3,\"props\":[\"name\"]}]" } | - # | 12 | Traverse | 11 | { "vertexProps": "[{\"tagId\":3,\"props\":[\"name\"]}]" } | - # | 11 | Argument | | | - When profiling query: + When executing query: """ MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" MATCH (n)-[]-(l) @@ -221,7 +129,7 @@ Feature: Multi Query Parts | "Tim Duncan" | "Aron Baynes" | "Pistons" | "Grant Hill" | | "Tim Duncan" | "Aron Baynes" | "Spurs" | "Aron Baynes" | | "Tim Duncan" | "Aron Baynes" | "Spurs" | "Boris Diaw" | - When profiling query: + When executing query: """ MATCH (v:player{name:"Tony Parker"}) WITH v AS a @@ -233,20 +141,8 @@ Feature: Multi Query Parts | "Tim Duncan" | | "Tim Duncan" | - # And the execution plan should be: - # | id | name | dependencies | operator info | - # | 10 | Project | 11 | | - # | 11 | BiInnerJoin | 14, 9 | | - # | 14 | Project | 3 | | - # | 3 | AppendVertices | 12 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | - # | 12 | IndexScan | 2 | | - # | 2 | Start | | | - # | 9 | Project | 8 | | - # | 8 | AppendVertices | 7 | { "props": "[{\"props\":[\"name\", \"_tag\"],\"tagId\":3}, {\"props\":[\"name\", \"age\", \"_tag\"],\"tagId\":2}, {\"props\":[\"name\", , \"speciality\", \"_tag\"],\"tagId\":4}, {\"props\":[\"id\", \"ts\", \"_tag\"],\"tagId\":6}]" } | - # | 7 | Traverse | 6 | { "vertexProps": "[{\"props\":[\"name\", \"_tag\"],\"tagId\":3}, {\"props\":[\"name\", \"age\", \"_tag\"],\"tagId\":2}, {\"props\":[\"name\", , \"speciality\", \"_tag\"],\"tagId\":4}, {\"props\":[\"id\", \"ts\", \"_tag\"],\"tagId\":6}]" } | - # | 6 | Argument | | | Scenario: Optional Match - When profiling query: + When executing query: """ MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" OPTIONAL MATCH (n)<-[:serve]-(l) @@ -264,25 +160,8 @@ Feature: Multi Query Parts | "Tim Duncan" | "Manu Ginobili" | NULL | | "Tim Duncan" | "Manu Ginobili" | NULL | | "Tim Duncan" | "Manu Ginobili" | NULL | - # And the execution plan should be: - # | id | name | dependencies | operator info | - # | 16 | DataCollect | 17 | | - # | 17 | TopN | 13 | | - # | 13 | Project | 12 | | - # | 12 | BiLeftJoin | 19, 11 | | - # | 19 | Project | 18 | | - # | 18 | Filter | 5 | | - # | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":2}]" } | - # | 4 | Traverse | 2 | | - # | 2 | Dedup | 1 | | - # | 1 | PassThrough | 3 | | - # | 3 | Start | | | - # | 11 | Project | 10 | | - # | 10 | AppendVertices | 9 | { "props": "[{\"props\":[\"name\", \"_tag\"],\"tagId\":3}, {\"props\":[\"name\", \"age\", \"_tag\"],\"tagId\":2}, {\"props\":[\"name\", , \"speciality\", \"_tag\"],\"tagId\":4}, {\"props\":[\"id\", \"ts\", \"_tag\"],\"tagId\":6}]" } | - # | 9 | Traverse | 8 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | - # | 8 | Argument | | | - # Below scenario is not suppoted for the execution plan has a scan. - When profiling query: + # Below scenario is not supported for the execution plan has a scan. + When executing query: """ MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" OPTIONAL MATCH (a)<-[]-(b) @@ -291,7 +170,7 @@ Feature: Multi Query Parts Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down. Scenario: Multi Query Parts - When profiling query: + When executing query: """ MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" WITH n, n.player.name AS n1 ORDER BY n1 LIMIT 10 @@ -312,7 +191,7 @@ Feature: Multi Query Parts | "Boris Diaw" | "Spurs" | | "Boris Diaw" | "Suns" | | "Boris Diaw" | "Tim Duncan" | - When profiling query: + When executing query: """ MATCH (m:player{name:"Tim Duncan"})-[:like]-(n)--() WITH m,count(*) AS lcount @@ -322,7 +201,7 @@ Feature: Multi Query Parts Then the result should be, in order: | scount | lcount | | 19 | 110 | - When profiling query: + When executing query: """ MATCH (m:player{name:"Tim Duncan"})-[:like]-(n)--() WITH m,n @@ -332,21 +211,7 @@ Feature: Multi Query Parts Then the result should be, in order: | scount | | 270 | - # And the execution plan should be: - # | id | name | dependencies | operator info | - # | 12 | Aggregate | 13 | | - # | 13 | BiInnerJoin | 15, 11 | | - # | 15 | Project | 5 | | - # | 5 | AppendVertices | 4 | { "props": "[]" } | - # | 4 | Traverse | 3 | { "vertexProps": "" } | - # | 3 | Traverse | 14 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | - # | 14 | IndexScan | 2 | | - # | 2 | Start | | | - # | 11 | Project | 10 | | - # | 10 | AppendVertices | 9 | { "props": "[]" } | - # | 9 | Traverse | 8 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]" } | - # | 8 | Argument | | | - # Below scenario is not suppoted for the execution plan has a scan. + # Below scenario is not supported for the execution plan has a scan. When executing query: """ MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" @@ -357,14 +222,14 @@ Feature: Multi Query Parts Then a ExecutionError should be raised at runtime: Scan vertices or edges need to specify a limit number, or limit number can not push down. Scenario: Some Erros - When profiling query: + When executing query: """ MATCH (m)-[]-(n) WHERE id(m)=="Tim Duncan" WITH n, n.player.name AS n1 ORDER BY n1 LIMIT 10 RETURN m """ Then a SemanticError should be raised at runtime: Alias used but not defined: `m' - When profiling query: + When executing query: """ MATCH (v:player)-[e]-(v:team) RETURN v, e """ diff --git a/tests/tck/features/yield/NoSpaceChosen.feature b/tests/tck/features/yield/NoSpaceChosen.feature index 5a30e0a6c32..3c79382d44f 100644 --- a/tests/tck/features/yield/NoSpaceChosen.feature +++ b/tests/tck/features/yield/NoSpaceChosen.feature @@ -1,4 +1,3 @@ -@wang Feature: Yield Background: From 679ff6736647a05d53239f2ca8c6d745113bd4fd Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Fri, 11 Feb 2022 11:41:53 +0800 Subject: [PATCH 26/27] fix header macro --- src/graph/planner/plan/PlanNodeVisitor.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/graph/planner/plan/PlanNodeVisitor.h b/src/graph/planner/plan/PlanNodeVisitor.h index bd3e9f9ed2e..eb8b6d8d0da 100644 --- a/src/graph/planner/plan/PlanNodeVisitor.h +++ b/src/graph/planner/plan/PlanNodeVisitor.h @@ -3,8 +3,8 @@ * This source code is licensed under Apache 2.0 License. */ -#ifndef PLAN_PLANVISITOR_H_ -#define PLAN_PLANVISITOR_H_ +#ifndef PLAN_PLANNODEVISITOR_H_ +#define PLAN_PLANNODEVISITOR_H_ #include "graph/planner/plan/PlanNode.h" #include "graph/planner/plan/Query.h" @@ -28,4 +28,4 @@ class PlanNodeVisitor { } // namespace graph } // namespace nebula -#endif // PLAN_PLANVISITOR_H_ +#endif // PLAN_PLANNODEVISITOR_H_ From 1e936eac5c9e4cc1f76e4f6bfb2498d82168aedb Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Fri, 11 Feb 2022 13:02:59 +0800 Subject: [PATCH 27/27] update tck --- .../optimizer/PrunePropertiesRule.feature | 87 +++++++++++-------- 1 file changed, 51 insertions(+), 36 deletions(-) diff --git a/tests/tck/features/optimizer/PrunePropertiesRule.feature b/tests/tck/features/optimizer/PrunePropertiesRule.feature index b1ef947521e..55f4455c4c1 100644 --- a/tests/tck/features/optimizer/PrunePropertiesRule.feature +++ b/tests/tck/features/optimizer/PrunePropertiesRule.feature @@ -9,34 +9,38 @@ Feature: Prune Properties rule Scenario: Single Match When profiling query: """ - MATCH p = (v:player{name: "Tony Parker"})-[e:like]-(v2) + MATCH p = (v:player{name: "Tony Parker"})-[e:like]->(v2) RETURN v2.player.age """ Then the result should be, in order: | v2.player.age | - | 29 | - | 41 | - | 36 | - | 33 | - | 33 | | 42 | - | 42 | - | 32 | + | 33 | + | 41 | + # And the execution plan should be: + # | id | name | dependencies | operator info | + # | 8 | Project | 4 | | + # | 4 | AppendVertices | 3 | { "props": "[{\"props\":[\"age\"],\"tagId\":2}]" } | + # | 3 | Traverse | 7 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]", "edgeProps": "[{\"props\":[\"_dst\", \"_rank\", \"_type\", \"_src\"],\"type\":5}]" } | + # | 7 | IndexScan | 2 | | + # | 2 | Start | | | When profiling query: """ - MATCH p = (v:player{name: "Tony Parker"})-[e:like]-(v2) - RETURN v.player.age + MATCH p = (v:player{name: "Tony Parker"})-[e:like]->(v2) + RETURN v.player.name """ Then the result should be, in order: - | v.player.age | - | 36 | - | 36 | - | 36 | - | 36 | - | 36 | - | 36 | - | 36 | - | 36 | + | v.player.name | + | "Tony Parker" | + | "Tony Parker" | + | "Tony Parker" | + # And the execution plan should be: + # | id | name | dependencies | operator info | + # | 8 | Project | 4 | | + # | 4 | AppendVertices | 3 | { "props": } | + # | 3 | Traverse | 7 | { "vertexProps": "[{\"props\":[\"name\", \"age\"],\"tagId\":2}]", "edgeProps": "[{\"props\":[\"_dst\", \"_rank\", \"_type\", \"_src\"],\"type\":5}]" } | + # | 7 | IndexScan | 2 | | + # | 2 | Start | | | When profiling query: """ MATCH p = (v:player{name: "Tony Parker"})-[e:like]-(v2) @@ -52,23 +56,32 @@ Feature: Prune Properties rule | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | + # And the execution plan should be: + # | id | name | dependencies | operator info | + # | 8 | Project | 4 | | + # | 4 | AppendVertices | 3 | { "props": } | + # | 3 | Traverse | 7 | { "vertexProps": "[{\"props\":[\"name\", \"age\"],\"tagId\":2}]", "edgeProps": "[{\"props\":[\"_dst\", \"_rank\", \"_type\", \"_src\"],\"type\":5}]" } | + # | 7 | IndexScan | 2 | | + # | 2 | Start | | | When profiling query: """ - MATCH p = (v:player{name: "Tony Parker"})-[e:like]-(v2) + MATCH p = (v:player{name: "Tony Parker"})-[e:like]->(v2) RETURN v2 """ Then the result should be, in order: | v2 | - | ("Dejounte Murray" :player{age: 29, name: "Dejounte Murray"}) | - | ("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"}) | - | ("Boris Diaw" :player{age: 36, name: "Boris Diaw"}) | - | ("LaMarcus Aldridge" :player{age: 33, name: "LaMarcus Aldridge"}) | - | ("LaMarcus Aldridge" :player{age: 33, name: "LaMarcus Aldridge"}) | - | ("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"}) | | ("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"}) | - | ("Marco Belinelli" :player{age: 32, name: "Marco Belinelli"}) | - # The rule will not take affect in this case - When profiling query: + | ("LaMarcus Aldridge" :player{age: 33, name: "LaMarcus Aldridge"}) | + | ("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"}) | + # And the execution plan should be: + # | id | name | dependencies | operator info | + # | 8 | Project | 4 | | + # | 4 | AppendVertices | 3 | { "props": "[{\"props\":[\"name\", \"_tag\"],\"tagId\":3}, {\"props\":[\"name\", \"name\", \"age\", \"_tag\"],\"tagId\":2}, {\"props\":[\"name\", \"speciality\", \"_tag\"], \"tagId\":4}]" } | + # | 3 | Traverse | 7 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]", "edgeProps": "[{\"props\":[\"_dst\", \"_rank\", \"_type\", \"_src\"],\"type\":5}]" } | + # | 7 | IndexScan | 2 | | + # | 2 | Start | | | + # The rule will not take affect in this case because it returns the whole path + When executing query: """ MATCH p = (v:player{name: "Tony Parker"})-[e:like]-(v2) RETURN p @@ -85,7 +98,7 @@ Feature: Prune Properties rule | <("Tony Parker" :player{age: 36, name: "Tony Parker"})<-[:like@0 {likeness: 50}]-("Marco Belinelli" :player{age: 32, name: "Marco Belinelli"})> | When profiling query: """ - MATCH p = (v:player{name: "Tony Parker"})-[e:like]-(v2) + MATCH p = (v:player{name: "Tony Parker"})-[e:like]->(v2) RETURN type(e) """ Then the result should be, in order: @@ -93,12 +106,14 @@ Feature: Prune Properties rule | "like" | | "like" | | "like" | - | "like" | - | "like" | - | "like" | - | "like" | - | "like" | - When profiling query: + # And the execution plan should be: + # | id | name | dependencies | operator info | + # | 8 | Project | 4 | | + # | 4 | AppendVertices | 3 | { "props": } | + # | 3 | Traverse | 7 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":2}]", "edgeProps": "[{\"props\":[\"_dst\", \"_rank\", \"_type\", \"_src\"],\"type\":5}]" } | + # | 7 | IndexScan | 2 | | + # | 2 | Start | | | + When executing query: """ MATCH (v:player{name: "Tony Parker"})-[:like]-(v2)--(v3) WITH v3, v3.player.age AS age