janko / rodauth-rails

Rails integration for Rodauth authentication framework
https://github.com/jeremyevans/rodauth
MIT License
565 stars 40 forks source link

Autogenerated migration issue with Rails 7, MySQL and CURRENT_TIMESTAMP #159

Closed adamlofting closed 1 year ago

adamlofting commented 1 year ago

Thanks for all the great work on rodauth-rails.

I've just tried an install on a fresh Rails 7 app using MySQL and caught one small issue, which I've solved but thought it might be helpful to share here.

In the autogenerated migrations for the accounts table (db/migrate/xxx_create_rodauth.rb), there are 3 fields that use the CURRENT_TIMESTAMP as the default value for datetime fields.

When running rails db:migrate in Rails 7, this generates an error :

ActiveRecord::StatementInvalid: Mysql2::Error: Invalid default value for xxxxxxxxx

I found the solution discussed here on the rails repo: https://github.com/rails/rails/issues/43292

I was able to make the rodauth-rails migrations work by changing the examples like this:

t.datetime :email_last_sent, null: false, default: -> { "CURRENT_TIMESTAMP" }

to:

t.datetime :email_last_sent, null: false, precision: nil, default: -> { "CURRENT_TIMESTAMP()" }
janko commented 1 year ago

Thanks for reporting the issue. For some reason I wasn't able to reproduce the issue on Rails 7.0.4 and MySQL 8.0.30, for me the migration succeeds, and I can create records that rely on this default value. What versions are you using?

adamlofting commented 1 year ago

No worries, and that's interesting.

I'm running:

I don't think this is important, but kust in case it helps, I was running rails generate rodauth:install --jwt

janko commented 1 year ago

OK, so the only difference is the minor version of MySQL, I don't know if the recent version happens to fix this issue 🤷🏻‍♂️. I'm on MacBook M1 (Apple Silicon), I don't know if that makes any difference. You're able to reproduce this in a fresh Rails app?

adamlofting commented 1 year ago

Yes, it's very strange 🧐

For the sake of ruling things out, I've just created a completely fresh Rails App and was able to repro this again.

I then updated MySQL to 8.0.30 and was still able to repro this (notes below on the commands I ran).

It looks like the only other difference is that I'm running an Intel MacBook.

Screenshot 2022-09-19 at 14 51 27
$ rails new roaduth-rails-repro --database=mysql

Add to gemfile:

gem 'rodauth-rails', '~> 1.6'
$ bundle install
$ rails generate rodauth:install --jwt
$ bundle add jwt
$ bin/rails db:create
$ rails db:migrate
== 20220919133647 CreateRodauth: migrating ====================================
-- create_table(:accounts)
   -> 0.0132s
-- create_table(:account_password_reset_keys, {:id=>false})
rails aborted!
StandardError: An error has occurred, all later migrations canceled:

Mysql2::Error: Invalid default value for 'email_last_sent'
/Users/adamlofting/code/roaduth-rails-repro/db/migrate/20220919133647_create_rodauth.rb:11:in `change'

Caused by:
ActiveRecord::StatementInvalid: Mysql2::Error: Invalid default value for 'email_last_sent'
/Users/adamlofting/code/roaduth-rails-repro/db/migrate/20220919133647_create_rodauth.rb:11:in `change'

Caused by:
Mysql2::Error: Invalid default value for 'email_last_sent'
/Users/adamlofting/code/roaduth-rails-repro/db/migrate/20220919133647_create_rodauth.rb:11:in `change'
$ brew update
$ brew install mysql
$ mysql -V
mysql  Ver 8.0.30 for macos12.4 on x86_64 (Homebrew)

Then repeat the steps above with the same results.

janko commented 1 year ago

Thanks for trying it out. It seems there might be differences in MySQL installations, but the documentation clearly states that if a precision is specified for the column type, then the same precision needs to be used for the default value. Instead of adding precision: nil, and that way overriding Active Record's default, I think it would make more sense to use CURRENT_TIMESTAMP(6) for the default value. Could you test whether this works for you?

janko commented 1 year ago

I suspect my MySQL installation was reusing the database created for MariaDB, so I just re-created it locally, and also updated to latest macOS version, and I was able to finally reproduce the issue 🤘🏻

adamlofting commented 1 year ago

Excellent, thanks for responding so quickly.

I can also confirm that CURRENT_TIMESTAMP(6) works here too.

zavan commented 1 year ago

FYI I'm using the latest MySQL 5.7 and Rails 7 and had to use CURRENT_TIMESTAMP(6) for it to work.

janko commented 1 year ago

@zavan I see from your other activity that you're using the Trilogy adapter, which isn't currently being handled here (only mysql2). I can add it to the conditional.

zavan commented 1 year ago

@janko I had to go back to mysql2 since Sequel doesn't support trilogy, so no worries. Thanks!

janko commented 1 year ago

For anyone interested, the Trilogy adapter has landed in Sequel master, so it will be part of the next release.