Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Migration rollback fails for add_index with custom name #8858

Closed
nathansamson opened this Issue · 8 comments

3 participants

@nathansamson

I have a migration with the following content:

def change
...
add_index(:very_long_table_name_and_i_really_mean_very_long, [:column_a, :column_b], name: 'index_shorter_name')
...
end

(The custom name was necessary because I got a "Index name 'index_very_long_table_name_and_i_really_mean_very_long on table 'very_long_table_name_and_i_really_mean_very_long is too long; the limit is 64 characters" error)

Migration succceded. in db/scheme.rb I see a
`add_index "very_long_table_name_and_i_really_mean_very_long", ['column_a', 'column_b'], name: "index_shorter_name", unique: true

In development I decided to rollback the migration (because I wanted to change it slightly) and I got a error:

Index name 'index_very_long_table_name_and_i_really_mean_very_long on table 'very_long_table_name_and_i_really_mean_very_long' does not exist

Obviosuly the error message is true, that index indeed doesn't exist. But actually it should be deleting 'index_shorther_name'.
The remove_index (opposite of add_index) seems to ignore the name: parameter, which it obviously shouln't ignore.

I am using rails 4 (from master git, updated right before posting this comment)

@tehgeekmeister

I'm looking into this. Seems like something I might be able to figure out. What database are you using?

@tehgeekmeister

Okay, I can reproduce it with sqlite and current HEAD. Looking into it more. Worst case, I'll make an example app so someone with some more knowledge about rails can dive in easily.

@tehgeekmeister

Here's a stacktrace for posterity:

Ezekiels-MacBook-Pro:/Users/tehgeekmeister/code/write_some| rake db:rollback
D, [2013-01-09T23:51:27.687491 #56053] DEBUG -- :   ActiveRecord::SchemaMigration Load (1.0ms)  SELECT "schema_migrations".* FROM "schema_migrations" 
D, [2013-01-09T23:51:27.690562 #56053] DEBUG -- :   ActiveRecord::SchemaMigration Load (0.1ms)  SELECT "schema_migrations".* FROM "schema_migrations" 
I, [2013-01-09T23:51:27.690822 #56053]  INFO -- : Migrating to IndexAnThing (20130110074946)
D, [2013-01-09T23:51:27.690988 #56053] DEBUG -- :    (0.1ms)  begin transaction
==  IndexAnThing: reverting ===================================================
-- remove_index(:very_long_table_name_and_i_really_mean_very_longs, {:name=>"index_shorter_name", :column=>:foo})
D, [2013-01-09T23:51:27.714005 #56053] DEBUG -- :    (0.0ms)  rollback transaction
rake aborted!
An error has occurred, this and all later migrations canceled:

Index name 'index_very_long_table_name_and_i_really_mean_very_longs_on_foo' on table 'very_long_table_name_and_i_really_mean_very_longs' does not exist
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb:685:in `index_name_for_remove'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb:432:in `remove_index'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:595:in `block in method_missing'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:567:in `block in say_with_time'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:567:in `say_with_time'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:587:in `method_missing'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:432:in `block in revert'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:431:in `each'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:431:in `revert'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:539:in `exec_migration'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:525:in `block (2 levels) in migrate'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:524:in `block in migrate'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:302:in `with_connection'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:523:in `migrate'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:666:in `migrate'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:860:in `block (2 levels) in migrate'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:940:in `block in ddl_transaction'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:201:in `block in transaction'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:209:in `within_new_transaction'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:201:in `transaction'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/transactions.rb:209:in `transaction'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:940:in `ddl_transaction'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:859:in `block in migrate'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:855:in `each'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:855:in `migrate'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:718:in `down'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:800:in `move'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/migration.rb:700:in `rollback'
/Users/tehgeekmeister/code/rails/activerecord/lib/active_record/railties/databases.rake:132:in `block (2 levels) in <top (required)>'
/Users/tehgeekmeister/.rvm/gems/ruby-1.9.3-p327/bin/ruby_noexec_wrapper:14:in `eval'
/Users/tehgeekmeister/.rvm/gems/ruby-1.9.3-p327/bin/ruby_noexec_wrapper:14:in `<main>'
Tasks: TOP => db:rollback
(See full trace by running task with --trace)
@tehgeekmeister

It looks like it goes wrong here.

My impression is that it expects to only ever hit the name case when there is no column name specified, which is causing the problem. I'm still tracing down precisely where that column option gets set, though.

@tehgeekmeister

Okay, between the previous link and here, what happens is that column always gets set, and since name is only used in reversion if column isn't set, it seems that the name option will never be used for removing an index if the command recorder is being used.

@tehgeekmeister

I've got a fix worked up, though I'm sure something of it will need changing to get merged, if my approach is considered acceptable at all. Pull request incoming!

@tehgeekmeister tehgeekmeister referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@tehgeekmeister

Will this need a changelog entry?

@tehgeekmeister tehgeekmeister referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@tehgeekmeister tehgeekmeister referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@tehgeekmeister tehgeekmeister referenced this issue from a commit in tehgeekmeister/rails
@tehgeekmeister tehgeekmeister If an index can't be found by column, use the index name.
schema_statements uses the column name by default to construct the index name, and then raises an exception if it doesn't exist, even if the name option is specified, which causes #8858.  this commit makes index_name_for_remove fall back to constructing the index name to remove based on the name option.
b6226c3
@rafaelfranca
Owner

Closed by #8868

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.