Move UPDATE/DELETE with JOIN handling to the Arel side

5 files changed, 92 insertions, 106 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 feacdf6931..0059f0b773 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
@@ -410,16 +410,6 @@ module ActiveRecord
         end
       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 adapters we redefine this to do that.
-      def join_to_update(update, select, key) # :nodoc:
-        subselect = subquery_for(key, select)
-
-        update.where key.in(subselect)
-      end
-      alias join_to_delete join_to_update
-
       private
         def default_insert_value(column)
           Arel.sql("DEFAULT")
@@ -460,13 +450,6 @@ module ActiveRecord
           total_sql.join(";\n")
         end
 
-        # Returns 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 ActiveRecord::Result instance.
         def select(sql, name = nil, binds = [])
           exec_query(sql, name, binds, prepare: false)
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 09242a0f14..d40f38fb77 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -223,18 +223,6 @@ module ActiveRecord
         execute "ROLLBACK"
       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 join_to_update(update, select, key) # :nodoc:
-        if select.limit || select.offset || select.orders.any?
-          super
-        else
-          update.table select.source
-          update.wheres = select.constraints
-        end
-      end
-
       def empty_insert_statement_value(primary_key = nil)
         "VALUES ()"
       end
@@ -733,20 +721,6 @@ module ActiveRecord
           [remove_column_for_alter(table_name, :updated_at), remove_column_for_alter(table_name, :created_at)]
         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. Ugh!
-        def subquery_for(key, select)
-          subselect = select.clone
-          subselect.projections = [key]
-
-          # Materialize subquery by adding distinct
-          # to work with MySQL 5.7.6 which sets optimizer_switch='derived_merge=on'
-          subselect.distinct unless select.limit || select.offset || select.orders.any?
-
-          key_name = quote_column_name(key.name)
-          Arel::SelectManager.new(subselect.as("__active_record_temp")).project(Arel.sql(key_name))
-        end
-
         def supports_rename_index?
           mariadb? ? false : version >= "5.7.6"
         end
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index d8cff30b88..d5b6082d13 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -348,7 +348,12 @@ module ActiveRecord
       end
 
       stmt = Arel::UpdateManager.new
-      stmt.table(table)
+      stmt.table(arel.join_sources.empty? ? table : arel.source)
+      stmt.key = arel_attribute(primary_key)
+      stmt.take(arel.limit)
+      stmt.offset(arel.offset)
+      stmt.order(*arel.orders)
+      stmt.wheres = arel.constraints
 
       if updates.is_a?(Hash)
         stmt.set _substitute_values(updates)
@@ -356,16 +361,6 @@ module ActiveRecord
         stmt.set Arel.sql(klass.sanitize_sql_for_assignment(updates, table.name))
       end
 
-      if has_join_values?
-        @klass.connection.join_to_update(stmt, arel, arel_attribute(primary_key))
-      else
-        stmt.key = arel_attribute(primary_key)
-        stmt.take(arel.limit)
-        stmt.offset(arel.offset)
-        stmt.order(*arel.orders)
-        stmt.wheres = arel.constraints
-      end
-
       @klass.connection.update stmt, "#{@klass} Update All"
     end
 
@@ -483,17 +478,12 @@ module ActiveRecord
       end
 
       stmt = Arel::DeleteManager.new
-      stmt.from(table)
-
-      if has_join_values?
-        @klass.connection.join_to_delete(stmt, arel, arel_attribute(primary_key))
-      else
-        stmt.key = arel_attribute(primary_key)
-        stmt.take(arel.limit)
-        stmt.offset(arel.offset)
-        stmt.order(*arel.orders)
-        stmt.wheres = arel.constraints
-      end
+      stmt.from(arel.join_sources.empty? ? table : arel.source)
+      stmt.key = arel_attribute(primary_key)
+      stmt.take(arel.limit)
+      stmt.offset(arel.offset)
+      stmt.order(*arel.orders)
+      stmt.wheres = arel.constraints
 
       affected = @klass.connection.delete(stmt, "#{@klass} Destroy")
 
@@ -648,10 +638,6 @@ module ActiveRecord
         end
       end
 
-      def has_join_values?
-        joins_values.any? || left_outer_joins_values.any?
-      end
-
       def exec_queries(&block)
         skip_query_cache_if_necessary do
           @records =
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)