refactor(optimizer): reset expression display id for explain #8006
Conversation
| // In order to let expression display id started from 1 for explaining. | ||
| // We will reset expression display id to 0 and clone the whole plan to reset the schema. | ||
| let plan = { | ||
| let rules = vec![]; | ||
| // Use a optimizer without rules to clone a new plan. | ||
| let mut heuristic_optimizer = HeuristicOptimizer::new(&ApplyOrder::TopDown, &rules); | ||
| let old_expr_display_id = self.ctx().get_expr_display_id(); | ||
| self.ctx().set_expr_display_id(0); | ||
| let plan = heuristic_optimizer.optimize(self.clone()); | ||
| self.ctx().set_expr_display_id(old_expr_display_id); | ||
| plan | ||
| }; |
Codecov Report
@@ Coverage Diff @@
## main #8006 +/- ##
==========================================
+ Coverage 71.30% 71.35% +0.04%
==========================================
Files 1123 1124 +1
Lines 181038 181098 +60
==========================================
+ Hits 129093 129225 +132
+ Misses 51945 51873 -72
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Don't do this! Because of #7998, the optimizer calls |
Any problem? We will reset the |
| self.ctx().set_expr_display_id(0); | ||
| let plan = PlanCloner::clone_whole_plan(self.clone()); | ||
| self.ctx().set_expr_display_id(old_expr_display_id); |
There was a problem hiding this comment.
Agree with @st1page. A little counterintuitive to me and I am not sure if it is worth it.
There was a problem hiding this comment.
The clone_whole_plan is restricted in Explain scope only, so I think it is acceptability. Otherwise it is painful to add a new optimization, because it would affect lots of plan tests.
chenzl25
added
the
mergify/can-merge
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Checklist
./risedev check(or alias,./risedev c)Documentation
Click here for Documentation
Types of user-facing changes
Please keep the types that apply to your changes, and remove the others.
Release note