Optimise length of default index name for polymorphic references

This reduces the possibility of generating index names that are too long
for certain databases (e.g. Postgres which has a 63 character limit) by
naming the index based on the reference name rather than the type and id
column names (the default behaviour of t.index).

This still allows the name to be explicitly specified by passing options.

activerecord/CHANGELOG.md

@@ -1,3 +1,13 @@
+*   Optimise the length of index names for polymorphic references by using the reference name rather than the type and id column names.
+
+    Because the default behaviour when adding an index with multiple columns is to use all column names in the index name, this could frequently lead to overly long index names for polymorphic references which would fail the migration if it exceeded the database limit.
+
+    This change reduces the chance of that happening by using the reference name, e.g. `index_my_table_on_my_reference`.
+
+    Fixes #38655.
+
+    *Luke Redpath*
+
 *   MySQL: Uniqueness validator now respects default database collation,
     no longer enforce case sensitive comparison by default.
 

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

@@ -169,7 +169,7 @@ module ActiveRecord
         end
 
         if index
-          table.index(column_names, **index_options)
+          table.index(column_names, **index_options(table.name))
         end
 
         if foreign_key
@@ -188,8 +188,14 @@ module ActiveRecord
           as_options(polymorphic).merge(options.slice(:null, :first, :after))
         end
 
-        def index_options
-          as_options(index)
+        def polymorphic_index_name(table_name)
+          "index_#{table_name}_on_#{name}"
+        end
+
+        def index_options(table_name)
+          index_options = as_options(index)
+          index_options[:name] ||= polymorphic_index_name(table_name) if polymorphic
+          index_options
         end
 
         def foreign_key_options

activerecord/lib/active_record/migration/compatibility.rb

@@ -16,6 +16,57 @@ module ActiveRecord
       V6_1 = Current
 
       class V6_0 < V6_1
+        class ReferenceDefinition < ConnectionAdapters::ReferenceDefinition
+          def index_options(table_name)
+            as_options(index)
+          end
+        end
+
+        module TableDefinition
+          def references(*args, **options)
+            args.each do |ref_name|
+              ReferenceDefinition.new(ref_name, **options).add_to(self)
+            end
+          end
+          alias :belongs_to :references
+        end
+
+        def create_table(table_name, **options)
+          if block_given?
+            super { |t| yield compatible_table_definition(t) }
+          else
+            super
+          end
+        end
+
+        def change_table(table_name, **options)
+          if block_given?
+            super { |t| yield compatible_table_definition(t) }
+          else
+            super
+          end
+        end
+
+        def create_join_table(table_1, table_2, **options)
+          if block_given?
+            super { |t| yield compatible_table_definition(t) }
+          else
+            super
+          end
+        end
+
+        def add_reference(table_name, ref_name, **options)
+          ReferenceDefinition.new(ref_name, **options).add_to(update_table_definition(table_name, self))
+        end
+        alias :add_belongs_to :add_reference
+
+        private
+          def compatible_table_definition(t)
+            class << t
+              prepend TableDefinition
+            end
+            t
+          end
       end
 
       class V5_2 < V6_0

activerecord/test/cases/migration/compatibility_test.rb

@@ -269,6 +269,72 @@ module ActiveRecord
         end
       end
 
