From 2c66b2a458614c5f365836a2642d40bc0c77d594 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Mon, 22 Jan 2024 14:47:23 +0200 Subject: [PATCH] Ensure pre-7.1 migrations use legacy index names when using `rename_table` --- .../abstract/schema_statements.rb | 12 +++++++---- .../abstract_mysql_adapter.rb | 2 +- .../postgresql/schema_statements.rb | 2 +- .../connection_adapters/sqlite3_adapter.rb | 2 +- .../active_record/migration/compatibility.rb | 1 + .../cases/migration/compatibility_test.rb | 21 +++++++++++++++++++ .../test/cases/migration/rename_table_test.rb | 12 +++++++++++ 7 files changed, 45 insertions(+), 7 deletions(-) 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 ec5e668748..7a477569c0 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -971,7 +971,11 @@ def rename_index(table_name, old_name, new_name) def index_name(table_name, options) # :nodoc: if Hash === options if options[:column] - generate_index_name(table_name, options[:column]) + if options[:_uses_legacy_index_name] + "index_#{table_name}_on_#{Array(options[:column]) * '_and_'}" + else + generate_index_name(table_name, options[:column]) + end elsif options[:name] options[:name] else @@ -1645,11 +1649,11 @@ def index_name_for_remove(table_name, column_name, options) end end - def rename_table_indexes(table_name, new_name) + def rename_table_indexes(table_name, new_name, **options) indexes(new_name).each do |index| - generated_index_name = index_name(table_name, column: index.columns) + generated_index_name = index_name(table_name, column: index.columns, **options) if generated_index_name == index.name - rename_index new_name, generated_index_name, index_name(new_name, column: index.columns) + rename_index new_name, generated_index_name, index_name(new_name, column: index.columns, **options) end 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 8d1e6318bf..961a8ab667 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -340,7 +340,7 @@ def rename_table(table_name, new_name, **options) schema_cache.clear_data_source_cache!(table_name.to_s) schema_cache.clear_data_source_cache!(new_name.to_s) execute "RENAME TABLE #{quote_table_name(table_name)} TO #{quote_table_name(new_name)}" - rename_table_indexes(table_name, new_name) + rename_table_indexes(table_name, new_name, **options) end # Drops a table from the database. 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 b8c63f70b7..c3ec2489d3 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -400,7 +400,7 @@ def rename_table(table_name, new_name, **options) execute "ALTER TABLE #{seq.quoted} RENAME TO #{quote_table_name(new_seq)}" end end - rename_table_indexes(table_name, new_name) + rename_table_indexes(table_name, new_name, **options) end def add_column(table_name, column_name, type, **options) # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index a6510dd2a4..90a16aacc1 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -291,7 +291,7 @@ def rename_table(table_name, new_name, **options) schema_cache.clear_data_source_cache!(table_name.to_s) schema_cache.clear_data_source_cache!(new_name.to_s) exec_query "ALTER TABLE #{quote_table_name(table_name)} RENAME TO #{quote_table_name(new_name)}" - rename_table_indexes(table_name, new_name) + rename_table_indexes(table_name, new_name, **options) end def add_column(table_name, column_name, type, **options) # :nodoc: diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb index 5325acd0a4..e82062cb3c 100644 --- a/activerecord/lib/active_record/migration/compatibility.rb +++ b/activerecord/lib/active_record/migration/compatibility.rb @@ -119,6 +119,7 @@ def create_table(table_name, **options) def rename_table(table_name, new_name, **options) options[:_uses_legacy_table_name] = true + options[:_uses_legacy_index_name] = true super end diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb index 76db6af884..6d458f6500 100644 --- a/activerecord/test/cases/migration/compatibility_test.rb +++ b/activerecord/test/cases/migration/compatibility_test.rb @@ -420,6 +420,27 @@ def migrate(x) connection.drop_table :more_testings rescue nil end + def test_rename_table_errors_on_too_long_index_name_7_0 + long_table_name = "a" * connection.table_name_length + + migration = Class.new(ActiveRecord::Migration[7.0]) { + def migrate(x) + add_index :testings, :foo + long_table_name = "a" * connection.table_name_length + rename_table :testings, long_table_name + end + }.new + + error = assert_raises(StandardError) do + ActiveRecord::Migrator.new(:up, [migration], @schema_migration, @internal_metadata).migrate + end + + assert_match(/index_#{long_table_name}_on_foo/i, error.message) + assert_match(/is too long/i, error.message) + ensure + connection.drop_table long_table_name, if_exists: true + end + if current_adapter?(:Mysql2Adapter, :TrilogyAdapter) def test_change_table_collation_not_unset_7_0 migration = Class.new(ActiveRecord::Migration[7.0]) { diff --git a/activerecord/test/cases/migration/rename_table_test.rb b/activerecord/test/cases/migration/rename_table_test.rb index 3ad60914d5..2c054683d1 100644 --- a/activerecord/test/cases/migration/rename_table_test.rb +++ b/activerecord/test/cases/migration/rename_table_test.rb @@ -79,6 +79,18 @@ def test_rename_table_with_an_index assert_equal "index_octopi_on_url", index.name end + def test_rename_table_with_long_table_name_and_index + long_name = "a" * connection.table_name_length + + add_index :test_models, :url + rename_table :test_models, long_name + + index = connection.indexes(long_name).first + assert_includes index.columns, "url" + ensure + rename_table long_name, :test_models + end + def test_rename_table_does_not_rename_custom_named_index add_index :test_models, :url, name: "special_url_idx"