stats: do not split excluded lower value ranges #12009
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alivxxx
added
type/bugfix
Codecov Report
@@ Coverage Diff @@
## master #12009 +/- ##
===========================================
Coverage 81.5929% 81.5929%
===========================================
Files 452 452
Lines 98060 98060
===========================================
Hits 80010 80010
Misses 12401 12401
Partials 5649 5649 |
winoros
reviewed
Sep 4, 2019
|
@winoros Main reason that I did not choose this solution is that there are old histograms and it is not easy to determine the original number of columns, because it may already been |
eurekaka
reviewed
Sep 11, 2019
| @@ -766,11 +766,11 @@ func formatBuckets(hg *statistics.Histogram, lowBkt, highBkt, idxCols int) strin | |||
| return hg.BucketToString(lowBkt, idxCols) | |||
| } | |||
| if lowBkt+1 == highBkt { | |||
| return fmt.Sprintf("%s, %s", hg.BucketToString(lowBkt, 0), hg.BucketToString(highBkt, 0)) | |||
| return fmt.Sprintf("%s, %s", hg.BucketToString(lowBkt, idxCols), hg.BucketToString(highBkt, idxCols)) | |||
There was a problem hiding this comment.
Why do we need this change?
There was a problem hiding this comment.
Without it, the result for index is unreadable.
Co-Authored-By: Kenan Yao <cauchy1992@gmail.com>
eurekaka
added
status/LGT2
|
/run-all-tests |
|
cherry pick to release-2.1 failed |
|
cherry pick to release-3.0 failed |
alivxxx
added a commit
to alivxxx/tidb
that referenced
this pull request
Sep 12, 2019
alivxxx
added a commit
to alivxxx/tidb
that referenced
this pull request
Sep 12, 2019
alivxxx
added a commit
to alivxxx/tidb
that referenced
this pull request
Sep 12, 2019
sre-bot
pushed a commit
that referenced
this pull request
Sep 13, 2019
sre-bot
pushed a commit
that referenced
this pull request
Sep 16, 2019
sre-bot
pushed a commit
that referenced
this pull request
Sep 16, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Fix #11907
This bug would occur on indices that have multiply columns like
index idx(a,b), and the bug happens when:Queries that only request ranges on prefix like
where a >= 10are issued and collected by feedback, which result in one of the bucket upper bound becomes single encoded value(10), and there is a chance that next buckets's upper bound is(10, 11).Then another queries comes like
where a >= 9, soSplitRangeis called with[9, +inf), so we will split by(10)and(10, 11), which result in ranges[9, 10],(10,(10,11)],..., and every thing looks fine now.The caller, which is
IndexRangesToKVRanges, will process the splited ranges(10,(10,11)]. Since lower is excluded,(10,(10,11)]will be transformed to[11, (10,12))by usingPrefixNext, so the invalid ranges happens.What is changed and how it works?
When we split ranges, do not generate execluded lower ranges. This PR does it by split the ranges by lower bound and always generate included lower ranges and excluded upper ranges.
Check List
Tests
Code changes
Side effects
Related changes
Release note