Fix optimizer symbols. #4106
Fix optimizer symbols. #4106
Conversation
Shylock-Hg
added
ready-for-testing
| for (std::size_t i = 0; i < dependencies_.size(); i++) { | ||
| const OptGroup *dep = dependencies_[i]; | ||
| node_->setInputVar(dep->outputVar(), i); | ||
| for (auto *groupNode : dep->groupNodes()) { |
There was a problem hiding this comment.
I think we need not to rebuild all OptGroupNodes in the depended OptGroup since there are many unchanged nodes which are not in the path of the OptRule. It will save much time to rebuild the nodes of transformed by the OptRule. WDYT?
There was a problem hiding this comment.
We could let users do it in rule, and only check it here. As you said before.
| @@ -31,6 +31,13 @@ const PlanNode *MatchedResult::planNode(const std::vector<int32_t> &pos) const { | |||
| return DCHECK_NOTNULL(result->node)->node(); | |||
| } | |||
|
|
|||
| void MatchedResult::collectBoundary(std::vector<OptGroup *> &boundary) const { | |||
There was a problem hiding this comment.
It's better to collect all OptGroupNode rather than OptGroup in the path of matched result.
There was a problem hiding this comment.
But the dependencies_ is OptGroup.
| @@ -30,6 +30,8 @@ class OptGroup; | |||
| struct MatchedResult { | |||
| const OptGroupNode *node{nullptr}; | |||
| std::vector<MatchedResult> dependencies; | |||
| // Boundary of the whole matched result, aka the dependencies of leaf node in MatchedResult. | |||
| std::vector<OptGroup *> boundary_; | |||
There was a problem hiding this comment.
It looks like there is no need to save this variable.
There was a problem hiding this comment.
Yes, we could collect it in match method directly.
|
Replaced by #4146 |
What type of PR is this?
What problem(s) does this PR solve?
Issue(s) number:
Close #3826
Description:
Add two constraints to optimizer:
And add one implicit transform:
And some improvement:
How do you solve it?
Special notes for your reviewer, ex. impact of this fix, design document, etc:
Checklist:
Tests:
Affects:
Release notes:
Please confirm whether to be reflected in release notes and how to describe: