Move UPDATE/DELETE with JOIN handling to the Arel side

2 files changed, 80 insertions, 37 deletions

diff --git a/activerecord/lib/arel/visitors/mysql.rb b/activerecord/lib/arel/visitors/mysql.rb
index eb8a449079..081452caeb 100644
--- a/activerecord/lib/arel/visitors/mysql.rb
+++ b/activerecord/lib/arel/visitors/mysql.rb
@@ -65,12 +65,42 @@ module Arel # :nodoc: all
           collector
         end
 
+        # 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.
+        def prepare_update_statement(o)
+          if has_join_sources?(o)
+            if has_limit_or_offset_or_orders?(o)
+              super
+            else
+              o
+            end
+          elsif o.offset
+            super
+          else
+            o
+          end
+        end
+
+        def prepare_delete_statement(o)
+          if has_join_sources?(o) || o.offset
+            super
+          else
+            o
+          end
+        end
+
+        # 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.
         def build_subselect(key, o)
           subselect = super
 
           # Materialize subquery by adding distinct
           # to work with MySQL 5.7.6 which sets optimizer_switch='derived_merge=on'
-          subselect.distinct unless subselect.limit || subselect.offset || subselect.orders.any?
+          unless has_limit_or_offset_or_orders?(subselect)
+            core = subselect.cores.last
+            core.set_quantifier = Arel::Nodes::Distinct.new
+          end
 
           Nodes::SelectStatement.new.tap do |stmt|
             core = stmt.cores.last
@@ -78,22 +108,6 @@ module Arel # :nodoc: all
             core.projections = [Arel.sql(quote_column_name(key.name))]
           end
         end
-
-        def collect_where_for(o, collector)
-          return super if o.offset
-
-          unless o.wheres.empty?
-            collector << " WHERE "
-            collector = inject_join o.wheres, collector, " AND "
-          end
-
-          unless o.orders.empty?
-            collector << " ORDER BY "
-            collector = inject_join o.orders, collector, ", "
-          end
-
-          maybe_visit o.limit, collector
-        end
     end
   end
 end
diff --git a/activerecord/lib/arel/visitors/to_sql.rb b/activerecord/lib/arel/visitors/to_sql.rb
index 0172204fc8..7c0f6c2e97 100644
--- a/activerecord/lib/arel/visitors/to_sql.rb
+++ b/activerecord/lib/arel/visitors/to_sql.rb
@@ -74,26 +74,17 @@ module Arel # :nodoc: all
       private
 
         def visit_Arel_Nodes_DeleteStatement(o, collector)
+          o = prepare_delete_statement(o)
+
           collector << "DELETE FROM "
           collector = visit o.relation, collector
 
           collect_where_for(o, collector)
         end
 
-        # FIXME: we should probably have a 2-pass visitor for this
-        def build_subselect(key, o)
-          stmt             = Nodes::SelectStatement.new
-          core             = stmt.cores.first
-          core.froms       = o.relation
-          core.wheres      = o.wheres
-          core.projections = [key]
-          stmt.limit       = o.limit
-          stmt.offset      = o.offset
-          stmt.orders      = o.orders
-          stmt
-        end
-
         def visit_Arel_Nodes_UpdateStatement(o, collector)
+          o = prepare_update_statement(o)
+
           collector << "UPDATE "
           collector = visit o.relation, collector
           unless o.values.empty?
@@ -800,19 +791,57 @@ module Arel # :nodoc: all
           }
         end
 
-        def collect_where_for(o, collector)
-          if o.orders.empty? && o.limit.nil? && o.offset.nil?
-            wheres = o.wheres
+        def has_join_sources?(o)
+          o.relation.is_a?(Nodes::JoinSource) && !o.relation.right.empty?
+        end
+
+        def has_limit_or_offset_or_orders?(o)
+          o.limit || o.offset || !o.orders.empty?
+        end
+
+        # The default strategy for an UPDATE with joins is to use a subquery. This doesn't work
+        # on MySQL (even when aliasing the tables), but MySQL allows using JOIN directly in
+        # an UPDATE statement, so in the MySQL visitor we redefine this to do that.
+        def prepare_update_statement(o)
+          if o.key && (has_limit_or_offset_or_orders?(o) || has_join_sources?(o))
+            stmt = o.clone
+            stmt.limit = nil
+            stmt.offset = nil
+            stmt.orders = []
+            stmt.wheres = [Nodes::In.new(o.key, [build_subselect(o.key, o)])]
+            stmt.relation = o.relation.left if has_join_sources?(o)
+            stmt
           else
-            wheres = [Nodes::In.new(o.key, [build_subselect(o.key, o)])]
+            o
           end
+        end
+        alias :prepare_delete_statement :prepare_update_statement
+
+        # FIXME: we should probably have a 2-pass visitor for this
+        def build_subselect(key, o)
+          stmt             = Nodes::SelectStatement.new
+          core             = stmt.cores.first
+          core.froms       = o.relation
+          core.wheres      = o.wheres
+          core.projections = [key]
+          stmt.limit       = o.limit
+          stmt.offset      = o.offset
+          stmt.orders      = o.orders
+          stmt
+        end
 
-          unless wheres.empty?
+        def collect_where_for(o, collector)
+          unless o.wheres.empty?
             collector << " WHERE "
-            collector = inject_join wheres, collector, " AND "
+            collector = inject_join o.wheres, collector, " AND "
           end
 
-          collector
+          unless o.orders.empty?
+            collector << " ORDER BY "
+            collector = inject_join o.orders, collector, ", "
+          end
+
+          maybe_visit o.limit, collector
         end
 
         def infix_value(o, collector, value)