+      def test_create_table_with_polymorphic_reference_uses_all_column_names_in_index
+        migration = Class.new(ActiveRecord::Migration[6.0]) {
+          def migrate(x)
+            create_table :more_testings do |t|
+              t.references :widget, polymorphic: true, index: true
+              t.belongs_to :gizmo, polymorphic: true, index: true
+            end
+          end
+        }.new
+
+        ActiveRecord::Migrator.new(:up, [migration], @schema_migration).migrate
+
+        assert connection.index_exists?(:more_testings, [:widget_type, :widget_id], name: :index_more_testings_on_widget_type_and_widget_id)
+        assert connection.index_exists?(:more_testings, [:gizmo_type, :gizmo_id], name: :index_more_testings_on_gizmo_type_and_gizmo_id)
+      ensure
+        connection.drop_table :more_testings rescue nil
+      end
+
+      def test_change_table_with_polymorphic_reference_uses_all_column_names_in_index
+        migration = Class.new(ActiveRecord::Migration[6.0]) {
+          def migrate(x)
+            change_table :testings do |t|
+              t.references :widget, polymorphic: true, index: true
+              t.belongs_to :gizmo, polymorphic: true, index: true
+            end
+          end
+        }.new
+
+        ActiveRecord::Migrator.new(:up, [migration], @schema_migration).migrate
+
+        assert connection.index_exists?(:testings, [:widget_type, :widget_id], name: :index_testings_on_widget_type_and_widget_id)
+        assert connection.index_exists?(:testings, [:gizmo_type, :gizmo_id], name: :index_testings_on_gizmo_type_and_gizmo_id)
+      end
+
+      def test_create_join_table_with_polymorphic_reference_uses_all_column_names_in_index
+        migration = Class.new(ActiveRecord::Migration[6.0]) {
+          def migrate(x)
+            create_join_table :more, :testings do |t|
+              t.references :widget, polymorphic: true, index: true
+              t.belongs_to :gizmo, polymorphic: true, index: true
+            end
+          end
+        }.new
+
+        ActiveRecord::Migrator.new(:up, [migration], @schema_migration).migrate
+
+        assert connection.index_exists?(:more_testings, [:widget_type, :widget_id], name: :index_more_testings_on_widget_type_and_widget_id)
+        assert connection.index_exists?(:more_testings, [:gizmo_type, :gizmo_id], name: :index_more_testings_on_gizmo_type_and_gizmo_id)
+      ensure
+        connection.drop_table :more_testings rescue nil
+      end
+
+      def test_polymorphic_add_reference_uses_all_column_names_in_index
+        migration = Class.new(ActiveRecord::Migration[6.0]) {
+          def migrate(x)
+            add_reference :testings, :widget, polymorphic: true, index: true
+            add_belongs_to :testings, :gizmo, polymorphic: true, index: true
+          end
+        }.new
+
+        ActiveRecord::Migrator.new(:up, [migration], @schema_migration).migrate
+
+        assert connection.index_exists?(:testings, [:widget_type, :widget_id], name: :index_testings_on_widget_type_and_widget_id)
+        assert connection.index_exists?(:testings, [:gizmo_type, :gizmo_id], name: :index_testings_on_gizmo_type_and_gizmo_id)
+      end
+
       private
         def precision_implicit_default
           if current_adapter?(:Mysql2Adapter)

activerecord/test/cases/migration/references_index_test.rb

@@ -57,7 +57,15 @@ module ActiveRecord
             t.references :foo, polymorphic: true, index: true
           end
 
-          assert connection.index_exists?(table_name, [:foo_type, :foo_id], name: :index_testings_on_foo_type_and_foo_id)
+          assert connection.index_exists?(table_name, [:foo_type, :foo_id], name: :index_testings_on_foo)
+        end
+
+        def test_creates_polymorphic_index_with_custom_name
+          connection.create_table table_name do |t|
+            t.references :foo, polymorphic: true, index: { name: :testings_foo_index }
+          end
+
+          assert connection.index_exists?(table_name, [:foo_type, :foo_id], name: :testings_foo_index)
         end
       end
 
@@ -95,7 +103,16 @@ module ActiveRecord
             t.references :foo, polymorphic: true, index: true
           end
 
-          assert connection.index_exists?(table_name, [:foo_type, :foo_id], name: :index_testings_on_foo_type_and_foo_id)
+          assert connection.index_exists?(table_name, [:foo_type, :foo_id], name: :index_testings_on_foo)
+        end
+
+        def test_creates_polymorphic_index_for_existing_table_with_custom_name
+          connection.create_table table_name
+          connection.change_table table_name do |t|
+            t.references :foo, polymorphic: true, index: { name: :testings_foo_index }
+          end
+
+          assert connection.index_exists?(table_name, [:foo_type, :foo_id], name: :testings_foo_index)
         end
       end
     end