*: refine the behavior of StrToInt and StrToFloat and support convert JSON to date, time and timestamp #17902
Conversation
Codecov Report
@@ Coverage Diff @@
## master #17902 +/- ##
===========================================
Coverage 79.7056% 79.7056%
===========================================
Files 526 526
Lines 144306 144306
===========================================
Hits 115020 115020
Misses 20086 20086
Partials 9200 9200 |
|
/run-all-tests |
|
/run-all-tests -tidb-test=pr/1067 |
| @@ -1110,7 +1110,7 @@ func (b *builtinCastStringAsIntSig) handleOverflow(origRes int64, origStr string | |||
| } | |||
|
|
|||
| sc := b.ctx.GetSessionVars().StmtCtx | |||
| if sc.InSelectStmt && types.ErrOverflow.Equal(origErr) { | |||
| if types.ErrOverflow.Equal(origErr) { | |||
There was a problem hiding this comment.
It looks like it would only happen in select statements before. Does the change make the insert statement different from mysql?
There was a problem hiding this comment.
According to the test, this change won't break the existing test. The reason I delete sc.InSelectStmt is that we shouldn't use InSelectStmt to decide how to handle the err. We should use TruncateAsWarning , IgnoreTruncate to do that.
| arg1Type = types.ETInt | ||
| } | ||
| bf, err := newBaseBuiltinFuncWithTp(ctx, c.funcName, args, types.ETDuration, arg0Type, arg1Type, types.ETReal) | ||
| bf, err := newBaseBuiltinFuncWithTp(ctx, c.funcName, args, types.ETDuration, types.ETInt, types.ETInt, types.ETReal) |
There was a problem hiding this comment.
For all types, arg is evaluated rounding to int64? Please add some comments.
Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>
|
/run-all-tests |
|
/run-common-test |
|
/run-integration-common-test |
|
/run-all-tests -tidb-test=pr/1067 |
|
/run-all-tests -tidb-test=pr/1067 |
2 similar comments
|
/run-all-tests -tidb-test=pr/1067 |
|
/run-all-tests -tidb-test=pr/1067 |
|
@Reminiscent, @qw4990, @hanfei1991, PTAL. |
|
/run-all-tests |
|
/run-all-tests -tidb-test=pr/1067 |
1 similar comment
|
/run-all-tests -tidb-test=pr/1067 |
|
/run-all-tests -tidb-test=pr/1067 |
|
/run-all-tests -tidb-test=pr/1067 |
|
/merge |
ti-srebot
added
the
status/can-merge
|
/run-all-tests |
|
@wjhuang2016 merge failed. |
|
/merge |
|
Sorry @wjhuang2016, you don't have permission to trigger auto merge event on this branch. You are not a committer for the related sigs:execution(slack). |
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
|
cherry pick to release-4.0 in PR #18159 |
Signed-off-by: zyguan <zhongyangguan@gmail.com>
What problem does this PR solve?
Issue Number: close #17898
Problem Summary:
TiDB don't distinguish cast(explicit CAST function or implicit cast) from cast(when casting into a column type, CastValue()).
We try to distinguish them according to if the statement is a select statement or an insert statement.
However, it's not correct, especially when the virtual generated column comes out. We can't make sure the value inserted and read are the same.
So we can't depend on if the type of the statement. We need to keep a principle. If some values are converted to a column type, then use CastValue(). Otherwise, don't use it.
Here is an example:
We didn't support converted a JSON to date. When we had a virtual generated column and it needed to be converted from a JSON to date, we used CastJsonAsDuration to do that. This is wrong. So I supported convert JSON to date, time, and timestamp within CastValue(), though their logics are quite similar.
Finally, I correct some tests. If you test these tests in MySQL and previously TiDB, you will see that the warnings of TiDB are wrong.
What is changed and how it works?
Related changes
Check List
Tests
Side effects
Release note