expression,ddl: fix fraction part handle of current_timestamp #7355
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/run-all-tests |
lysu
added
type/bugfix
|
LGTM |
coocood
reviewed
Aug 13, 2018
| @@ -121,10 +121,10 @@ CREATE TABLE `te` ( | |||
| `device_identy` varchar(36) NOT NULL, | |||
| `uuid` varchar(32) NOT NULL, | |||
| `status_flag` tinyint(4) NOT NULL, | |||
| `client_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, | |||
| `client_create_time` timestamp(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3), | |||
There was a problem hiding this comment.
Can we not adding (3) to CURRENT_TIMESTAMP?
There was a problem hiding this comment.
check fsp in create table is rollbacked in this PR, 83b4e233457a98dd5f52d633a44445d63b351e08.
tiancaiamao
reviewed
Aug 13, 2018
| @@ -1760,7 +1760,7 @@ func (s *testDBSuite) TestTableDDLWithTimeType(c *C) { | |||
| s.testErrorCode(c, "alter table t change column a aa time(7)", tmysql.ErrTooBigPrecision) | |||
| s.testErrorCode(c, "alter table t change column a aa datetime(7)", tmysql.ErrTooBigPrecision) | |||
| s.testErrorCode(c, "alter table t change column a aa timestamp(7)", tmysql.ErrTooBigPrecision) | |||
| s.mustExec(c, "alter table t change column a aa timestamp(0)") | |||
| s.mustExec(c, "alter table t change column a aa datetime(0)") | |||
There was a problem hiding this comment.
Why this is changed to datetime ?
There was a problem hiding this comment.
because previous test is wrong and should be failure with
ERROR 1105 (HY000): unsupported modify column type 7 not match origin 12
- - and it's strange mustExec in ddl_test seems ignore the err and doesn't fail the case.
There was a problem hiding this comment.
The test doesn't init the testKit, but reuse the one in the suite, so the test case never fail.
|
LGTM |
coocood
added
status/LGT2
|
/run-all-tests |
|
@lysu plz resolve conflicts so we can merge this PR 😄 |
|
@zz-jason done~ |
zz-jason
approved these changes
Aug 14, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
In this PR, fix the bug that
=retrieves the record that inserted bycurrent_timestamp.This bug is caused by
current_timestampdoesn't take care of thefspargument.user often use default
fspoftimestampand it is0, but current_timestamp generate and insert data with fsp part into storage. so when user selection table with no fsp value(they know nothing about fsp also not care about that..) and got the empty response.for old data after this PR still cannot retrieve by
=, but new data will work.Still exists question
expect that when creating a table with current_timestamp we should confirm
column's fsp == funcation's fsp, but solve this will break compatible, so 83b4e233457a98dd5f52d633a44445d63b351e08 rollbacked.also see: https://dev.mysql.com/doc/refman/5.7/en/timestamp-initialization.html
What is changed and how it works?
GetTimeValuefollow fsp argument.Check List
Code changes
Side effects
Related changes
This change is