planner: fix should not use point get plan #21124
Conversation
planner/core/point_get_plan.go
Outdated
| dKind := d.Kind() | ||
| switch col.FieldType.EvalType() { | ||
| case ptypes.ETString: | ||
| switch dKind { |
There was a problem hiding this comment.
Should we use the white list?
There was a problem hiding this comment.
For now I just find String type column with numeric constant will cause the bug. So I just use black list style.
planner/core/point_get_plan_test.go
Outdated
| tk.MustExec("create table t3(id varchar(20), d varchar(20), unique(id, d))") | ||
| tk.MustExec("create table t4(id int, d varchar(20), c varchar(20), unique(id, d))") | ||
|
|
||
| tk.MustQuery("explain select * from t1 where id = 0").Check(testkit.Rows( |
There was a problem hiding this comment.
Please use the test data to record the plan test.
| case ptypes.ETString: | ||
| switch kind { | ||
| case types.KindInt64, types.KindUint64, | ||
| types.KindFloat32, types.KindFloat64, types.KindMysqlDecimal: |
There was a problem hiding this comment.
Should bit be contained here?
There was a problem hiding this comment.
I will do bit type test on mysql later.
There was a problem hiding this comment.
OK, please also test year and other int like types.
There was a problem hiding this comment.
datetime serise is tested, no problem now. do we need test case for this?
There was a problem hiding this comment.
That's much better. More test cases of different types could help us avoid this problem in the future.
There was a problem hiding this comment.
@lzmhhh123 bit and Hex binary literal tested same as MySQL, so just char column with numeric literal need to be reject from try point get plan.
|
@lzmhhh123 datetime series literal equals with char column has some corner case. But with numeric literal and bit or hex literal is perform same as MySQL. |
The corner case between |
|
@lzmhhh123 PTAL #21167 |
|
@blacktear23 I found a related bug too, Could you take a look? #21204 |
|
/merge |
ti-srebot
added
the
status/can-merge
|
Your auto merge job has been accepted, waiting for:
|
|
/run-all-tests |
|
@blacktear23 merge failed. |
|
/merge |
|
Your auto merge job has been accepted, waiting for:
|
|
/run-all-tests |
|
@blacktear23 merge failed. |
Yes |
|
@crazycs520 PTAL |
|
/run-all-tests |
|
/merge |
|
/run-all-tests |
ti-srebot
added
status/LGT3
|
/merge |
|
Your auto merge job has been accepted, waiting for:
|
|
@blacktear23 merge failed. |
|
/run-all-tests |
|
@bb7133 do we need cherry-pick? |
|
/run-cherry-picker |
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
|
cherry pick to release-4.0 in PR #21244 |
What problem does this PR solve?
Issue Number: close #21123 , close #21204
Problem Summary:
In MySQL string convert to numbers it will use number prefix to convert to number, for example:
So if we create a table like
create table t (id varchar(10) primary key), then insert ('asdf'), ('1asdf') into this table. When we execute a query like:the plan should be convert column id into int and compare with number 0, so we should get 'asdf' as the result. But in TiDB it will convert number 0 to string '0' and use point get plan to fetch
id = '0'rows, so we will get an empty set result.What is changed and how it works?
What's Changed:
Add type check for TryFastPlan, if column type is string like and
inor=value is number just return empty plan.Related changes
Check List
Tests
Side effects
Release note