refactor(stream): remove Executor::clear_cache #1820
Conversation
Signed-off-by: TennyZhuang <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #1820 +/- ##
==========================================
+ Coverage 70.75% 70.77% +0.02%
==========================================
Files 607 607
Lines 79461 79411 -50
==========================================
- Hits 56220 56203 -17
+ Misses 23241 23208 -33
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
| self.managed_lowest_state.clear_cache(); | ||
| self.managed_middle_state.clear_cache(); | ||
| self.managed_highest_state.clear_cache(); | ||
| self.first_execution = true; |
There was a problem hiding this comment.
This is quite suspicious. Why setting first_execution here before? Could that be related to @BugenZhao 's recently discovered bug?
By the way, we can remove no cache e2e test in CI.
There was a problem hiding this comment.
The HashJoin has not been migrated to executor v2 and its clear_cache still makes sense. Let's keep it for a while.
There was a problem hiding this comment.
IIRC, the first_execution indicates whether we need to fetch the state from storage and was necessary when writing this. 🤣 I'll investigate into TopN in next days. cc @lmatz
Signed-off-by: TennyZhuang [email protected]
What's changed and what's your intention?
It's impossible to implement
clear_cacheinExecutorV2, we may investigate another way.Checklist
Refer to a related PR or issue link (optional)