Skip to content
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

Add support for precision in DEFAULT NOW #78

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

albertorestifo
Copy link

This PR adds supports for passing a precision to the time in the DEFAULT statement, for example:

CREATE TABLE `test` (
    `valid` TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP(3)
)

According to the mysql documentation, CURRENT_TIMESTAMP is an alias for NOW().

This PR also adds a transformation to normalize CURRENT_TIMESTAMP into NOW.

@albertorestifo
Copy link
Author

I found another failing case regarding the timestamp precision. I'm working in a fix.

@hrvoj3e
Copy link

hrvoj3e commented Aug 16, 2019

I think CURRENT_TIMESTAMP should be kept as is - not normalize it.
Reasoning - online MySQL docs offer examples with CURRENT_TIMESTAMP and not NOW (or other aliases).
So this tool would detect schema changes when there are none (because aliasing).
You would essentialy be running an opinionated syntax checker.

E.g. show create table sometablename; returns no value for precision on 10.3.17-MariaDB

`time_created` timestamp NULL DEFAULT current_timestamp(),

Maybe a flag as an opt-in option?

@albertorestifo
Copy link
Author

@hrvoj3e I'm willing to drop this change if the maintainers agree with your reasoning, however keep in mind that Schemalex already does normalization.

@hrvoj3e
Copy link

hrvoj3e commented Aug 18, 2019

however keep in mind that Schemalex already does normalization.

@albertorestifo I did not know that. It makes sense to be consistent across the app - maintainers will decide at the end. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants