From e4ba34e8aa1d992f31c986371eff529076a6c4b3 Mon Sep 17 00:00:00 2001 From: yixinglu <2520865+yixinglu@users.noreply.github.com> Date: Mon, 5 Dec 2022 23:12:07 +0800 Subject: [PATCH] Add validator for OptGroup --- src/graph/optimizer/CMakeLists.txt | 2 +- src/graph/optimizer/OptGroup.cpp | 67 +++++++++++++++++++++++++++++- src/graph/optimizer/OptGroup.h | 4 ++ src/graph/optimizer/Optimizer.cpp | 1 + 4 files changed, 71 insertions(+), 3 deletions(-) diff --git a/src/graph/optimizer/CMakeLists.txt b/src/graph/optimizer/CMakeLists.txt index cae7300d687..6a8bc6c67eb 100644 --- a/src/graph/optimizer/CMakeLists.txt +++ b/src/graph/optimizer/CMakeLists.txt @@ -27,7 +27,7 @@ nebula_add_library( rule/PushFilterDownAggregateRule.cpp rule/PushFilterDownProjectRule.cpp rule/PushFilterDownLeftJoinRule.cpp - rule/PushFilterDownHashInnerJoinRule.cpp + # rule/PushFilterDownHashInnerJoinRule.cpp rule/PushFilterDownInnerJoinRule.cpp rule/PushFilterDownNodeRule.cpp rule/PushFilterDownScanVerticesRule.cpp diff --git a/src/graph/optimizer/OptGroup.cpp b/src/graph/optimizer/OptGroup.cpp index 5cb47a1baee..e0b42fbc487 100644 --- a/src/graph/optimizer/OptGroup.cpp +++ b/src/graph/optimizer/OptGroup.cpp @@ -92,6 +92,28 @@ Status OptGroup::validateSubPlan(const OptGroupNode *gn, return Status::OK(); } +Status OptGroup::validate(const OptRule *rule) const { + if (groupNodes_.empty() && !groupNodesReferenced_.empty()) { + return Status::Error( + "The OptGroup has no any OptGroupNode but used by other OptGroupNode " + "when applying the rule: %s, numGroupNodesRef: %lu", + rule->toString().c_str(), + groupNodesReferenced_.size()); + } + for (auto *gn : groupNodes_) { + NG_RETURN_IF_ERROR(gn->validate(rule)); + if (gn->node()->outputVar() != outputVar_) { + return Status::Error( + "The output columns of the OptGroupNode is different from OptGroup " + "when applying the rule: %s, %s vs. %s", + rule->toString().c_str(), + gn->node()->outputVar().c_str(), + outputVar_.c_str()); + } + } + return Status::OK(); +} + void OptGroup::addGroupNode(OptGroupNode *groupNode) { DCHECK_EQ(this, DCHECK_NOTNULL(groupNode)->group()); if (outputVar_.empty()) { @@ -144,6 +166,10 @@ Status OptGroup::explore(const OptRule *rule) { auto resStatus = rule->transform(ctx_, matched); NG_RETURN_IF_ERROR(resStatus); auto result = std::move(resStatus).value(); + + // for (auto *ngn : result.newGroupNodes) { + // NG_RETURN_IF_ERROR(validateSubPlan(ngn, rule, leaves)); + // } // In some cases, we can apply optimization rules even if the control flow and data flow are // inconsistent. For now, let the optimization rules themselves guarantee correctness. if (result.eraseAll) { @@ -152,7 +178,7 @@ Status OptGroup::explore(const OptRule *rule) { } groupNodes_.clear(); for (auto ngn : result.newGroupNodes) { - NG_RETURN_IF_ERROR(validateSubPlan(ngn, rule, leaves)); + // NG_RETURN_IF_ERROR(validateSubPlan(ngn, rule, leaves)); addGroupNode(ngn); } break; @@ -160,7 +186,7 @@ Status OptGroup::explore(const OptRule *rule) { if (!result.newGroupNodes.empty()) { for (auto ngn : result.newGroupNodes) { - NG_RETURN_IF_ERROR(validateSubPlan(ngn, rule, leaves)); + // NG_RETURN_IF_ERROR(validateSubPlan(ngn, rule, leaves)); addGroupNode(ngn); } @@ -300,5 +326,42 @@ void OptGroupNode::release() { } } +Status OptGroupNode::validate(const OptRule *rule) const { + if (!node_) { + return Status::Error("The OptGroupNode does not have plan node when applying the rule: %s", + rule->toString().c_str()); + } + if (!group_) { + return Status::Error( + "The OptGroupNode does not have the right OptGroup when applying the rule: %s", + rule->toString().c_str()); + } + if (!bodies_.empty()) { + if (node_->kind() != PlanNode::Kind::kLoop) { + return Status::Error( + "The plan node is not Loop in OptGroupNode when applying the rule: %s, planNode: %s", + rule->toString().c_str(), + PlanNode::toString(node_->kind())); + } + for (auto *g : bodies_) { + NG_RETURN_IF_ERROR(g->validate(rule)); + } + } + if (dependencies_.empty()) { + if (node_->kind() != PlanNode::Kind::kStart && node_->kind() != PlanNode::Kind::kArgument) { + return Status::Error( + "The leaf plan node is not Start or Argument in OptGroupNode when applying the rule: %s, " + "planNode: %s", + rule->toString().c_str(), + PlanNode::toString(node_->kind())); + } + } else { + for (auto *g : dependencies_) { + NG_RETURN_IF_ERROR(g->validate(rule)); + } + } + return Status::OK(); +} + } // namespace opt } // namespace nebula diff --git a/src/graph/optimizer/OptGroup.h b/src/graph/optimizer/OptGroup.h index 230d7a15cb4..1f264928229 100644 --- a/src/graph/optimizer/OptGroup.h +++ b/src/graph/optimizer/OptGroup.h @@ -58,6 +58,8 @@ class OptGroup final { isRootGroup_ = true; } + Status validate(const OptRule *rule) const; + private: friend ObjectPool; explicit OptGroup(OptContext *ctx) noexcept; @@ -133,6 +135,8 @@ class OptGroupNode final { // Release the opt group node from its opt group void release(); + Status validate(const OptRule *rule) const; + private: friend ObjectPool; OptGroupNode(graph::PlanNode *node, const OptGroup *group) noexcept; diff --git a/src/graph/optimizer/Optimizer.cpp b/src/graph/optimizer/Optimizer.cpp index e9767ffd0a5..b6909361669 100644 --- a/src/graph/optimizer/Optimizer.cpp +++ b/src/graph/optimizer/Optimizer.cpp @@ -81,6 +81,7 @@ Status Optimizer::doExploration(OptContext *octx, OptGroup *rootGroup) { for (auto rule : ruleSet->rules()) { // Explore until the maximum number of iterations(Rules) is reached NG_RETURN_IF_ERROR(rootGroup->exploreUntilMaxRound(rule)); + NG_RETURN_IF_ERROR(rootGroup->validate(rule)); rootGroup->setUnexplored(rule); } }