Refactor `length`, `order`, and `opclass` index options dumping

6 files changed, 41 insertions, 27 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
index 88ef8e0819..86406d95ad 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -6,7 +6,7 @@ module ActiveRecord
     # this type are typically created and returned by methods in database
     # adapters. e.g. ActiveRecord::ConnectionAdapters::MySQL::SchemaStatements#indexes
     class IndexDefinition # :nodoc:
-      attr_reader :table, :name, :unique, :columns, :lengths, :orders, :where, :type, :using, :opclass, :comment
+      attr_reader :table, :name, :unique, :columns, :lengths, :orders, :opclasses, :where, :type, :using, :comment
 
       def initialize(
         table, name,
@@ -14,24 +14,33 @@ module ActiveRecord
         columns = [],
         lengths: {},
         orders: {},
+        opclasses: {},
         where: nil,
         type: nil,
         using: nil,
-        opclass: {},
         comment: nil
       )
         @table = table
         @name = name
         @unique = unique
         @columns = columns
-        @lengths = lengths
-        @orders = orders
+        @lengths = concise_options(lengths)
+        @orders = concise_options(orders)
+        @opclasses = concise_options(opclasses)
         @where = where
         @type = type
         @using = using
-        @opclass = opclass
         @comment = comment
       end
+
+      private
+        def concise_options(options)
+          if columns.size == options.size && options.values.uniq.size == 1
+            options.values.first
+          else
+            options
+          end
+        end
     end
 
     # Abstract representation of a column definition. Instances of this type
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
index fac9e1cca2..efc934c34c 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -1202,7 +1202,7 @@ module ActiveRecord
             when Hash
               order = order.symbolize_keys
               quoted_columns.each { |name, column| column << " #{order[name].upcase}" if order[name].present? }
-            when String
+            else
               quoted_columns.each { |name, column| column << " #{order.upcase}" if order.present? }
             end
           end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index 143dc6d08d..41c628302d 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -611,7 +611,7 @@ module ActiveRecord
             when Hash
               length = length.symbolize_keys
               quoted_columns.each { |name, column| column << "(#{length[name]})" if length[name].present? }
-            when Integer
+            else
               quoted_columns.each { |name, column| column << "(#{length})" }
             end
           end
diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb
index a15c7d1787..d5b2c18ee6 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb
@@ -22,23 +22,26 @@ module ActiveRecord
                   index_using = mysql_index_type
                 end
 
-                indexes << IndexDefinition.new(
+                indexes << [
                   row[:Table],
                   row[:Key_name],
                   row[:Non_unique].to_i == 0,
+                  [],
+                  lengths: {},
+                  orders: {},
                   type: index_type,
                   using: index_using,
                   comment: row[:Index_comment].presence
-                )
+                ]
               end
 
-              indexes.last.columns << row[:Column_name]
-              indexes.last.lengths.merge!(row[:Column_name] => row[:Sub_part].to_i) if row[:Sub_part]
-              indexes.last.orders.merge!(row[:Column_name] => :desc) if row[:Collation] == "D"
+              indexes.last[-2] << row[:Column_name]
+              indexes.last[-1][:lengths].merge!(row[:Column_name] => row[:Sub_part].to_i) if row[:Sub_part]
+              indexes.last[-1][:orders].merge!(row[:Column_name] => :desc) if row[:Collation] == "D"
             end
           end
 
-          indexes
+          indexes.map { |index| IndexDefinition.new(*index) }
         end
 
         def remove_column(table_name, column_name, type = nil, options = {})
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
index 64f08e0ee1..7b4d5711cb 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
@@ -109,6 +109,9 @@ module ActiveRecord
 
             using, expressions, where = inddef.scan(/ USING (\w+?) \((.+?)\)(?: WHERE (.+))?\z/).flatten
 
+            orders = {}
+            opclasses = {}
+
             if indkey.include?(0)
               columns = expressions
             else
@@ -119,21 +122,12 @@ module ActiveRecord
                 AND a.attnum IN (#{indkey.join(",")})
               SQL
 
-              orders = {}
-              opclasses = {}
-
               # add info on sort order (only desc order is explicitly specified, asc is the default)
               # and non-default opclasses
               expressions.scan(/(\w+)(?: (?!DESC)(\w+))?(?: (DESC))?/).each do |column, opclass, desc|
-                opclasses[column] = opclass if opclass
+                opclasses[column] = opclass.to_sym if opclass
                 orders[column] = :desc if desc
               end
-
-              # Use a string for the opclass description (instead of a hash) when all columns
-              # have the same opclass specified.
-              if columns.count == opclasses.count && opclasses.values.uniq.count == 1
-                opclasses = opclasses.values.first
-              end
             end
 
             IndexDefinition.new(
@@ -142,9 +136,9 @@ module ActiveRecord
               unique,
               columns,
               orders: orders,
+              opclasses: opclasses,
               where: where,
               using: using.to_sym,
-              opclass: opclasses,
               comment: comment.presence
             )
           end
diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb
index 8cb2851557..16ccba6b6c 100644
--- a/activerecord/lib/active_record/schema_dumper.rb
+++ b/activerecord/lib/active_record/schema_dumper.rb
@@ -184,12 +184,12 @@ HEADER
           "name: #{index.name.inspect}",
         ]
         index_parts << "unique: true" if index.unique
-        index_parts << "length: { #{format_options(index.lengths)} }" if index.lengths.present?
-        index_parts << "order: { #{format_options(index.orders)} }" if index.orders.present?
+        index_parts << "length: #{format_index_parts(index.lengths)}" if index.lengths.present?
+        index_parts << "order: #{format_index_parts(index.orders)}" if index.orders.present?
+        index_parts << "opclass: #{format_index_parts(index.opclasses)}" if index.opclasses.present?
         index_parts << "where: #{index.where.inspect}" if index.where
         index_parts << "using: #{index.using.inspect}" if !@connection.default_index_type?(index)
         index_parts << "type: #{index.type.inspect}" if index.type
-        index_parts << "opclass: #{index.opclass.inspect}" if index.opclass.present?
         index_parts << "comment: #{index.comment.inspect}" if index.comment
         index_parts
       end
@@ -232,6 +232,14 @@ HEADER
         options.map { |key, value| "#{key}: #{value.inspect}" }.join(", ")
       end
 
+      def format_index_parts(options)
+        if options.is_a?(Hash)
+          "{ #{format_options(options)} }"
+        else
+          options.inspect
+        end
+      end
+
       def remove_prefix_and_suffix(table)
         prefix = Regexp.escape(@options[:table_name_prefix].to_s)
         suffix = Regexp.escape(@options[:table_name_suffix].to_s)