Skip to content
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.

Sign up for GitHub

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

Jump to bottom

stats: convert small range to points in selectivity #11524

Merged
merged 8 commits into from
Aug 5, 2019
Merged

stats: convert small range to points in selectivity #11524

merged 8 commits into from
Aug 5, 2019

Conversation

alivxxx
Copy link
Contributor

@alivxxx alivxxx commented Jul 30, 2019

What problem does this PR solve?

Previously, we always use histogram to estimate the range query, but if the range is small and the bucket is large, it is very likely that the estimation would be wrong.

What is changed and how it works?

Convert small range to points and use cm sketch to estimate them.

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change

Side effects

  • None

Related changes

  • None

@alivxxx alivxxx added type/enhancement The issue or PR belongs to an enhancement. component/statistics labels Jul 30, 2019
@codecov
Copy link

codecov bot commented Jul 30, 2019 •
edited
Loading

Codecov Report

Merging #11524 into master will decrease coverage by 0.0253%.
The diff coverage is 78.5714%.

@@               Coverage Diff               @@
##            master     #11524        +/-   ##
===============================================
- Coverage   81.355%   81.3297%   -0.0254%     
===============================================
  Files          426        425         -1     
  Lines        92320      91536       -784     
===============================================
- Hits         75107      74446       -661     
+ Misses       11864      11737       -127     
- Partials      5349       5353         +4

lzmhhh123
lzmhhh123 reviewed Jul 31, 2019
View reviewed changes
lzmhhh123
lzmhhh123 reviewed Jul 31, 2019
View reviewed changes
Copy link
Contributor

@lzmhhh123 lzmhhh123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@lzmhhh123 lzmhhh123 added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 31, 2019
winoros
winoros reviewed Jul 31, 2019
View reviewed changes
@alivxxx alivxxx requested a review from winoros July 31, 2019 10:46
eurekaka
eurekaka reviewed Aug 1, 2019
View reviewed changes
@alivxxx alivxxx requested a review from eurekaka August 1, 2019 10:49
eurekaka
eurekaka approved these changes Aug 5, 2019
View reviewed changes
Copy link
Contributor

@eurekaka eurekaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@eurekaka eurekaka added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 5, 2019
@zz-jason zz-jason merged commit 03bb8d7 into pingcap:master Aug 5, 2019
@alivxxx alivxxx deleted the small-range branch August 5, 2019 02:46
@eurekaka eurekaka mentioned this pull request Feb 24, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qw4990 qw4990 qw4990 left review comments

@lzmhhh123 lzmhhh123 lzmhhh123 left review comments

@eurekaka eurekaka eurekaka approved these changes

@winoros winoros Awaiting requested review from winoros

Assignees
No one assigned
Labels
component/statistics status/LGT2 Indicates that a PR has LGTM 2. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@alivxxx @eurekaka @qw4990 @winoros @lzmhhh123 @zz-jason @lonng