From 8f05035b7e595e2086759ee10ec9df9431e5e351 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 5 Apr 2019 06:09:44 +0900
Subject: Stash `left_joins` into `joins` to deduplicate redundant LEFT JOIN

Originally the `JoinDependency` has the deduplication for eager loading
(LEFT JOIN). This re-uses that deduplication for `left_joins`.

And also, This makes left join order into part of joins, i.e.:

Before:

```
association joins -> stash joins (eager loading, etc) -> string joins -> left joins
```

After:

```
association joins -> stash joins (eager loading, left joins, etc) -> string joins
```

Now string joins are able to refer left joins.

Fixes #34325.
Fixes #34332.
Fixes #34536.
---
 .../lib/active_record/relation/finder_methods.rb   |  6 -----
 activerecord/lib/active_record/relation/merger.rb  | 23 +++++-----------
 .../lib/active_record/relation/query_methods.rb    | 31 ++++++++++++++++------
 3 files changed, 30 insertions(+), 30 deletions(-)

(limited to 'activerecord/lib')

diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb
index e2efd4aa0d..fcb0da1a42 100644
--- a/activerecord/lib/active_record/relation/finder_methods.rb
+++ b/activerecord/lib/active_record/relation/finder_methods.rb
@@ -370,12 +370,6 @@ module ActiveRecord
         relation
       end
 
-      def construct_join_dependency(associations)
-        ActiveRecord::Associations::JoinDependency.new(
-          klass, table, associations
-        )
-      end
-
       def apply_join_dependency(eager_loading: group_values.empty?)
         join_dependency = construct_join_dependency(eager_load_values + includes_values)
         relation = except(:includes, :eager_load, :preload).joins!(join_dependency)
diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb
index 4de7465128..6bb77b355c 100644
--- a/activerecord/lib/active_record/relation/merger.rb
+++ b/activerecord/lib/active_record/relation/merger.rb
@@ -117,16 +117,14 @@ module ActiveRecord
           if other.klass == relation.klass
             relation.joins!(*other.joins_values)
           else
-            joins_dependency = other.joins_values.map do |join|
+            associations, others = other.joins_values.partition do |join|
               case join
-              when Hash, Symbol, Array
-                other.send(:construct_join_dependency, join)
-              else
-                join
+              when Hash, Symbol, Array; true
               end
             end
 
-            relation.joins!(*joins_dependency)
+            join_dependency = other.construct_join_dependency(associations)
+            relation.joins!(join_dependency, *others)
           end
         end
 
@@ -136,16 +134,9 @@ module ActiveRecord
           if other.klass == relation.klass
             relation.left_outer_joins!(*other.left_outer_joins_values)
           else
-            joins_dependency = other.left_outer_joins_values.map do |join|
-              case join
-              when Hash, Symbol, Array
-                other.send(:construct_join_dependency, join)
-              else
-                join
-              end
-            end
-
-            relation.left_outer_joins!(*joins_dependency)
+            associations = other.left_outer_joins_values
+            join_dependency = other.construct_join_dependency(associations)
+            relation.joins!(join_dependency)
           end
         end
 
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index 5f728f2263..7ea18c3069 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -992,6 +992,12 @@ module ActiveRecord
       @arel ||= build_arel(aliases)
     end
 
+    def construct_join_dependency(associations) # :nodoc:
+      ActiveRecord::Associations::JoinDependency.new(
+        klass, table, associations
+      )
+    end
+
     protected
       def build_subquery(subquery_alias, select_value) # :nodoc:
         subquery = except(:optimizer_hints).arel.as(subquery_alias)
@@ -1021,8 +1027,11 @@ module ActiveRecord
       def build_arel(aliases)
         arel = Arel::SelectManager.new(table)
 
-        aliases = build_joins(arel, joins_values.flatten, aliases) unless joins_values.empty?
-        build_left_outer_joins(arel, left_outer_joins_values.flatten, aliases) unless left_outer_joins_values.empty?
+        if !joins_values.empty?
+          build_joins(arel, joins_values.flatten, aliases)
+        elsif !left_outer_joins_values.empty?
+          build_left_outer_joins(arel, left_outer_joins_values.flatten, aliases)
+        end
 
         arel.where(where_clause.ast) unless where_clause.empty?
         arel.having(having_clause.ast) unless having_clause.empty?
@@ -1072,22 +1081,28 @@ module ActiveRecord
         end
       end
 
-      def build_left_outer_joins(manager, outer_joins, aliases)
-        buckets = outer_joins.group_by do |join|
-          case join
+      def valid_association_list(associations)
+        associations.each do |association|
+          case association
           when Hash, Symbol, Array
-            :association_join
-          when ActiveRecord::Associations::JoinDependency
-            :stashed_join
+            # valid
           else
             raise ArgumentError, "only Hash, Symbol and Array are allowed"
           end
         end
+      end
 
+      def build_left_outer_joins(manager, outer_joins, aliases)
+        buckets = { association_join: valid_association_list(outer_joins) }
         build_join_query(manager, buckets, Arel::Nodes::OuterJoin, aliases)
       end
 
       def build_joins(manager, joins, aliases)
+        unless left_outer_joins_values.empty?
+          left_joins = valid_association_list(left_outer_joins_values.flatten)
+          joins << construct_join_dependency(left_joins)
+        end
+
         buckets = joins.group_by do |join|
           case join
           when String
-- 
cgit v1.2.3