-
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: fix autoid doesn't handle float, double type and tiny cleanup #11110
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11110 +/- ##
===========================================
Coverage 81.3669% 81.3669%
===========================================
Files 423 423
Lines 90479 90479
===========================================
Hits 73620 73620
Misses 11574 11574
Partials 5285 5285 |
/run-all-tests |
@tiancaiamao @lysu PTAL |
/run-unit-test |
1 similar comment
/run-unit-test |
There's another case the pr doesn't handle. eg: update stmt. I would like to mark the pr to "WIP". And more test case should add also. |
@winkyao @lzmhhh123 please review again, thanks. |
/run-all-tests |
/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
PTAL @winkyao |
PTAL @crazycs520 @lzmhhh123 |
executor/insert_test.go
Outdated
{ | ||
`insert into t4(id) values(6)`, | ||
`select * from t4 where id = 6`, | ||
testkit.Rows(`6 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.
It isn't compatible with MySQL5.7, the result of MySQL is as follows:
mysql> select * from t4 where id = 6;
+----+---+
| id | n |
+----+---+
| 6 | 7 |
+----+---+
1 row in set (0.01 sec)
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.
@zimulala fixed! thank you!
BTW, the update behavior is the same as MySQL8.0, but is not the same as MySQL5.7. I think the behavior of MySQL8.0 is expected to me.
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.
/run-all-tests |
cherry pick to release-2.1 failed |
cherry pick to release-3.0 failed |
…origin-release-3.0
…origin-release-3.0
…origin-release-3.0
pingcap#11110 Signed-off-by: crazycs <crazycs520@gmail.com>
What problem does this PR solve?
Fixes #11109
What is changed and how it works?
modify the
adjustAutoIncrementDatum
method logic, handle the float and double scenario.Check List
Tests
Code changes
Side effects
Related changes