Skip to content

Commit

Permalink
Merge pull request rails#52773 from sobrinho/ar-drop-tables
Browse files Browse the repository at this point in the history
Allow drop_table to accept an array of table names
  • Loading branch information
rafaelfranca authored Sep 17, 2024
2 parents 0c6e24f + 1666bad commit 4aa7811
Show file tree
Hide file tree
Showing 9 changed files with 79 additions and 20 deletions.
10 changes: 10 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
* Allow `drop_table` to accept an array of table names.

This will let you to drop multiple tables in a single call.

```ruby
ActiveRecord::Base.connection.drop_table(:users, :posts)
```

*Gabriel Sobrinho*

* Add support for PostgreSQL `IF NOT EXISTS` via the `:if_not_exists` option
on the `add_enum_value` method.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ def rename_table(table_name, new_name, **)
raise NotImplementedError, "rename_table is not implemented"
end

# Drops a table from the database.
# Drops a table or tables from the database.
#
# [<tt>:force</tt>]
# Set to +:cascade+ to drop dependent objects as well.
Expand All @@ -536,10 +536,12 @@ def rename_table(table_name, new_name, **)
#
# Although this command ignores most +options+ and the block if one is given,
# it can be helpful to provide these in a migration's +change+ method so it can be reverted.
# In that case, +options+ and the block will be used by #create_table.
def drop_table(table_name, **options)
schema_cache.clear_data_source_cache!(table_name.to_s)
execute "DROP TABLE#{' IF EXISTS' if options[:if_exists]} #{quote_table_name(table_name)}"
# In that case, +options+ and the block will be used by #create_table except if you provide more than one table which is not supported.
def drop_table(*table_names, **options)
table_names.each do |table_name|
schema_cache.clear_data_source_cache!(table_name.to_s)
execute "DROP TABLE#{' IF EXISTS' if options[:if_exists]} #{quote_table_name(table_name)}"
end
end

# Add a new +type+ column named +column_name+ to +table_name+.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ def rename_table(table_name, new_name, **options)
rename_table_indexes(table_name, new_name, **options)
end

# Drops a table from the database.
# Drops a table or tables from the database.
#
# [<tt>:force</tt>]
# Set to +:cascade+ to drop dependent objects as well.
Expand All @@ -346,10 +346,10 @@ def rename_table(table_name, new_name, **options)
#
# Although this command ignores most +options+ and the block if one is given,
# it can be helpful to provide these in a migration's +change+ method so it can be reverted.
# In that case, +options+ and the block will be used by create_table.
def drop_table(table_name, **options)
schema_cache.clear_data_source_cache!(table_name.to_s)
execute "DROP#{' TEMPORARY' if options[:temporary]} TABLE#{' IF EXISTS' if options[:if_exists]} #{quote_table_name(table_name)}#{' CASCADE' if options[:force] == :cascade}"
# In that case, +options+ and the block will be used by #create_table except if you provide more than one table which is not supported.
def drop_table(*table_names, **options)
table_names.each { |table_name| schema_cache.clear_data_source_cache!(table_name.to_s) }
execute "DROP#{' TEMPORARY' if options[:temporary]} TABLE#{' IF EXISTS' if options[:if_exists]} #{table_names.map { |table_name| quote_table_name(table_name) }.join(', ')}#{' CASCADE' if options[:force] == :cascade}"
end

