-
Notifications
You must be signed in to change notification settings - Fork 21.6k
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
MySQL 5.6 Fractional Seconds #14359
MySQL 5.6 Fractional Seconds #14359
Conversation
You might want to branch it to include this only for 5.6, but passing these values to < 5.6 doesn't cause issues either.
MySQL 5.6 Fractional Seconds
Is there an installable rails version to benefit from this? I'm using rails |
@Silex not actually, we will need to use Rails edge(master) for now, as this will be part of 4.2.x only |
@arthurnn: Thanks. It is really annoying not being able to store milliseconds from a time... Do you know if your patch also takes care about migrations? Right now I'm unable to create a |
Yep, you should be able to, look at the migration on the tests in this PR, https://github.com/rails/rails/pull/14359/files#diff-95d3dfc38be7275845454d444b0826f9R678, so you should be able to do: t.datetime : fetched_at, limit: 6 |
Ah, |
Note that this causes undesirable rounding behavior on MySQL 5.6 with lower-precision temporal fields. In 5.5, when you insert This has undesirable consequences like breaking apps that rely on Ideally, we'd format the datetime string according to the precision of the datetime field. Provide microseconds, milliseconds, tens of seconds, etc. No more precision than needed. |
Yay! I'm super glad this got merged at long last. I added the corresponding support to the mysql2 gem ages ago per Miyagawa's suggestion. Users will need mysql2 >= 0.3.12 for DATETIME and >= 0.3.17 for TIME with microseconds. |
@jeremy This is breaking our tests on Rails 4.2 like how you described. An On paper this looks like it should work, but MySQL stores the 23:23:17.xxxxxx as 23:23:18, so the We're on MySQL 5.6.16, but the column isn't set to store fractional seconds. Was a pain to track down the issue because this causes non-deterministic test failures :( |
@MaxGabriel I believe I just fixed this issue. Could you check 4-2-stable? |
@rafaelfranca That fixes it! Thanks very much 👍 |
❤️ |
@rafaelfranca we are upgrading our app to 4.2.0 right now and are experiencing the issue that @MaxGabriel was seeing, our db doesnt store the microseconds, but the query generated looks like SELECT `orders`.* FROM `orders` WHERE (`orders`.`charged_on` BETWEEN '2015-02-20 18:35:44.376956' AND '2015-02-20 18:35:44.376957') which causes test failures for us. |
@trobrock have you checked 4-2-stable? |
@rafaelfranca I saw your PR in the changelog at the 4.2.0 tag so I assumed it was in there, let me try the branch. |
@rafaelfranca looks like 4-2-stable fixed it, but according to this https://github.com/rails/rails/blob/v4.2.0/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb#L77 the fix should be in 4.2.0 that is on rubygems correct? |
Not really. The fix was in another place. |
@rafaelfranca ahh, do you have a link to that fix so I can track when it makes it to the gem? Until then I'll use 4-2-stable, thanks for the help. |
No, I don't have it. It will make into the gem in 4.2.1 |
@rafaelfranca @miyagawa thanks for the help. |
You will also need mysql2 gem >= 0.3.12 for DATETIME and >= 0.3.18 for TIME fields. (A bug in argument count caused us to fail to select milliseconds from a TIME field into Ruby Time object prior to the 0.3.18 release). |
Common methods in both mysql adapters are should be added to `AbstractMysqlAdapter`, but some methods had been added to `Mysql2Adapter`. (8744632, 0306f82, rails#14359) Some methods already moved from `Mysql2Adapter` to `AbstractMysqlAdapter`. (rails#17601, rails#17998) Common methods in both mysql adapters are remaining only the `explain` method in `Mysql2Adapter`.
Adding microseconds support for mysql 5.6
This should not affect mysql 5.5 so IMO we dont need a version check.
this is PR #8240 + tests and Changelog entry.
review @rafaelfranca @jeremy @tenderlove