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.
This commit is contained in:
parent
cf80ede036
commit
3682aa1016
@ -1,3 +1,24 @@
|
||||
* Limit max length of auto generated index names
|
||||
|
||||
Auto generated index names are now limited to 62 bytes, which fits within
|
||||
the default index name length limits for MySQL, Postgres and SQLite.
|
||||
|
||||
Any index name over the limit will fallback to the new short format.
|
||||
|
||||
Before (too long):
|
||||
```
|
||||
index_testings_on_foo_and_bar_and_first_name_and_last_name_and_administrator
|
||||
```
|
||||
|
||||
After (short format):
|
||||
```
|
||||
ix_on_foo_bar_first_name_last_name_administrator_5939248142
|
||||
```
|
||||
|
||||
The short format includes a hash to ensure the name is unique database-wide.
|
||||
|
||||
*Mike Coutermarsh*
|
||||
|
||||
* Introduce a more stable and optimized Marshal serializer for Active Record models.
|
||||
|
||||
Can be enabled with `config.active_record.marshalling_format_version = 7.1`.
|
||||
|
@ -962,7 +962,7 @@ def rename_index(table_name, old_name, new_name)
|
||||
def index_name(table_name, options) # :nodoc:
|
||||
if Hash === options
|
||||
if options[:column]
|
||||
"index_#{table_name}_on_#{Array(options[:column]) * '_and_'}"
|
||||
generate_index_name(table_name, options[:column])
|
||||
elsif options[:name]
|
||||
options[:name]
|
||||
else
|
||||
@ -1509,7 +1509,26 @@ def valid_primary_key_options # :nodoc:
|
||||
[:limit, :default, :precision]
|
||||
end
|
||||
|
||||
# Returns the maximum length of an index name in bytes.
|
||||
def max_index_name_size
|
||||
62
|
||||
end
|
||||
|
||||
private
|
||||
def generate_index_name(table_name, column)
|
||||
name = "index_#{table_name}_on_#{Array(column) * '_and_'}"
|
||||
return name if name.bytesize <= max_index_name_size
|
||||
|
||||
# Fallback to short version, add hash to ensure uniqueness
|
||||
hashed_identifier = "_" + OpenSSL::Digest::SHA256.hexdigest(name).first(10)
|
||||
name = "ix_on_#{Array(column) * '_'}"
|
||||
|
||||
short_limit = max_index_name_size - hashed_identifier.bytesize
|
||||
short_name = name.mb_chars.limit(short_limit).to_s
|
||||
|
||||
"#{short_name}#{hashed_identifier}"
|
||||
end
|
||||
|
||||
def validate_change_column_null_argument!(value)
|
||||
unless value == true || value == false
|
||||
raise ArgumentError, "change_column_null expects a boolean value (true for NULL, false for NOT NULL). Got: #{value.inspect}"
|
||||
|
@ -54,6 +54,10 @@ def add_column(table_name, column_name, type, **options)
|
||||
super
|
||||
end
|
||||
|
||||
def add_index(table_name, column_name, **options)
|
||||
options[:name] = legacy_index_name(table_name, column_name) if options[:name].nil?
|
||||
super
|
||||
end
|
||||
|
||||
def create_table(table_name, **options)
|
||||
options[:_uses_legacy_table_name] = true
|
||||
@ -105,6 +109,32 @@ class << t
|
||||
end
|
||||
super
|
||||
end
|
||||
|
||||
def legacy_index_name(table_name, options)
|
||||
if Hash === options
|
||||
if options[:column]
|
||||
"index_#{table_name}_on_#{Array(options[:column]) * '_and_'}"
|
||||
elsif options[:name]
|
||||
options[:name]
|
||||
else
|
||||
raise ArgumentError, "You must specify the index name"
|
||||
end
|
||||
else
|
||||
legacy_index_name(table_name, index_name_options(options))
|
||||
end
|
||||
end
|
||||
|
||||
def index_name_options(column_names)
|
||||
if expression_column_name?(column_names)
|
||||
column_names = column_names.scan(/\w+/).join("_")
|
||||
end
|
||||
|
||||
{ column: column_names }
|
||||
end
|
||||
|
||||
def expression_column_name?(column_name)
|
||||
column_name.is_a?(String) && /\W/.match?(column_name)
|
||||
end
|
||||
end
|
||||
|
||||
class V6_1 < V7_0
|
||||
|
@ -550,6 +550,22 @@ def migrate(x)
|
||||
connection.drop_table :more_testings rescue nil
|
||||
end
|
||||
|
||||
def test_add_index_errors_on_too_long_name_7_0
|
||||
migration = Class.new(ActiveRecord::Migration[7.0]) {
|
||||
def migrate(x)
|
||||
add_column :testings, :very_long_column_name_to_test_with, :string
|
||||
add_index :testings, [:foo, :bar, :very_long_column_name_to_test_with]
|
||||
end
|
||||
}.new
|
||||
|
||||
error = assert_raises(StandardError) do
|
||||
ActiveRecord::Migrator.new(:up, [migration], @schema_migration, @internal_metadata).migrate
|
||||
end
|
||||
|
||||
assert_match(/index_testings_on_foo_and_bar_and_very_long_column_name_to_test_with/i, error.message)
|
||||
assert_match(/is too long/i, error.message)
|
||||
end
|
||||
|
||||
def test_add_reference_on_6_0
|
||||
create_migration = Class.new(ActiveRecord::Migration[6.0]) {
|
||||
def version; 100 end
|
||||
|
@ -103,6 +103,11 @@ def test_add_index_with_if_not_exists_matches_exact_index
|
||||
assert connection.index_name_exists?(table_name, "index_testings_on_foo_and_bar")
|
||||
end
|
||||
|
||||
def test_add_index_fallback_to_short_name
|
||||
connection.add_index(table_name, [:foo, :bar, :first_name, :last_name, :administrator])
|
||||
assert connection.index_name_exists?(table_name, "ix_on_foo_bar_first_name_last_name_administrator_5939248142")
|
||||
end
|
||||
|
||||
def test_remove_index_which_does_not_exist_doesnt_raise_with_option
|
||||
connection.add_index(table_name, "foo")
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user