-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
util, executor: use UnionRanges build index kv range for INLJ #15727
Conversation
05c57bf
to
13dd5bb
Compare
Codecov Report
@@ Coverage Diff @@
## master #15727 +/- ##
===========================================
Coverage 80.4907% 80.4907%
===========================================
Files 504 504
Lines 134608 134608
===========================================
Hits 108347 108347
Misses 17789 17789
Partials 8472 8472 |
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
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
Co-Authored-By: Kenan Yao <cauchy1992@gmail.com>
I cannot figure out why the old codes is wrong since the merge algo is the same. Can you create a issue for future opotimization? |
/merge |
/run-all-tests |
@XuHuaiyu merge failed. |
unit test failed because of #15594 |
Signed-off-by: sre-bot <sre-bot@pingcap.com>
cherry pick to release-3.0 in PR #15753 |
Signed-off-by: sre-bot <sre-bot@pingcap.com>
cherry pick to release-3.1 in PR #15754 |
cherry pick to release-4.0 in PR #15756 |
What problem does this PR solve?
Issue Number: close #15686
close #15693
close #15687
Problem Summary:
Before this commit, we build a wrong kv ranges which will cause data lost when index join fetchInnerResult.
What is changed and how it works?
What's Changed:
Use UnionRanges to merge ranges.
How it Works:
Related changes
Check List
Tests
Side effects
Release note