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

Allow using Action Mailbox on MySQL 5.5 #34956

Merged
merged 1 commit into from
Jan 18, 2019

Conversation

kamipo
Copy link
Member

@kamipo kamipo commented Jan 17, 2019

Active Record still support MySQL 5.5 which doesn't support datetime
with precision.

def supports_datetime_with_precision?
mariadb? || version >= "5.6.4"
end

So we should check supports_datetime_with_precision? on the
connection.

@@ -4,8 +4,11 @@ def change
t.integer :status, default: 0, null: false
t.string :message_id

t.datetime :created_at, precision: 6, null: false
t.datetime :updated_at, precision: 6, null: false
if supports_datetime_with_precision?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we move this into the timestamps method itself? We can define a 6.0 migration version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the precision is for MySQL 5.6+, since datetime with no precision means unlimited precision for PostgreSQL and SQLite.
If we want explicit precision for the timestamps, I'll create new PR as new migration default in Active Record.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think the precision is for MySQL. Well, if we could make it omit the precision on Postgres/SQLite and only add it on MySQL if the server supports it, that seems ideal to me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just implemented #34970.

@kamipo kamipo force-pushed the actionmailbox_datetime_precision branch from f9e7c39 to 2d0bb87 Compare January 17, 2019 21:10
Active Record still support MySQL 5.5 which doesn't support datetime
with precision.

https://github.com/rails/rails/blob/9e34df00039d63b5672315419e76f06f80ef3dc4/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb#L99-L101

So we should check `supports_datetime_with_precision?` on the
connection.
@kamipo kamipo force-pushed the actionmailbox_datetime_precision branch from 2d0bb87 to db077e8 Compare January 17, 2019 22:04
@kamipo kamipo merged commit 9e4283e into rails:master Jan 18, 2019
@kamipo kamipo deleted the actionmailbox_datetime_precision branch January 18, 2019 00:57
kamipo added a commit that referenced this pull request Jan 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants