executor: fix insert ignore invalid year #26097
Conversation
ti-chi-bot
added
the
size/XS
table/column.go
Outdated
| @@ -265,6 +265,8 @@ func CastValue(ctx sessionctx.Context, val types.Datum, col *model.ColumnInfo, r | |||
| if innCasted, exit, innErr := handleZeroDatetime(ctx, col, casted, val.GetString(), types.ErrWrongValue.Equal(err)); exit { | |||
| return innCasted, innErr | |||
| } | |||
| } else if (sc.InInsertStmt || sc.InUpdateStmt) && err != nil && col.FieldType.Tp == mysql.TypeYear { | |||
There was a problem hiding this comment.
Maybe usingtypes.ErrInvalidYear.Equal(err) instead of err != nil will be better.
There was a problem hiding this comment.
I have modified it, thanks. @mmyj
executor/insert_test.go
Outdated
| _, err = tk.Exec(`insert ignore into t values(1900);`) | ||
| c.Assert(err, IsNil) | ||
| tk.MustQuery("show warnings;").Check(testutil.RowsWithSep("|", "Warning|8033|invalid year")) | ||
| tk.MustQuery(`select * from t;`).Check(testkit.Rows(`0`)) |
There was a problem hiding this comment.
According to the doc
YEAR follows the following format rules:
Four-digit numeral ranges from 1901 to 2155
Four-digit string ranges from '1901' to '2155'
One-digit or two-digit numeral ranges from 1 to 99. Accordingly, 1-69 is converted to 2001-2069 and 70-99 is converted to 1970-1999
One-digit or two-digit string ranges from '0' to '99'
Value 0 is taken as 0000 whereas the string '0' or '00' is taken as 2000
Invalid YEAR value is automatically converted to 0000 (if users are not using the NO_ZERO_DATE SQL mode).
I think we need to add a new test unit with the both valid and invalid case.
By the way, what value the invalid YEAR value will be converted when using NO_ZERO_DATE SQL mode? In my local, whatever using or not, the invalid value is always converted to 0.
There was a problem hiding this comment.
ok, I will add some test cases and check NO_ZERO_DATE mode
There was a problem hiding this comment.
@mmyj
NO_ZERO_DATE will not affect the insertion of year type data, we can ignore it.
According to MySQL doc https://dev.mysql.com/doc/refman/8.0/en/sql-mode.html#sql-mode-strict
If the input parameter is an invalid value, an error will be produced only in strict SQL mode and without ignore, otherwise it can be inserted successfully and results in a warning.
I have add some test case, PTAL
ti-chi-bot
added
size/L
ti-chi-bot
added
size/M
| _, err = tk.Exec(`insert ignore into t1 values(1900), (2156), ("1900"), ("2156");`) | ||
| c.Assert(err, IsNil) | ||
| tk.MustQuery("show warnings;").Check(testutil.RowsWithSep("|", | ||
| "Warning|8033|invalid year", |
There was a problem hiding this comment.
This warning is different with MySQL-8.0.23, MySQL-5.7.33, TiDB-5.7.25-TiDB-v4.0.11. They all will produce this error (1264, "Out of range value for column 'y' at row 1").
show warnings;
+---------+------+--------------------------------------------+
| Level | Code | Message |
+---------+------+--------------------------------------------+
| Warning | 1264 | Out of range value for column 'y' at row 1 |
| Warning | 1264 | Out of range value for column 'y' at row 2 |
| Warning | 1264 | Out of range value for column 'y' at row 3 |
| Warning | 1264 | Out of range value for column 'y' at row 4 |
+---------+------+--------------------------------------------+
There was a problem hiding this comment.
I noticed this bug, but if I modified it, there will be some test cases that fail to pass, we'd better raise a new issue separately to deal with this problem.
There was a problem hiding this comment.
OK. Could you please help to create a bug issue and add a comment about it in this PR?
|
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
ti-chi-bot
added
the
needs-rebase
ti-chi-bot
removed
the
needs-rebase
|
@b41sh: The following test failed, say
Full PR test history. Your PR dashboard. 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/test-infra repository. I understand the commands that are listed here. |
|
@b41sh: The following tests failed, say
Full PR test history. Your PR dashboard. 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/test-infra repository. I understand the commands that are listed here. |
What problem does this PR solve?
Issue Number: close #25993
Problem Summary:
What is changed and how it works?
What's Changed: convert Invalid YEAR to 0000
Related changes
Check List
Tests
Release note