Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Merge pull request #14546 from eileencodes/fix_delete_all_to_not_use_…

…IN_statement

Fix delete all to not produce sql in statement
Conflicts:
	activerecord/test/cases/associations/has_many_associations_test.rb

Merge pull request #14727 from robin850/patch-17

Add a changelog entry for #14546 [ci skip]
Conflicts:
	activerecord/CHANGELOG.md
  • Loading branch information...
commit 4bdff5b36656196d977147d625eeb0a98d2f4842 1 parent b31b2fe
@tenderlove tenderlove authored senny committed
View
14 activerecord/CHANGELOG.md
@@ -1,3 +1,17 @@
+* Calling `delete_all` on an unloaded `CollectionProxy` no longer
+ generates a SQL statement containing each id of the collection:
+
+ Before:
+
+ DELETE FROM `model` WHERE `model`.`parent_id` = 1
+ AND `model`.`id` IN (1, 2, 3...)
+
+ After:
+
+ DELETE FROM `model` WHERE `model`.`parent_id` = 1
+
+ *Eileen M. Uchitelle*, *Aaron Patterson*
+
* Fixed a problem where count used with a grouping was not returning a Hash.
Fixes #14721.
View
2  activerecord/lib/active_record/associations/collection_association.rb
@@ -249,7 +249,7 @@ def delete(*records)
dependent = _options[:dependent] || options[:dependent]
if records.first == :all
- if loaded? || dependent == :destroy
+ if (loaded? || dependent == :destroy) && dependent != :delete_all
delete_or_destroy(load_target, dependent)
else
delete_records(:all, dependent)
View
18 activerecord/test/cases/associations/has_many_associations_test.rb
@@ -24,6 +24,9 @@
require 'models/speedometer'
require 'models/college'
require 'models/student'
+require 'models/reference'
+require 'models/job'
+
class HasManyAssociationsTestForReorderWithJoinDependency < ActiveRecord::TestCase
fixtures :authors, :posts, :comments
@@ -41,7 +44,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
fixtures :accounts, :categories, :companies, :developers, :projects,
:developers_projects, :topics, :authors, :comments,
:people, :posts, :readers, :taggings, :cars, :essays,
- :categorizations
+ :categorizations, :jobs
def setup
Client.destroyed_client_ids.clear
@@ -116,6 +119,19 @@ def test_do_not_call_callbacks_for_delete_all
assert_equal 0, Bulb.count, "bulbs should have been deleted using :delete_all strategy"
end
+ def test_delete_all_on_association_is_the_same_as_not_loaded
+ author = authors :david
+ author.thinking_posts.create!(:body => "test")
+ author.reload
+ expected_sql = capture_sql { author.thinking_posts.delete_all }
+
+ author.thinking_posts.create!(:body => "test")
+ author.reload
+ author.thinking_posts.inspect
+ loaded_sql = capture_sql { author.thinking_posts.delete_all }
+ assert_equal(expected_sql, loaded_sql)
+ end
+
def test_building_the_associated_object_with_implicit_sti_base_class
firm = DependentFirm.new
company = firm.companies.build
View
6 activerecord/test/cases/test_case.rb
@@ -19,6 +19,12 @@ def assert_date_from_db(expected, actual, message = nil)
end
end
+ def capture_sql
+ SQLCounter.clear_log
+ yield
+ SQLCounter.log_all.dup
+ end
+
def assert_sql(*patterns_to_match)
SQLCounter.clear_log
yield

2 comments on commit 4bdff5b

Please sign in to comment.
Something went wrong with that request. Please try again.