Merge pull request rails#26687 from kamipo/fix_add_index_to_normalize_options

Fix `add_index` to normalize column names and options

Author: matthewd

activerecord/CHANGELOG.md

@@ -1,3 +1,11 @@
+*   Support index length and order options using both string and symbol
+    column names.
+
+    Fixes #27243.
+
+    *Ryuta Kamizono*
+
+
 ## Rails 5.0.1.rc1 (December 01, 2016) ##
 
 *   Fix that unsigned with zerofill is treated as signed.

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

@@ -1169,6 +1169,7 @@ def add_index_sort_order(quoted_columns, **options)
           if order = options[:order]
             case order
             when Hash
+              order = order.symbolize_keys
               quoted_columns.each { |name, column| column << " #{order[name].upcase}" if order[name].present? }
             when String
               quoted_columns.each { |name, column| column << " #{order.upcase}" if order.present? }

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -716,6 +716,7 @@ def add_index_length(quoted_columns, **options)
         if length = options[:length]
           case length
           when Hash
+            length = length.symbolize_keys
             quoted_columns.each { |name, column| column << "(#{length[name]})" if length[name].present? }
           when Integer
             quoted_columns.each { |name, column| column << "(#{length})" }

activerecord/test/cases/adapters/mysql2/active_schema_test.rb

@@ -27,13 +27,16 @@ def (ActiveRecord::Base.connection).index_name_exists?(*); false; end
     assert_equal expected, add_index(:people, :last_name, :length => 10)
 
     expected = "CREATE  INDEX `index_people_on_last_name_and_first_name`  ON `people` (`last_name`(15), `first_name`(15)) "
-    assert_equal expected, add_index(:people, [:last_name, :first_name], :length => 15)
+    assert_equal expected, add_index(:people, [:last_name, :first_name], length: 15)
+    assert_equal expected, add_index(:people, ["last_name", "first_name"], length: 15)
 
     expected = "CREATE  INDEX `index_people_on_last_name_and_first_name`  ON `people` (`last_name`(15), `first_name`) "
-    assert_equal expected, add_index(:people, [:last_name, :first_name], :length => {:last_name => 15})
+    assert_equal expected, add_index(:people, [:last_name, :first_name], length: { last_name: 15 })
+    assert_equal expected, add_index(:people, ["last_name", "first_name"], length: { last_name: 15 })
 
     expected = "CREATE  INDEX `index_people_on_last_name_and_first_name`  ON `people` (`last_name`(15), `first_name`(10)) "
-    assert_equal expected, add_index(:people, [:last_name, :first_name], :length => {:last_name => 15, :first_name => 10})
+    assert_equal expected, add_index(:people, [:last_name, :first_name], length: { last_name: 15, first_name: 10 })
+    assert_equal expected, add_index(:people, ["last_name", :first_name], length: { last_name: 15, "first_name" => 10 })
 
     %w(SPATIAL FULLTEXT UNIQUE).each do |type|
       expected = "CREATE #{type} INDEX `index_people_on_last_name`  ON `people` (`last_name`) "

activerecord/test/cases/adapters/postgresql/active_schema_test.rb

@@ -39,6 +39,10 @@ def test_add_index
     expected = %(CREATE  INDEX CONCURRENTLY "index_people_on_last_name" ON "people"  ("last_name"))
     assert_equal expected, add_index(:people, :last_name, algorithm: :concurrently)
 
+    expected = %(CREATE  INDEX  "index_people_on_last_name_and_first_name" ON "people"  ("last_name" DESC, "first_name" ASC))
+    assert_equal expected, add_index(:people, [:last_name, :first_name], order: { last_name: :desc, first_name: :asc })
+    assert_equal expected, add_index(:people, ["last_name", :first_name], order: { last_name: :desc, "first_name" => :asc })
+
     %w(gin gist hash btree).each do |type|
       expected = %(CREATE  INDEX  "index_people_on_last_name" ON "people" USING #{type} ("last_name"))
       assert_equal expected, add_index(:people, :last_name, using: type)