Generated schema ignores id: false on mysql2 #3440

Closed
rmehner opened this Issue Oct 26, 2011 · 10 comments

Comments

Projects
None yet
4 participants
Contributor

rmehner commented Oct 26, 2011

Hi there,

using Rails 3.1.1 (on 1.9.2), when I have the following migration

class CreateCodes < ActiveRecord::Migration
  def change
    create_table :codes, id: false do |t|
      t.string :key, null: false
    end

    add_index :codes, :key, unique: true
  end
end

I would expect the following schema

create_table "codes", :id => false, :force => true do |t|
  t.string "key", :null => false
end

add_index "codes", ["key"], :name => "index_codes_on_key", :unique => true

but what I really get is:

create_table "codes", :primary_key => "key", :force => true do |t|
end

add_index "codes", ["key"], :name => "index_codes_on_key", :unique => true

Note that this does not happen using sqlite3, but when using mysql2 (0.3.7) and mysql (2.8.1)

I really don't know if this is a Rails bug or more on the mysql driver side so I'm happy to take some pointers.

If there is any information missing please let me know

Thank you in advance for looking into it,

  • Robin
@ghost

ghost commented Oct 26, 2011

+1

Contributor

rmehner commented Oct 27, 2011

Just a short addition:

When I leave out the add_index or the null: false then it works as expected.

Thanks again,

  • Robin
Contributor

kennyj commented Oct 30, 2011

I looked into the specifications of mysql's describe command.

$ mysql -u kennyj
Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 22
Server version: 5.0.77 Source distribution

mysql> create database foo;
Query OK, 1 row affected (0.00 sec)

mysql> use foo;
Database changed

mysql> create table bar (
    ->   baz varchar(255) not null
    -> );
Query OK, 0 rows affected (0.02 sec)

mysql> alter table bar add unique (baz);
Query OK, 0 rows affected (0.04 sec)
Records: 0  Duplicates: 0  Warnings: 0

mysql> desc bar;
+-------+--------------+------+-----+---------+-------+
| Field | Type         | Null | Key | Default | Extra |
+-------+--------------+------+-----+---------+-------+
| baz   | varchar(255) | NO   | PRI | NULL    |       |
+-------+--------------+------+-----+---------+-------+
1 row in set (0.00 sec)

By finding "PRI", rails are looking for a primary key column.
Please see https://github.com/rails/rails/blob/master/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb#L500
I think that this is the cause of this problem.

But, mysql seems to distinguish the unique key and primary key properly.

mysql> show create table bar;
+-------+---------------------------------------------------------------------------------------------------------------------+
| Table | Create Table                                                                                                        |
+-------+---------------------------------------------------------------------------------------------------------------------+
| bar   | CREATE TABLE `bar` (
  `baz` varchar(255) NOT NULL,
  UNIQUE KEY `baz` (`baz`)
) ENGINE=MyISAM DEFAULT CHARSET=utf8 |
+-------+---------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)

This problem may be able to fix it :)

Contributor

kennyj commented Oct 30, 2011

I have thought the first draft.
https://gist.github.com/1326131

Contributor

kennyj commented Oct 31, 2011

@rmehner
By the above commit, it's work fine on my environment.
Can you test this commit?

Contributor

rmehner commented Oct 31, 2011

@kennyj

Thanks for your work. While it works now in the direct migration (no id field created, correct primary key etc.), the generated schema is still wrong.

The above mentioned migration now yields the following schema.rb:

create_table "codes", :primary_key => "key", :force => true do |t|
end

add_index "codes", ["key"], :name => "index_codes_on_key", :unique => true

as you can see the type information for the column 'key' (string) is lost then.

So we're very close. Thank you for your work.

Greetings,

  • Robin
Contributor

kennyj commented Nov 1, 2011

@rmehner

For me, It's work fine (edge rails with ruby 1.9.2-p180).
It is strange :-(

My confirmation step is

$ git clone git://github.com/kennyj/rails.git
$ cd rails
$ git checkout -b fix_3440-1 origin/fix_3440-1 # with my fix.
$ cd ..
$ rails/railties/bin/rails new foo -d mysql
$ cd foo
$ (edit Gemfile)

gem 'rails', '3.2.0.beta', :path => '/path/to/rails'
gem 'journey', :git => 'git://github.com/rails/journey.git'

$ bundle install
$ ../rails/railties/bin/rails g model Code
$ (edit db/migrate/20111101154946_create_codes.rb)

class CreateCodes < ActiveRecord::Migration
  def change
    create_table :codes, id: false do |t|
      t.string :key, null: false
    end

    add_index :codes, :key, unique: true
  end
end

$ bundle exec rake db:create:all
$ bundle exec rake db:migrate
$ cat db/schema.rb
...
ActiveRecord::Schema.define(:version => 20111101154946) do

  create_table "codes", :id => false, :force => true do |t|
    t.string "key", :null => false
  end

  add_index "codes", ["key"], :name => "index_codes_on_key", :unique => true

end

★ Good :-)

$ cd ../rails
$ git checkout master # without my fix.
$ cd -
$ bundle exec rake db:drop
$ bundle exec rake db:create:all
$ bundle exec rake db:migrate
$ cat db/schema.rb
...
ActiveRecord::Schema.define(:version => 20111101154946) do

  create_table "codes", :primary_key => "key", :force => true do |t|
  end

  add_index "codes", ["key"], :name => "index_codes_on_key", :unique => true

end

★ Bad

Contributor

kennyj commented Nov 4, 2011

/cc @tenderlove @jonleighton

I confirm it again, but it's work fine (for me).
edge rails / ruby 1.9.2-p180 / mysql 5.0.77

Someone, please test the above steps?

Contributor

rmehner commented Nov 4, 2011

Tested. It works as expected now.

Thank you very much!

Contributor

kennyj commented Nov 5, 2011

Thanks for your confrimation.
I'll send pull request.

@kennyj kennyj added a commit to kennyj/rails that referenced this issue Nov 5, 2011

@kennyj kennyj Barckport to 3-1-stable: fixed an issue id false option is ignored on…
… mysql/mysql2 (fix #3440)
fb73423
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment