Skip to content

Commit 3682aa1

Browse files
committed
Fix Rails generated index name being too long
This updates the index name generation to always create a valid index name if one is not passed by the user. Set the limit to 62 bytes to ensure it works for the default configurations of Sqlite, mysql & postgres. MySQL: 64 Postgres: 63 Sqlite: 62 When over the limit, we fallback to a "short format" that includes a hash to guarantee uniqueness in the generated index name.
1 parent cf80ede commit 3682aa1

File tree

5 files changed

+92
-1
lines changed

5 files changed

+92
-1
lines changed

activerecord/CHANGELOG.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,24 @@
1+
* Limit max length of auto generated index names
2+
3+
Auto generated index names are now limited to 62 bytes, which fits within
4+
the default index name length limits for MySQL, Postgres and SQLite.
5+
6+
Any index name over the limit will fallback to the new short format.
7+
8+
Before (too long):
9+
```
10+
index_testings_on_foo_and_bar_and_first_name_and_last_name_and_administrator
11+
```
12+
13+
After (short format):
14+
```
15+
ix_on_foo_bar_first_name_last_name_administrator_5939248142
16+
```
17+
18+
The short format includes a hash to ensure the name is unique database-wide.
19+
20+
*Mike Coutermarsh*
21+
122
* Introduce a more stable and optimized Marshal serializer for Active Record models.
223
324
Can be enabled with `config.active_record.marshalling_format_version = 7.1`.

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -962,7 +962,7 @@ def rename_index(table_name, old_name, new_name)
962962
def index_name(table_name, options) # :nodoc:
963963
if Hash === options
964964
if options[:column]
965-
"index_#{table_name}_on_#{Array(options[:column]) * '_and_'}"
965+
generate_index_name(table_name, options[:column])
966966
elsif options[:name]
967967
options[:name]
968968
else
@@ -1509,7 +1509,26 @@ def valid_primary_key_options # :nodoc:
15091509
[:limit, :default, :precision]
15101510
end
15111511

1512+
# Returns the maximum length of an index name in bytes.
1513+
def max_index_name_size
1514+
62
1515+
end
1516+
15121517
private
1518+
def generate_index_name(table_name, column)
1519+
name = "index_#{table_name}_on_#{Array(column) * '_and_'}"
1520+
return name if name.bytesize <= max_index_name_size
1521+
1522+
# Fallback to short version, add hash to ensure uniqueness
1523+
hashed_identifier = "_" + OpenSSL::Digest::SHA256.hexdigest(name).first(10)
1524+
name = "ix_on_#{Array(column) * '_'}"
1525+
1526+
short_limit = max_index_name_size - hashed_identifier.bytesize
1527+
short_name = name.mb_chars.limit(short_limit).to_s
1528+
1529+
"#{short_name}#{hashed_identifier}"
1530+
end
1531+
15131532
def validate_change_column_null_argument!(value)
15141533
unless value == true || value == false
15151534
raise ArgumentError, "change_column_null expects a boolean value (true for NULL, false for NOT NULL). Got: #{value.inspect}"

activerecord/lib/active_record/migration/compatibility.rb

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ def add_column(table_name, column_name, type, **options)
5454
super
5555
end
5656

57+
def add_index(table_name, column_name, **options)
58+
options[:name] = legacy_index_name(table_name, column_name) if options[:name].nil?
59+
super
60+
end
5761

5862
def create_table(table_name, **options)
5963
options[:_uses_legacy_table_name] = true
@@ -105,6 +109,32 @@ class << t
105109
end
106110
super
107111
end
112+
113+
def legacy_index_name(table_name, options)
114+
if Hash === options
115+
if options[:column]
116+
"index_#{table_name}_on_#{Array(options[:column]) * '_and_'}"
117+
elsif options[:name]
118+
options[:name]
119+
else
120+
raise ArgumentError, "You must specify the index name"
121+
end
122+
else
123+
legacy_index_name(table_name, index_name_options(options))
124+
end
125+
end
126+
127+
def index_name_options(column_names)
128+
if expression_column_name?(column_names)
129+
column_names = column_names.scan(/\w+/).join("_")
130+
end
131+
132+
{ column: column_names }
133+
end
134+
135+
def expression_column_name?(column_name)
136+
column_name.is_a?(String) && /\W/.match?(column_name)
137+
end
108138
end
109139

110140
class V6_1 < V7_0

activerecord/test/cases/migration/compatibility_test.rb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,22 @@ def migrate(x)
550550
connection.drop_table :more_testings rescue nil
551551
end
552552

553+
def test_add_index_errors_on_too_long_name_7_0
554+
migration = Class.new(ActiveRecord::Migration[7.0]) {
555+
def migrate(x)
556+
add_column :testings, :very_long_column_name_to_test_with, :string
557+
add_index :testings, [:foo, :bar, :very_long_column_name_to_test_with]
558+
end
559+
}.new
560+
561+
error = assert_raises(StandardError) do
562+
ActiveRecord::Migrator.new(:up, [migration], @schema_migration, @internal_metadata).migrate
563+
end
564+
565+
assert_match(/index_testings_on_foo_and_bar_and_very_long_column_name_to_test_with/i, error.message)
566+
assert_match(/is too long/i, error.message)
567+
end
568+
553569
def test_add_reference_on_6_0
554570
create_migration = Class.new(ActiveRecord::Migration[6.0]) {
555571
def version; 100 end

activerecord/test/cases/migration/index_test.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,11 @@ def test_add_index_with_if_not_exists_matches_exact_index
103103
assert connection.index_name_exists?(table_name, "index_testings_on_foo_and_bar")
104104
end
105105

106+
def test_add_index_fallback_to_short_name
107+
connection.add_index(table_name, [:foo, :bar, :first_name, :last_name, :administrator])
108+
assert connection.index_name_exists?(table_name, "ix_on_foo_bar_first_name_last_name_administrator_5939248142")
109+
end
110+
106111
def test_remove_index_which_does_not_exist_doesnt_raise_with_option
107112
connection.add_index(table_name, "foo")
108113

0 commit comments

Comments
 (0)