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

rake db:test:purge creates mysql database with wrong charset & collation #1952

Merged
merged 1 commit into from Jul 6, 2011

Conversation

@simonbaird
Copy link
Contributor

@simonbaird simonbaird commented Jul 4, 2011

In db:test:purge ActiveRecord::Base.connection.recreate_database is called with the full database config options hash instead of the :charset and :collation hash that is expected.

This causes my test database to be created with the collation set incorrectly (I need utf8_bin) and my fixtures don't load and my tests fail. (Further explanation in comments).

See new method mysql_creation_options. It is used by both create_database and recreate_database so they are consistent.

See new method mysql_creation_options. It is used by both
create_database and recreate_database so they are consistent.
@simonbaird
Copy link
Contributor Author

@simonbaird simonbaird commented Jul 4, 2011

An alternative fix might be to change the options param here:
https://github.com/rails/rails/blob/master/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb#L510
eg,

 create_database(name, {:charset => options['charset'], :collation => options['collation']})

but that would ignore ENV['CHARSET'] and ENV['COLLATION']

@simonbaird
Copy link
Contributor Author

@simonbaird simonbaird commented Jul 5, 2011

More explanation

Imagine if you set charset and collation in config/database.yml. For this example lets use the test db:

...

test:
  adapter: mysql
  database: myapp_test
  username: root
  host: localhost
  charset: some_charset
  collation: some_collation

Now run

RAILS_ENV=test rake db:create

it calls create_database with these options:

{:charset=>"some_charset", :collation=>"some_collation"}

This is correct and creates the database with the correct charset and collation.

Now if you run

rake test

it calls recreate_database which then calls create_database with the following options, (ie the entire test db config from database.yml):

{"username"=>"root", "adapter"=>"mysql", "charset"=>"some_charset", "database"=>"myapp_test", "host"=>"localhost", "collation"=>"some_collation"}

Notice that it does have the collation and charset values there BUT the hash keys are symbols so there is no options[:collation] and no options[:charset] present. This causes the database to be created with the default charset of utf8 and an unspecified collation.

(See create_database method here:
https://github.com/rails/rails/blob/master/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb#L520 )

Impact

In my particular case using the default collation value causes the primary key constraint to consider primary keys as case insensitive, which means I can't load fixtures since I have some data where primary key constraints need to be case sensitive.

It's not hard to imagine other scenarios where having the test database created with a different charset and collation when running tests could lead to weirdness and problems.

The patch

It would be simple to patch recreate_database to work-around this (see comment above), but ideally we should respect the ENV['CHARSET'] and ENV['COLLATE'] values in the same way that rake db:create would do. So the fix is done in database.rake and involves a small refactor so create_database and recreate_database can work the same way.

In Rails 3.1

I have not confirmed it but from looking at the code I believe this problem exists in the 3.1 stable branch also. A port of this fix to 3.1 might be required if this doesn't merge cleanly.

@simonbaird
Copy link
Contributor Author

@simonbaird simonbaird commented Jul 6, 2011

Changed name to be more descriptive.

spastorino added a commit that referenced this pull request Jul 6, 2011
rake db:test:purge creates mysql database with wrong charset & collation
@spastorino spastorino merged commit 2f3eb7a into rails:3-0-stable Jul 6, 2011
@spastorino
Copy link
Member

@spastorino spastorino commented Jul 6, 2011

can you provide the same pull requests for 3-1-stable and master please?

@simonbaird
Copy link
Contributor Author

@simonbaird simonbaird commented Jul 6, 2011

Okay, done.

@simonbaird
Copy link
Contributor Author

@simonbaird simonbaird commented Jul 7, 2011

They are different commits but identical diffs.
#1989
#1990

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

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.