diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 754aad14b90..a1086eba528 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Allow bulk `ALTER` statements to drop and recreate indexes with the same name. + + *Eugene Kenny* + * `insert`, `insert_all`, `upsert`, and `upsert_all` now clear the query cache. *Eugene Kenny* 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 d2f7409a45a..fa348c40977 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -1195,9 +1195,6 @@ module ActiveRecord validate_index_length!(table_name, index_name, options.fetch(:internal, false)) - if data_source_exists?(table_name) && index_name_exists?(table_name, index_name) - raise ArgumentError, "Index name '#{index_name}' on table '#{table_name}' already exists" - end index_columns = quoted_columns_for_index(column_names, **options).join(", ") [index_name, index_type, index_columns, index_options, algorithm, using, comment] diff --git a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb index c2c357d0c14..66db2af2956 100644 --- a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb +++ b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb @@ -27,10 +27,6 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase end def test_add_index - # add_index calls data_source_exists? and index_name_exists? which can't work since execute is stubbed - def (ActiveRecord::Base.connection).data_source_exists?(*); true; end - def (ActiveRecord::Base.connection).index_name_exists?(*); false; end - expected = "CREATE INDEX `index_people_on_last_name` ON `people` (`last_name`) " assert_equal expected, add_index(:people, :last_name, length: nil) @@ -74,8 +70,6 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase end def test_index_in_create - def (ActiveRecord::Base.connection).data_source_exists?(*); false; end - %w(SPATIAL FULLTEXT UNIQUE).each do |type| expected = /\ACREATE TABLE `people` \(#{type} INDEX `index_people_on_last_name` \(`last_name`\)\)/ actual = ActiveRecord::Base.connection.create_table(:people, id: false) do |t| @@ -92,9 +86,6 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase end def test_index_in_bulk_change - def (ActiveRecord::Base.connection).data_source_exists?(*); true; end - def (ActiveRecord::Base.connection).index_name_exists?(*); false; end - %w(SPATIAL FULLTEXT UNIQUE).each do |type| expected = "ALTER TABLE `people` ADD #{type} INDEX `index_people_on_last_name` (`last_name`)" assert_sql(expected) do @@ -170,19 +161,12 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase end def test_indexes_in_create - assert_called_with( - ActiveRecord::Base.connection, - :data_source_exists?, - [:temp], - returns: false - ) do - expected = /\ACREATE TEMPORARY TABLE `temp` \( INDEX `index_temp_on_zip` \(`zip`\)\)(?: ROW_FORMAT=DYNAMIC)? AS SELECT id, name, zip FROM a_really_complicated_query/ - actual = ActiveRecord::Base.connection.create_table(:temp, temporary: true, as: "SELECT id, name, zip FROM a_really_complicated_query") do |t| - t.index :zip - end - - assert_match expected, actual + expected = /\ACREATE TEMPORARY TABLE `temp` \( INDEX `index_temp_on_zip` \(`zip`\)\)(?: ROW_FORMAT=DYNAMIC)? AS SELECT id, name, zip FROM a_really_complicated_query/ + actual = ActiveRecord::Base.connection.create_table(:temp, temporary: true, as: "SELECT id, name, zip FROM a_really_complicated_query") do |t| + t.index :zip end + + assert_match expected, actual end private diff --git a/activerecord/test/cases/adapters/postgresql/active_schema_test.rb b/activerecord/test/cases/adapters/postgresql/active_schema_test.rb index 62efaf3bfea..cdcb609caff 100644 --- a/activerecord/test/cases/adapters/postgresql/active_schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/active_schema_test.rb @@ -28,9 +28,6 @@ class PostgresqlActiveSchemaTest < ActiveRecord::PostgreSQLTestCase end def test_add_index - # add_index calls index_name_exists? which can't work since execute is stubbed - ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.define_method(:index_name_exists?) { |*| false } - expected = %(CREATE UNIQUE INDEX "index_people_on_last_name" ON "people" ("last_name") WHERE state = 'active') assert_equal expected, add_index(:people, :last_name, unique: true, where: "state = 'active'") @@ -73,8 +70,6 @@ class PostgresqlActiveSchemaTest < ActiveRecord::PostgreSQLTestCase assert_raise ArgumentError do add_index(:people, :last_name, algorithm: :copy) end - - ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.remove_method :index_name_exists? end def test_remove_index diff --git a/activerecord/test/cases/migration/index_test.rb b/activerecord/test/cases/migration/index_test.rb index 5e688efc2b6..3d07379acc6 100644 --- a/activerecord/test/cases/migration/index_test.rb +++ b/activerecord/test/cases/migration/index_test.rb @@ -49,13 +49,6 @@ module ActiveRecord assert connection.index_name_exists?(table_name, "old_idx") end - def test_double_add_index - connection.add_index(table_name, [:foo], name: "some_idx") - assert_raises(ArgumentError) { - connection.add_index(table_name, [:foo], name: "some_idx") - } - end - def test_remove_nonexistent_index assert_raise(ArgumentError) { connection.remove_index(table_name, "no_such_index") } end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 20f577b2c5a..7f380c396f2 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -938,6 +938,34 @@ if ActiveRecord::Base.connection.supports_bulk_alter? assert_equal "This is a comment", column(:birthdate).comment end + def test_changing_index + with_bulk_change_table do |t| + t.string :username + t.index :username, name: :username_index + end + + assert index(:username_index) + assert_not index(:username_index).unique + + classname = ActiveRecord::Base.connection.class.name[/[^:]*$/] + expected_query_count = { + "Mysql2Adapter" => 1, # mysql2 supports dropping and creating two indexes using one statement + "PostgreSQLAdapter" => 2, + }.fetch(classname) { + raise "need an expected query count for #{classname}" + } + + assert_queries(expected_query_count) do + with_bulk_change_table do |t| + t.remove_index name: :username_index + t.index :username, name: :username_index, unique: true + end + end + + assert index(:username_index) + assert index(:username_index).unique + end + private def with_bulk_change_table # Reset columns/indexes cache as we're changing the table