-
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
*: fix the estimation error on normal column when collation enabled (#18104) #18311
*: fix the estimation error on normal column when collation enabled (#18104) #18311
Conversation
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
/run-all-tests |
@winoros please accept the invitation then you can push to the cherry-pick pull requests. |
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.
There are many unnecessary changes in this PR, please fix them @winoros
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.
@winoros please resolve conflicts and fix CI.
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
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
/merge |
Your auto merge job has been accepted, waiting for:
|
/run-all-tests |
@ti-srebot merge failed. |
Tested with tikv/tikv#8620
|
/run-all-tests |
It's safe to merge this one before the related tikv pr. |
Is it a 4.0.6 release blocker? |
@AilinKid The tikv side will be merged. So this one should be merged too. |
cherry-pick #18104 to release-4.0
What problem does this PR solve?
Issue Number: close #14689
Problem Summary:
For index, its key is already generated by the sort key by the collation information. And when we query the index estimation, we also use
EncodeKey
which will first convert the column value to sort key then encode it. So it's automatically correct for index without any additional change.But for a column, when sampling we use its original value thus when query it with count-min sketch or histogram. We'll get a wrong answer since the order information is lost.
What is changed and how it works?
What's Changed:
For tikv part, use the sort key generated by the collation as the sampling data. tikv/tikv#8105
And when querying, convert to sort key first.
How it Works:
Related changes
Check List
Tests
Side effects
Release note