-
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
expression: return null when cast to huge binary type #8768
Conversation
/run-all-tests |
1 similar comment
/run-all-tests |
65b4514
to
af37275
Compare
/run-all-tests |
/run-common-test |
/run-integration-common-test |
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
/rebuild |
1 similar comment
/rebuild |
/run-integration-common-test |
/run-all-tests |
/run-unit-test |
/run-unit-test |
/run-all-tests |
@zz-jason @lamxTyler PTAL |
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?
Before this PR, for this query:
and tidb-server is down, because we would allocate about 4GB memory for the result, so it triggers golang runtime panic "out of memory" on my machine, but we don't catch this panic and do recover.
What is changed and how it works?
According to https://dev.mysql.com/doc/refman/5.7/en/cast-functions.html#function_cast, MySQL should also allocate this 4GB memory for binary result, and pad
\0
in the tail, but in my practical experiment, it does not. Here is the output of MySQL 5.7.10:In other word, MySQL does not behave as its documentation on this issue.
We have 2 choices to fix this problem:
these 2 choices may lead to different behaviors, I prefer the second one, because if we impose no check on the length and recover when panic occurs, it has risk to trigger panic in other places since binary type is assumed to be less than 255 bytes.Based on #8768 (comment), change the behavior to be compatible with MySQL 8.0.
Check List
Tests
Code changes
N/A
Side effects
as mentioned above,No compatibility breaking now.select cast(1 as binary(1024))
is able to be executed successfully before, but unable now.Related changes
This change is