Merge pull request #42075 from jdufresne/remove-columns

Optimize remove_columns to use a single SQL statement when supported

activerecord/CHANGELOG.md

@@ -1,3 +1,17 @@
+*   Optimize `remove_columns` to use a single SQL statement.
+
+    ```ruby
+    remove_columns :my_table, :col_one, :col_two
+    ```
+
+    Now results in the following SQL:
+
+    ```sql
+    ALTER TABLE "my_table" DROP COLUMN "col_one", DROP COLUMN "col_two"
+    ```
+
+    *Jon Dufresne*
+
 *   Ensure `has_one` autosave association callbacks get called once.
 
     Change the `has_one` autosave callback to be non cyclic as well.

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

@@ -629,9 +629,8 @@ module ActiveRecord
           raise ArgumentError.new("You must specify at least one column name. Example: remove_columns(:people, :first_name)")
         end
 
-        column_names.each do |column_name|
-          remove_column(table_name, column_name, type, **options)
-        end
+        remove_column_fragments = remove_columns_for_alter(table_name, *column_names, type: type, **options)
+        execute "ALTER TABLE #{quote_table_name(table_name)} #{remove_column_fragments.join(', ')}"
       end
 
       # Removes the column from the table definition.
@@ -1296,8 +1295,7 @@ module ActiveRecord
       #  remove_timestamps(:suppliers)
       #
       def remove_timestamps(table_name, **options)
-        remove_column table_name, :updated_at
-        remove_column table_name, :created_at
+        remove_columns table_name, :updated_at, :created_at
       end
 
       def update_table_definition(table_name, base) #:nodoc:

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

@@ -256,6 +256,18 @@ module ActiveRecord
         end
       end
 
+      def remove_columns(table_name, *column_names, type: nil, **options) # :nodoc:
+        alter_table(table_name) do |definition|
+          column_names.each do |column_name|
+            definition.remove_column column_name
+          end
+          column_names = column_names.map(&:to_s)
+          definition.foreign_keys.delete_if do |_, fk_options|
+            column_names.include?(fk_options[:column])
+          end
+        end
+      end
+
       def change_column_default(table_name, column_name, default_or_changes) #:nodoc:
         default = extract_new_default_value(default_or_changes)
 

activerecord/test/cases/migration/columns_test.rb

@@ -321,6 +321,26 @@ module ActiveRecord
       ensure
         connection.drop_table :my_table, if_exists: true
       end
+
+      def test_remove_columns_single_statement
+        connection.create_table "my_table" do |t|
+          t.integer "col_one"
+          t.integer "col_two"
+        end
+
+        # SQLite3's ALTER TABLE statement has several limitations. To mange
+        # this, the adapter creates a temporary table, copies the data, drops
+        # the old table, creates the new table, then copies the data back.
+        expected_query_count = current_adapter?(:SQLite3Adapter) ? 12 : 1
+        assert_queries(expected_query_count) do
+          connection.remove_columns("my_table", "col_one", "col_two")
+        end
+
+        columns = connection.columns("my_table").map(&:name)
+        assert_equal ["id"], columns
+      ensure
+        connection.drop_table :my_table, if_exists: true
+      end
     end
   end
 end