diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index a7ce316365..837313efe1 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,24 @@ +* Relation#destroy_all perform its work in batches + + Since destroy_all actually loads the entire relation and then iteratively destroys the records one by one, + you can blow your memory gasket very easily. So let's do the right thing by default + and do this work in batches of 100 by default and allow you to specify + the batch size like so: #destroy_all(batch_size: 100). + + Apps upgrading to 7.0 will get a deprecation warning. As of Rails 7.1, destroy_all will no longer + return the collection of objects that were destroyed. + + To transition to the new behaviour set the following in an initializer: + + ```ruby + config.active_record.destroy_all_in_batches = true + ``` + + The option is on by default for newly generated Rails apps. Can be set in + an initializer to prevent differences across environments. + + *Genadi Samokovarov*, *Roberto Miranda* + * Adds support for `if_not_exists` to `add_foreign_key` and `if_exists` to `remove_foreign_key`. Applications can set their migrations to ignore exceptions raised when adding a foreign key diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index da3ee97347..90717dd496 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -137,7 +137,7 @@ module ActiveRecord case method when :destroy if scope.klass.primary_key - count = scope.destroy_all.count(&:destroyed?) + count = scope.each(&:destroy).count(&:destroyed?) else scope.each(&:_run_destroy_callbacks) count = scope.delete_all diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index cf18053dc0..f256054328 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -77,6 +77,8 @@ module ActiveRecord class_attribute :default_shard, instance_writer: false + class_attribute :destroy_all_in_batches, instance_accessor: false, default: false + def self.application_record_class? # :nodoc: if ActiveRecord.application_record_class self == ActiveRecord.application_record_class diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index c80c135694..7d1a298487 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -560,8 +560,9 @@ module ActiveRecord # Destroys the records by instantiating each # record and calling its {#destroy}[rdoc-ref:Persistence#destroy] method. # Each object's callbacks are executed (including :dependent association options). - # Returns the collection of objects that were destroyed; each will be frozen, to - # reflect that no changes should be made (since they can't be persisted). + # Returns the collection of objects that were destroyed if + # +config.active_record.destroy_all_in_batches+ is set to +false+. Each + # will be frozen, to reflect that no changes should be made (since they can't be persisted). # # Note: Instantiation, callback execution, and deletion of each # record can be time consuming when you're removing many records at @@ -573,8 +574,40 @@ module ActiveRecord # ==== Examples # # Person.where(age: 0..18).destroy_all - def destroy_all - records.each(&:destroy).tap { reset } + # + # If +config.active_record.destroy_all_in_batches+ is set to +true+, it will ensure + # to perform the record's deletion in batches + # and destroy_all won't longer return the collection of the deleted records + # + # ==== Options + # * :start - Specifies the primary key value to start from, inclusive of the value. + # * :finish - Specifies the primary key value to end at, inclusive of the value. + # * :batch_size - Specifies the size of the batch. Defaults to 1000. + # * :error_on_ignore - Overrides the application config to specify if an error should be raised when + # an order is present in the relation. + # * :order - Specifies the primary key order (can be :asc or :desc). Defaults to :asc. + # + # NOTE: These arguments are honoured only if +config.active_record.destroy_all_in_batches+ is set to +true+. + # + # ==== Examples + # + # # Let's process from record 10_000 on, in batches of 2000. + # Person.destroy_all(start: 10_000, batch_size: 2000) + # + def destroy_all(start: nil, finish: nil, batch_size: 1000, error_on_ignore: nil, order: :asc) + if ActiveRecord::Base.destroy_all_in_batches + batch_options = { of: batch_size, start: start, finish: finish, error_on_ignore: error_on_ignore, order: order } + in_batches(**batch_options).each_record(&:destroy).tap { reset } + else + ActiveSupport::Deprecation.warn(<<~MSG.squish) + As of Rails 7.1, destroy_all will no longer return the collection + of objects that were destroyed. + To transition to the new behaviour set the following in an + initializer: + Rails.application.config.active_record.destroy_all_in_batches = true + MSG + records.each(&:destroy).tap { reset } + end end # Deletes the records without instantiating the records diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 34ce5a4aa0..79eb881bf9 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -29,6 +29,9 @@ ARTest.connect # Quote "type" if it's a reserved word for the current connection. QUOTED_TYPE = ActiveRecord::Base.connection.quote_column_name("type") +# FIXME: Remove this in Rails 7.1 when it's no longer needed. +ActiveRecord::Base.destroy_all_in_batches = true + def current_adapter?(*types) types.any? do |type| ActiveRecord::ConnectionAdapters.const_defined?(type) && diff --git a/activerecord/test/cases/relation/delete_all_test.rb b/activerecord/test/cases/relation/delete_all_test.rb index 6405e86ebc..2d15318e06 100644 --- a/activerecord/test/cases/relation/delete_all_test.rb +++ b/activerecord/test/cases/relation/delete_all_test.rb @@ -12,18 +12,37 @@ class DeleteAllTest < ActiveRecord::TestCase def test_destroy_all davids = Author.where(name: "David") + assert_difference("Author.count", -1) do + davids.destroy_all + end + end + + def test_destroy_all_no_longer_returns_a_collection + davids = Author.where(name: "David") + + assert_nil davids.destroy_all + end + + def test_destroy_all_deprecated_returns_collection + ActiveRecord::Base.destroy_all_in_batches = false + davids = Author.where(name: "David") + # Force load assert_equal [authors(:david)], davids.to_a assert_predicate davids, :loaded? - assert_difference("Author.count", -1) do - destroyed = davids.destroy_all - assert_equal [authors(:david)], destroyed - assert_predicate destroyed.first, :frozen? + assert_deprecated do + assert_difference("Author.count", -1) do + destroyed = davids.destroy_all + assert_equal [authors(:david)], destroyed + assert_predicate destroyed.first, :frozen? + end end assert_equal [], davids.to_a assert_predicate davids, :loaded? + ensure + ActiveRecord::Base.destroy_all_in_batches = true end def test_delete_all diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 70051b58f4..bb8d34dde5 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1846,8 +1846,22 @@ class RelationTest < ActiveRecord::TestCase assert_difference("Post.count", -3) { david.posts.destroy_by(body: "hello") } - destroyed = Author.destroy_by(id: david.id) - assert_equal [david], destroyed + assert_difference("Author.count", -1) do + Author.destroy_by(id: david.id) + end + end + + def test_destroy_all_deprecated_return_value + ActiveRecord::Base.destroy_all_in_batches = false + david = authors(:david) + + assert_deprecated do + assert_difference("Author.count", -1) do + assert_equal [david], Author.where(id: david.id).destroy_all + end + end + ensure + ActiveRecord::Base.destroy_all_in_batches = true end test "find_by with hash conditions returns the first matching record" do diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 0a524606c1..02ec06346b 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -511,6 +511,11 @@ The schema dumper adds two additional configuration options: `fk_rails_` are not exported to the database schema dump. Defaults to `/^fk_rails_[0-9a-f]{10}$/`. +* `config.active_record.destroy_all_in_batches` ensures + ActiveRecord::Relation#destroy_all to perform the record's deletion in batches. + ActiveRecord::Relation#destroy_all won't longer return the collection of the deleted + records after enabling this option. + ### Configuring Action Controller `config.action_controller` includes a number of configuration settings: diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_0.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_0.rb.tt index 5ce4fe5ea5..b6f96daedf 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_0.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_0.rb.tt @@ -43,3 +43,6 @@ # of the video). # Rails.application.config.active_storage.video_preview_arguments = # "-vf 'select=eq(n\\,0)+eq(key\\,1)+gt(scene\\,0.015),loop=loop=-1:size=2,trim=start_frame=1' -frames:v 1 -f image2" +# +# Enforce destroy in batches when calling ActiveRecord::Relation#destroy_all +Rails.application.config.active_record.destroy_all_in_batches = true