-
Notifications
You must be signed in to change notification settings - Fork 5.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
executor: handle missing timestamp value for load data #11093
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11093 +/- ##
===============================================
- Coverage 81.1334% 81.014% -0.1195%
===============================================
Files 419 419
Lines 90801 89324 -1477
===============================================
- Hits 73670 72365 -1305
+ Misses 11877 11720 -157
+ Partials 5254 5239 -15 |
Codecov Report
@@ Coverage Diff @@
## master #11093 +/- ##
===============================================
+ Coverage 81.2235% 81.255% +0.0314%
===============================================
Files 423 423
Lines 90230 90056 -174
===============================================
- Hits 73288 73175 -113
+ Misses 11634 11579 -55
+ Partials 5308 5302 -6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add unit test?
@crazycs520 test case will be added right now. |
c2d0f42
to
6893593
Compare
/run-all-tests |
executor/load_data.go
Outdated
for i := 0; i < len(e.row); i++ { | ||
if i >= len(cols) { | ||
// If some columns is missing and their type is time and has not null flag, they should be set as current time. | ||
if totalCols[i].Tp == mysql.TypeTimestamp || totalCols[i].Tp == mysql.TypeDate || totalCols[i].Tp == mysql.TypeDatetime { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is better to use IsTypeTime
.
PTAL @crazycs520 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PTAL @crazycs520 |
b1f948e
to
2f996fc
Compare
a66a7b4
to
cdce9a8
Compare
/run-all-tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What problem does this PR solve?
fix #10504
LOAD DATA
has different behavior with MySQL when handling missing timestamp/datetime value.For example:
If we create a table:
The result for MySQL and TiDB are same:
But if
t
isNOT NULL
, things are different.TiDB:
MySQL:
What is changed and how it works?
Add check for missing timestamp columns when insert load data.
If these columns have
NOT NULL
flag, set them toCURRENT_TIMESTAMP
.Check List
Tests
Side effects
Related changes