def rename_index(table_name, old_name, new_name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ def drop_database(name) # :nodoc:
execute "DROP DATABASE IF EXISTS #{quote_table_name(name)}"
end

def drop_table(table_name, **options) # :nodoc:
schema_cache.clear_data_source_cache!(table_name.to_s)
execute "DROP TABLE#{' IF EXISTS' if options[:if_exists]} #{quote_table_name(table_name)}#{' CASCADE' if options[:force] == :cascade}"
def drop_table(*table_names, **options) # :nodoc:
table_names.each { |table_name| schema_cache.clear_data_source_cache!(table_name.to_s) }
execute "DROP TABLE#{' IF EXISTS' if options[:if_exists]} #{table_names.map { |table_name| quote_table_name(table_name) }.join(', ')}#{' CASCADE' if options[:force] == :cascade}"
end

# Returns true if schema exists.
Expand Down
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/migration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ def initialize
#
# === Deletion
#
# * <tt>drop_table(name)</tt>: Drops the table called +name+.
# * <tt>drop_table(*names)</tt>: Drops the given tables.
# * <tt>drop_join_table(table_1, table_2, options)</tt>: Drops the join table
# specified by the given arguments.
# * <tt>remove_column(table_name, column_name, type, options)</tt>: Removes the column
Expand Down Expand Up @@ -602,7 +602,7 @@ def create_join_table(table_1, table_2, **options)
end
end

def drop_table(table_name, **options)
def drop_table(*table_names, **options)
if block_given?
super { |t| yield compatible_table_definition(t) }
else
Expand Down
13 changes: 9 additions & 4 deletions activerecord/lib/active_record/migration/command_recorder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -202,13 +202,18 @@ def invert_create_table(args, &block)
end

def invert_drop_table(args, &block)
if args.last.is_a?(Hash)
args.last.delete(:if_exists)
options = args.extract_options!
options.delete(:if_exists)

if args.size > 1
raise ActiveRecord::IrreversibleMigration, "To avoid mistakes, drop_table is only reversible if given a single table name."
end
if args.size == 1 && block == nil

if args.size == 1 && options == {} && block == nil
raise ActiveRecord::IrreversibleMigration, "To avoid mistakes, drop_table is only reversible if given options or a block (can be empty)."
end
super

super(args.push(options), &block)
end

def invert_rename_table(args)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@ def test_drop_table
assert_equal "DROP TABLE `people`", drop_table(:people)
end

def test_drop_tables
assert_equal "DROP TABLE `people`, `sobrinho`", drop_table(:people, :sobrinho)
end

def test_create_mysql_database_with_encoding
if ActiveRecord::Base.lease_connection.send(:row_format_dynamic_by_default?)
assert_equal "CREATE DATABASE `matt` DEFAULT CHARACTER SET `utf8mb4`", create_database(:matt)
Expand Down Expand Up @@ -147,6 +151,10 @@ def test_drop_table_with_specific_database
assert_equal "DROP TABLE `otherdb`.`people`", drop_table("otherdb.people")
end

def test_drop_tables_with_specific_database
assert_equal "DROP TABLE `otherdb`.`people`, `otherdb`.`sobrinho`", drop_table("otherdb.people", "otherdb.sobrinho")
end

def test_add_timestamps
with_real_execute do
ActiveRecord::Base.lease_connection.create_table :delete_me
Expand Down
14 changes: 14 additions & 0 deletions activerecord/test/cases/migration/change_schema_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -467,10 +467,24 @@ def test_drop_table_if_exists
assert_not connection.table_exists?(:testings)
end

def test_drop_tables_if_exists
connection.create_table(:testings)
connection.create_table(:sobrinho)
assert connection.table_exists?(:testings)
assert connection.table_exists?(:sobrinho)
connection.drop_table(:testings, :sobrinho, if_exists: true)
assert_not connection.table_exists?(:testings)
assert_not connection.table_exists?(:sobrinho)
end

def test_drop_table_if_exists_nothing_raised
assert_nothing_raised { connection.drop_table(:nonexistent, if_exists: true) }
end

def test_drop_tables_if_exists_nothing_raised
assert_nothing_raised { connection.drop_table(:nonexistent, :nonexistent_sobrinho, if_exists: true) }
end

private
def testing_table_with_only_foo_attribute
connection.create_table :testings, id: false do |t|
Expand Down
22 changes: 21 additions & 1 deletion activerecord/test/cases/migration/command_recorder_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,31 @@ def test_invert_drop_table_with_if_exists
end

def test_invert_drop_table_without_a_block_nor_option
assert_raises(ActiveRecord::IrreversibleMigration) do
assert_raises(ActiveRecord::IrreversibleMigration, match: "To avoid mistakes, drop_table is only reversible if given options or a block (can be empty).") do
@recorder.inverse_of :drop_table, [:people_reminders]
end
end

def test_invert_drop_table_with_multiple_tables
assert_raises(ActiveRecord::IrreversibleMigration, match: "To avoid mistakes, drop_table is only reversible if given a single table name.") do
@recorder.inverse_of :drop_table, [:musics, :artists]
end
end

def test_invert_drop_table_with_multiple_tables_and_options
assert_raises(ActiveRecord::IrreversibleMigration, match: "To avoid mistakes, drop_table is only reversible if given a single table name.") do
@recorder.inverse_of :drop_table, [:musics, :artists, id: false]
end
end

def test_invert_drop_table_with_multiple_tables_and_block
block = Proc.new { }

assert_raises(ActiveRecord::IrreversibleMigration, match: "To avoid mistakes, drop_table is only reversible if given a single table name.") do
@recorder.inverse_of :drop_table, [:musics, :artists], &block
end
end

def test_invert_create_join_table
drop_join_table = @recorder.inverse_of :create_join_table, [:musics, :artists]
assert_equal [:drop_join_table, [:musics, :artists], nil], drop_join_table
Expand Down

0 comments on commit 4aa7811

Please sign in to comment.