From d7ff4660fdce4ae9be97145490e4c320018f5d42 Mon Sep 17 00:00:00 2001 From: yixinglu <2520865+yixinglu@users.noreply.github.com> Date: Tue, 6 Dec 2022 11:41:06 +0800 Subject: [PATCH] Fuck the bug --- src/graph/optimizer/CMakeLists.txt | 2 +- src/graph/optimizer/OptGroup.cpp | 24 +++++++++--------- .../rule/PushFilterDownHashInnerJoinRule.cpp | 5 +++- src/graph/planner/plan/Query.cpp | 25 ++++++++----------- 4 files changed, 27 insertions(+), 29 deletions(-) diff --git a/src/graph/optimizer/CMakeLists.txt b/src/graph/optimizer/CMakeLists.txt index 6a8bc6c67eb..cae7300d687 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 e0b42fbc487..0fcc0fdcb35 100644 --- a/src/graph/optimizer/OptGroup.cpp +++ b/src/graph/optimizer/OptGroup.cpp @@ -126,13 +126,9 @@ void OptGroup::addGroupNode(OptGroupNode *groupNode) { } OptGroupNode *OptGroup::makeGroupNode(PlanNode *node) { - if (outputVar_.empty()) { - outputVar_ = node->outputVar(); - } else { - DCHECK_EQ(outputVar_, node->outputVar()); - } - groupNodes_.emplace_back(OptGroupNode::create(ctx_, node, this)); - return groupNodes_.back(); + auto *gn = OptGroupNode::create(ctx_, node, this); + addGroupNode(gn); + return gn; } Status OptGroup::explore(const OptRule *rule) { @@ -167,9 +163,15 @@ Status OptGroup::explore(const OptRule *rule) { NG_RETURN_IF_ERROR(resStatus); auto result = std::move(resStatus).value(); - // for (auto *ngn : result.newGroupNodes) { - // NG_RETURN_IF_ERROR(validateSubPlan(ngn, rule, leaves)); - // } + for (auto *gn : result.newGroupNodes) { + auto it = std::find(groupNodes_.begin(), groupNodes_.end(), gn); + if (it != groupNodes_.end()) { + return Status::Error("Should not push the OptGroupNode %s into the group in the rule: %s", + gn->node()->toString().c_str(), + rule->toString().c_str()); + } + } + // 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) { @@ -178,7 +180,6 @@ Status OptGroup::explore(const OptRule *rule) { } groupNodes_.clear(); for (auto ngn : result.newGroupNodes) { - // NG_RETURN_IF_ERROR(validateSubPlan(ngn, rule, leaves)); addGroupNode(ngn); } break; @@ -186,7 +187,6 @@ Status OptGroup::explore(const OptRule *rule) { if (!result.newGroupNodes.empty()) { for (auto ngn : result.newGroupNodes) { - // NG_RETURN_IF_ERROR(validateSubPlan(ngn, rule, leaves)); addGroupNode(ngn); } diff --git a/src/graph/optimizer/rule/PushFilterDownHashInnerJoinRule.cpp b/src/graph/optimizer/rule/PushFilterDownHashInnerJoinRule.cpp index 4c3a8c274b7..92b2513f80f 100644 --- a/src/graph/optimizer/rule/PushFilterDownHashInnerJoinRule.cpp +++ b/src/graph/optimizer/rule/PushFilterDownHashInnerJoinRule.cpp @@ -64,7 +64,10 @@ StatusOr PushFilterDownHashInnerJoinRule::transform( // produce new InnerJoin node auto* newInnerJoinNode = static_cast(oldInnerJoinNode->clone()); auto newJoinGroup = rightFilterUnpicked ? OptGroup::create(octx) : filterGroupNode->group(); - auto newGroupNode = OptGroupNode::create(octx, newInnerJoinNode, newJoinGroup); + // TODO(yee): it's too tricky + auto newGroupNode = rightFilterUnpicked + ? const_cast(newJoinGroup)->makeGroupNode(newInnerJoinNode) + : OptGroupNode::create(octx, newInnerJoinNode, newJoinGroup); newGroupNode->dependsOn(leftGroup); newGroupNode->dependsOn(rightGroup); newInnerJoinNode->setLeftVar(leftGroup->outputVar()); diff --git a/src/graph/planner/plan/Query.cpp b/src/graph/planner/plan/Query.cpp index 6145559f02a..2383987b78a 100644 --- a/src/graph/planner/plan/Query.cpp +++ b/src/graph/planner/plan/Query.cpp @@ -866,14 +866,15 @@ HashJoin::HashJoin(QueryContext* qctx, : BinaryInputNode(qctx, kind, left, right), hashKeys_(std::move(hashKeys)), probeKeys_(std::move(probeKeys)) { - auto lColNames = left->colNames(); - auto& rColNames = right->colNames(); - for (auto& rColName : rColNames) { - if (std::find(lColNames.begin(), lColNames.end(), rColName) == lColNames.end()) { - lColNames.emplace_back(rColName); + if (left && right) { + auto lColNames = left->colNames(); + for (auto& rColName : right->colNames()) { + if (std::find(lColNames.begin(), lColNames.end(), rColName) == lColNames.end()) { + lColNames.emplace_back(rColName); + } } + setColNames(lColNames); } - setColNames(lColNames); } std::unique_ptr HashLeftJoin::explain() const { @@ -883,9 +884,7 @@ std::unique_ptr HashLeftJoin::explain() const { } PlanNode* HashLeftJoin::clone() const { - auto* lnode = left() ? left()->clone() : nullptr; - auto* rnode = right() ? right()->clone() : nullptr; - auto* newLeftJoin = HashLeftJoin::make(qctx_, lnode, rnode); + auto* newLeftJoin = HashLeftJoin::make(qctx_, nullptr, nullptr); newLeftJoin->cloneMembers(*this); return newLeftJoin; } @@ -901,9 +900,7 @@ std::unique_ptr HashInnerJoin::explain() const { } PlanNode* HashInnerJoin::clone() const { - auto* lnode = left() ? left()->clone() : nullptr; - auto* rnode = right() ? right()->clone() : nullptr; - auto* newInnerJoin = HashInnerJoin::make(qctx_, lnode, rnode); + auto* newInnerJoin = HashInnerJoin::make(qctx_, nullptr, nullptr); newInnerJoin->cloneMembers(*this); return newInnerJoin; } @@ -942,9 +939,7 @@ void RollUpApply::cloneMembers(const RollUpApply& r) { } PlanNode* RollUpApply::clone() const { - auto* lnode = left() ? left()->clone() : nullptr; - auto* rnode = right() ? right()->clone() : nullptr; - auto* newRollUpApply = RollUpApply::make(qctx_, lnode, rnode, {}, nullptr); + auto* newRollUpApply = RollUpApply::make(qctx_, nullptr, nullptr, {}, nullptr); newRollUpApply->cloneMembers(*this); return newRollUpApply; }