executor: fix issue that update ignore stmt return error when meet incorrect timestamp value error #54878
Conversation
…correct timestamp value Signed-off-by: crazycs520 <[email protected]>
Signed-off-by: crazycs520 <[email protected]>
ti-chi-bot
bot
added
do-not-merge/needs-triage-completed
release-note-none
|
Hi @crazycs520. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. 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 kubernetes-sigs/prow repository. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #54878 +/- ##
=================================================
- Coverage 72.8127% 57.5471% -15.2657%
=================================================
Files 1557 1694 +137
Lines 438081 634110 +196029
=================================================
+ Hits 318979 364912 +45933
- Misses 99416 245350 +145934
- Partials 19686 23848 +4162
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Signed-off-by: crazycs520 <[email protected]>
|
/retest-required |
|
@crazycs520: Cannot trigger testing until a trusted user reviews the PR and leaves an 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 kubernetes-sigs/prow repository. |
|
/ok-to-test |
|
/retest-required |
1 similar comment
|
/retest-required |
Signed-off-by: crazycs520 <[email protected]>
|
|
||
| if types.ErrTruncatedWrongVal.Equal(err) && col != nil && col.ColumnInfo != nil && col.ColumnInfo.GetType() == mysql.TypeTimestamp { | ||
| ec := e.Ctx().GetSessionVars().StmtCtx.ErrCtx() | ||
| return errors.AddStack(ec.HandleErrorWithAlias(kv.ErrKeyExists, err, err)) |
There was a problem hiding this comment.
Related error handling logic in Insert statement:
tidb/pkg/executor/insert_common.go
Lines 324 to 328 in 47179ae
I've added error type and column type checking to avoid mishandling other errors here.
|
@lcwangchao @cfzjywxk PTAL |
ti-chi-bot
bot
added
approved
needs-1-more-lgtm
| @@ -351,7 +351,10 @@ func (*UpdateExec) handleErr(colName model.CIStr, rowIdx int, err error) error { | |||
| if types.ErrOverflow.Equal(err) { | |||
| return types.ErrWarnDataOutOfRange.GenWithStackByArgs(colName.O, rowIdx+1) | |||
| } | |||
|
|
|||
| if types.ErrTruncatedWrongVal.Equal(err) && col != nil && col.ColumnInfo != nil && col.ColumnInfo.GetType() == mysql.TypeTimestamp { | |||
There was a problem hiding this comment.
So the condition here allowed to be igmored is "timestamp column && truncated value", right?
Is this condition check consistent with MySQL behaviour?
There was a problem hiding this comment.
Yes, this is compatible with MySQL, except for the error message.
MySQL 8.0.29
mysql> create table t(t timestamp);
Query OK, 0 rows affected (0.09 sec)
mysql> insert into t values("2000-01-01");
Query OK, 1 row affected (0.00 sec)
mysql> update ignore t set t=cast("2099-01-01" as date);
Query OK, 1 row affected, 1 warning (0.00 sec)
Rows matched: 1 Changed: 1 Warnings: 1
mysql> show warnings;
+---------+------+--------------------------------------------+
| Level | Code | Message |
+---------+------+--------------------------------------------+
| Warning | 1264 | Out of range value for column 't' at row 1 |
+---------+------+--------------------------------------------+
1 row in set (0.00 sec)
mysql> select * from t;
+---------------------+
| t |
+---------------------+
| 0000-00-00 00:00:00 |
+---------------------+
1 row in set (0.00 sec)
mysql> select version();
+-----------+
| version() |
+-----------+
| 8.0.29 |
+-----------+This PR
mysql> create table t(t timestamp);
Query OK, 0 rows affected (0.02 sec)
mysql> insert into t values("2000-01-01");
Query OK, 1 row affected (0.00 sec)
mysql> update ignore t set t=cast("2099-01-01" as date);
Query OK, 1 row affected, 1 warning (0.01 sec)
Rows matched: 1 Changed: 1 Warnings: 1
mysql> show warnings;
+---------+------+-----------------------------------------+
| Level | Code | Message |
+---------+------+-----------------------------------------+
| Warning | 1292 | Incorrect timestamp value: '2099-01-01' |
+---------+------+-----------------------------------------+
1 row in set (0.00 sec)
mysql> select * from t;
+---------------------+
| t |
+---------------------+
| 0000-00-00 00:00:00 |
+---------------------+
1 row in set (0.00 sec)
mysql> select tidb_version();
+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| tidb_version() |
+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Release Version: v8.2.0-alpha-675-g38a5139c3e
Edition: Community
Git Commit Hash: 38a5139c3eae82d3e2ea5f82f6ea89ceeabb505f
Git Branch: fix-50308
UTC Build Time: 2024-07-30 09:04:16
GoVersion: go1.21.5
Race Enabled: false
Check Table Before Drop: false
Store: unistore |
+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
|
/ok-to-test |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cfzjywxk, lcwangchao 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 |
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
crazycs520
added
the
needs-cherry-pick-release-8.1
|
In response to a cherrypick label: new pull request created to branch |
…correct timestamp value error (pingcap#54878) close pingcap#50308
What problem does this PR solve?
Issue Number: close #50308
Problem Summary: As title said, see more detail in issue #50308
What changed and how does it work?
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.