Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Rails 3.0.0 add_index fails silently on invalid index names

#5645

If add_index is called with a name that is too long, the migration appears to be successful but the index is not created.

The error is very easy to reproduce. Create a new rails project, then run the following migration. After execution the table will be created but the index will not.

class TestOfAddIndex < ActiveRecord::Migration
  def self.up
    create_table :blah do |t|
      t.string :name
    end
    add_index :blah, [:name], :name => "a really long name that is too long for the database to handle and should cause an error"
  end
  def self.down
  end
end

The output of the migration is:

$ rake db:migrate
(in /Volumes/projects/rails/add_index_test)
==  TestOfAddIndex: migrating =================================================
-- create_table(:blah)
   -> 0.0016s
-- add_index(:blah, [:name], {:name=>"a really long name that is too long for the database to handle and should cause an error"})
   -> 0.0002s
==  TestOfAddIndex: migrated (0.0020s) ========================================

And the resulting schema.rb is:

ActiveRecord::Schema.define(:version => 20100917024939) do
  create_table "blah", :force => true do |t|
    t.string "name"
  end
end

I have attached a patch that will fix the problem.

Reported by Tim Connor · September 17th, 2010 @ 04:08 AM

State: resolved
Milestone: none
Assigned to: José Valim José Valim
Importance: Low

Activity

  1. Tim Connor
    Tim Connor

    This issue also applies to index names automatically generated by rails, eg

    add_index :users, [:long_column_name, :other_long_column_name, :third_long_column_name], :unique => true
    

    will appear to work but the index will not be created as the generated name was too long

    September 17th, 2010 @ 04:14 AM

  2. Lake
    Lake

    Thanks for you contribution.

    Unfortunately, your patch actually breaks a couple of tests. I recommend that you check out those tests and try to resolve them.

      1) Failure:
    test_add_index_length_limit(MigrationTest) [test/cases/migration_test.rb:130]:
    Exception raised:
    <#<ArgumentError: Index name 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx' on table 'people' is too long; the limit is 64 characters>>.
    
      2) Failure:
    test_double_add_index(MigrationTest) [test/cases/migration_test.rb:157]:
    Exception raised:
    <#<ArgumentError: Index name 'some_idx' on table 'people' already exists>>.
    
    126 tests, 343 assertions, 2 failures, 0 errors, 0 skips
    

    September 17th, 2010 @ 04:48 AM

  3. Tim Connor
    Tim Connor

    Attached is a second patch which fixes a further issue with remove_index and updates the unit tests.

    September 17th, 2010 @ 05:38 AM

  4. Lake
    Lake

    Great! With your patches combined, the tests pass.

    +1

    September 18th, 2010 @ 01:51 AM

  5. Rohit Arondekar
    Rohit Arondekar
    • State changed from new to open
    • Assigned user set to José Valim
    • Importance changed from to Low

    September 18th, 2010 @ 02:49 AM

  6. Repository
  7. Repository