Properly quote index names in migrations (closes #4764) [John Long]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4239 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

activerecord/CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Properly quote index names in migrations (closes #4764) [John Long]
+
 * Fix the HasManyAssociation#count method so it uses the new ActiveRecord::Base#count syntax, while maintaining backwards compatibility.  [Rick]
 
 * Ensure that Associations#include_eager_conditions? checks both scoped and explicit conditions [Rick]

activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb

@@ -119,7 +119,7 @@ module ActiveRecord
       # Adds a new column to the named table.
       # See TableDefinition#column for details of the options you can use.
       def add_column(table_name, column_name, type, options = {})
-        add_column_sql = "ALTER TABLE #{table_name} ADD #{column_name} #{type_to_sql(type, options[:limit])}"
+        add_column_sql = "ALTER TABLE #{table_name} ADD #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit])}"
         add_column_options!(add_column_sql, options)
         execute(add_column_sql)
       end
@@ -128,7 +128,7 @@ module ActiveRecord
       # ===== Examples
       #  remove_column(:suppliers, :qualification)
       def remove_column(table_name, column_name)
-        execute "ALTER TABLE #{table_name} DROP #{column_name}"
+        execute "ALTER TABLE #{table_name} DROP #{quote_column_name(column_name)}"
       end
 
       # Changes the column's definition according to the new options.
@@ -184,7 +184,8 @@ module ActiveRecord
       # generates
       #  CREATE UNIQUE INDEX by_branch_party ON accounts(branch_id, party_id)
       def add_index(table_name, column_name, options = {})
-        index_name = "#{table_name}_#{Array(column_name).first}_index"
+        column_names = Array(column_name)
+        index_name   = index_name(table_name, :column => column_names.first)
 
         if Hash === options # legacy support, since this param was a string
           index_type = options[:unique] ? "UNIQUE" : ""
@@ -192,8 +193,8 @@ module ActiveRecord
         else
           index_type = options
         end
-
-        execute "CREATE #{index_type} INDEX #{index_name} ON #{table_name} (#{Array(column_name).join(", ")})"
+        quoted_column_names = column_names.map { |e| quote_column_name(e) }.join(", ")
+        execute "CREATE #{index_type} INDEX #{quote_column_name(index_name)} ON #{table_name} (#{quoted_column_names})"
       end
 
       # Remove the given index from the table.
@@ -209,7 +210,7 @@ module ActiveRecord
       #   add_index :accounts, [:username, :password]
       #   remove_index :accounts, :username
       def remove_index(table_name, options = {})
-        execute "DROP INDEX #{index_name(table_name, options)} ON #{table_name}"
+        execute "DROP INDEX #{quote_column_name(index_name(table_name, options))} ON #{table_name}"
       end
 
       def index_name(table_name, options) #:nodoc:

activerecord/lib/active_record/connection_adapters/mysql_adapter.rb

@@ -18,7 +18,6 @@ module ActiveRecord
         end
       end
 
-
       config = config.symbolize_keys
       host     = config[:host]
       port     = config[:port]

activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

@@ -337,8 +337,7 @@ module ActiveRecord
 
       def remove_index(table_name, options) #:nodoc:
         execute "DROP INDEX #{index_name(table_name, options)}"
-      end      
-
+      end
 
       private
         BYTEA_COLUMN_TYPE_OID = 17

activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb

@@ -213,13 +213,7 @@ module ActiveRecord
       end
 
       def remove_index(table_name, options={}) #:nodoc:
-        if Hash === options
-          index_name = options[:name]
-        else
-          index_name = "#{table_name}_#{options}_index"
-        end
-
-        execute "DROP INDEX #{index_name}"
+        execute "DROP INDEX #{quote_column_name(index_name(table_name, options))}"
       end
       
       def rename_table(name, new_name)

activerecord/test/migration_test.rb

@@ -34,6 +34,7 @@ if ActiveRecord::Base.connection.supports_migrations?
       Reminder.reset_column_information
 
       Person.connection.remove_column("people", "last_name") rescue nil
+      Person.connection.remove_column("people", "key") rescue nil
       Person.connection.remove_column("people", "bio") rescue nil
       Person.connection.remove_column("people", "age") rescue nil
       Person.connection.remove_column("people", "height") rescue nil
@@ -47,12 +48,17 @@ if ActiveRecord::Base.connection.supports_migrations?
     def test_add_index
       Person.connection.add_column "people", "last_name", :string        
       Person.connection.add_column "people", "administrator", :boolean
+      Person.connection.add_column "people", "key", :string
       
       assert_nothing_raised { Person.connection.add_index("people", "last_name") }
       assert_nothing_raised { Person.connection.remove_index("people", "last_name") }
 
       assert_nothing_raised { Person.connection.add_index("people", ["last_name", "first_name"]) }
       assert_nothing_raised { Person.connection.remove_index("people", "last_name") }
+      
+      # quoting
+      assert_nothing_raised { Person.connection.add_index("people", ["key"], :name => "key", :unique => true) }
+      assert_nothing_raised { Person.connection.remove_index("people", :name => "key") }
 
       # Sybase adapter does not support indexes on :boolean columns
       unless current_adapter?(:SybaseAdapter)