Fix delete_all when chained with joins.

Closes #5202 and #919

4 files changed, 59 insertions, 17 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
index e919068909..be712f2334 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
@@ -312,13 +312,27 @@ module ActiveRecord
       # on mysql (even when aliasing the tables), but mysql allows using JOIN directly in
       # an UPDATE statement, so in the mysql adapters we redefine this to do that.
       def join_to_update(update, select) #:nodoc:
-        subselect = select.clone
-        subselect.projections = [update.key]
+        key = update.key
+        subselect = subquery_for(key, select)
 
-        update.where update.key.in(subselect)
+        update.where key.in(subselect)
+      end
+
+      def join_to_delete(delete, select, key) #:nodoc:
+        subselect = subquery_for(key, select)
+
+        delete.where key.in(subselect)
       end
 
       protected
+
+        # Return a subquery for the given key using the join information.
+        def subquery_for(key, select)
+          subselect = select.clone
+          subselect.projections = [key]
+          subselect
+        end
+
         # Returns an array of record hashes with the column names as keys and
         # column values as values.
         def select(sql, name = nil, binds = [])
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index 64f922b7ad..d0ea468430 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -294,19 +294,10 @@ module ActiveRecord
 
       # In the simple case, MySQL allows us to place JOINs directly into the UPDATE
       # query. However, this does not allow for LIMIT, OFFSET and ORDER. To support
-      # these, we must use a subquery. However, MySQL is too stupid to create a
-      # temporary table for this automatically, so we have to give it some prompting
-      # in the form of a subsubquery. Ugh!
+      # these, we must use a subquery.
       def join_to_update(update, select) #:nodoc:
         if select.limit || select.offset || select.orders.any?
-          subsubselect = select.clone
-          subsubselect.projections = [update.key]
-
-          subselect = Arel::SelectManager.new(select.engine)
-          subselect.project Arel.sql(update.key.name)
-          subselect.from subsubselect.as('__active_record_temp')
-
-          update.where update.key.in(subselect)
+          super
         else
           update.table select.source
           update.wheres = select.constraints
@@ -558,6 +549,17 @@ module ActiveRecord
 
       protected
 
+      # MySQL is too stupid to create a temporary table for use subquery, so we have
+      # to give it some prompting in the form of a subsubquery. Ugh!
+      def subquery_for(key, select)
+        subsubselect = select.clone
+        subsubselect.projections = [key]
+
+        subselect = Arel::SelectManager.new(select.engine)
+        subselect.project Arel.sql(key.name)
+        subselect.from subsubselect.as('__active_record_temp')
+      end
+
       def add_index_length(option_strings, column_names, options = {})
         if options.is_a?(Hash) && length = options[:length]
           case length
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index b125449127..b007b8c168 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -400,8 +400,16 @@ module ActiveRecord
       if conditions
         where(conditions).delete_all
       else
-        statement = arel.compile_delete
-        affected = @klass.connection.delete(statement, 'SQL', bind_values)
+        stmt = Arel::DeleteManager.new(arel.engine)
+        stmt.from(table)
+
+        if joins_values.any?
+          @klass.connection.join_to_delete(stmt, arel, table[primary_key])
+        else
+          stmt.wheres = arel.constraints
+        end
+
+        affected = @klass.connection.delete(stmt, 'SQL', bind_values)
 
         reset
         affected
diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb
index adfd8e83a1..bb60aa0a66 100644
--- a/activerecord/test/cases/persistence_test.rb
+++ b/activerecord/test/cases/persistence_test.rb
@@ -13,12 +13,14 @@ require 'models/warehouse_thing'
 require 'models/parrot'
 require 'models/minivan'
 require 'models/person'
+require 'models/pet'
+require 'models/toy'
 require 'rexml/document'
 require 'active_support/core_ext/exception'
 
 class PersistencesTest < ActiveRecord::TestCase
 
-  fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, 'warehouse-things', :authors, :categorizations, :categories, :posts, :minivans
+  fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, 'warehouse-things', :authors, :categorizations, :categories, :posts, :minivans, :pets, :toys
 
   # Oracle UPDATE does not support ORDER BY
   unless current_adapter?(:OracleAdapter)
@@ -76,6 +78,22 @@ class PersistencesTest < ActiveRecord::TestCase
     assert_equal Topic.count, Topic.delete_all
   end
 
+  def test_delete_all_with_joins_and_where_part_is_hash
+    where_args = {:toys => {:name => 'Bone'}}
+    count = Pet.joins(:toys).where(where_args).count
+
+    assert_equal count, 1
+    assert_equal count, Pet.joins(:toys).where(where_args).delete_all
+  end
+
+  def test_delete_all_with_joins_and_where_part_is_not_hash
+    where_args = ['toys.name = ?', 'Bone']
+    count = Pet.joins(:toys).where(where_args).count
+
+    assert_equal count, 1
+    assert_equal count, Pet.joins(:toys).where(where_args).delete_all
+  end
+
   def test_update_by_condition
     Topic.update_all "content = 'bulk updated!'", ["approved = ?", true]
     assert_equal "Have a nice day", Topic.find(1).content