expression: fix the length of casting from INT/REAL/DECIMAL/.... to string | tidb-test=pr/2549 #61476
Conversation
ti-chi-bot
bot
added
release-note-none
|
Oops. I found it also affects the behavior. For example: select binary(123)Will return different values with/without this PR. I should be more careful. /hold |
ti-chi-bot
bot
added
the
do-not-merge/hold
YangKeao
force-pushed
the
fix-61350
branch
3 times, most recently
from
dc73f67 to
9a5c067
Compare
ti-chi-bot
bot
added
size/XL
YangKeao
force-pushed
the
fix-61350
branch
2 times, most recently
from
f0876f2 to
be36271
Compare
|
|
||
| // Only estimate the length for variable length string types, because different length for fixed | ||
| // length string types will have different behaviors and may cause compatibility issues. | ||
| if retFt.GetType() == mysql.TypeString { |
There was a problem hiding this comment.
select binary(123) will use cast to return TypeString. However, it's also not compatible with MySQL because MySQL also returns TypeVarString. Maybe we should modify it to TypeVarString in parser later.
BTW, binary function is deprecated by MySQL.
YangKeao
force-pushed
the
fix-61350
branch
4 times, most recently
from
4f70a7f to
7207764
Compare
|
/retest |
YangKeao
force-pushed
the
fix-61350
branch
2 times, most recently
from
794f99d to
d781589
Compare
YangKeao
changed the title
YangKeao
force-pushed
the
fix-61350
branch
4 times, most recently
from
6d1629a to
dd91f59
Compare
ti-chi-bot
bot
added
size/XXL
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
Signed-off-by: Yang Keao <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AilinKid, windtalker, xhebox The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
| // decimal representation. It's not necessary to keep all decimals. Ref: | ||
| // - https://github.com/ulfjack/ryu | ||
| // - https://dl.acm.org/doi/10.1145/93548.93559 | ||
| // So maybe 48/327 is enough for float/double, but we still set 87/370 for safety. |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
Signed-off-by: ti-chi-bot <[email protected]>
|
In response to a cherrypick label: new pull request created to branch |
|
/cherrypick release-8.1 |
Signed-off-by: ti-chi-bot <[email protected]>
|
@hawkingrei: new pull request created to branch In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository. |
Signed-off-by: ti-chi-bot <[email protected]>
|
@hawkingrei: new pull request created to branch In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository. |
What problem does this PR solve?
Issue Number: close #61350
Problem Summary:
The length returned by TiDB is not compatible with MySQL. Sometimes it's too small so that the client may fail to allocate big enough buffer and cause panic/security issue. This PR modifies the logic of calculating the length, and make it compatible with MySQL for most of the cases.
What changed and how does it work?
Check List
Tests
Run the SQLs provided in #61350 and check the results are the same.
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.