-
Notifications
You must be signed in to change notification settings - Fork 5.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
planner/core: implement skyline pruning #9337
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9337 +/- ##
==========================================
+ Coverage 67.13% 67.16% +0.02%
==========================================
Files 372 372
Lines 77768 77839 +71
==========================================
+ Hits 52211 52277 +66
- Misses 20887 20891 +4
- Partials 4670 4671 +1
Continue to review full report at Codecov.
|
@@ -567,11 +567,6 @@ func loadAllTests() ([]string, error) { | |||
if strings.HasSuffix(name, ".test") { | |||
name = strings.TrimSuffix(name, ".test") | |||
|
|||
// if we use record and the result file exists, skip generating |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why change this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make ./run-all-tests -r all
work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, maybe the logic in tidb-test repo should also be changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I will do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
besides, could you add an explain test that shows the power of skyline pruning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/run-all-tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What problem does this PR solve?
Fix #8593
What is changed and how it works?
Implement the skyline pruning according to the proposal.
Check List
Tests
Code changes
Side effects
Related changes