Joined tables in association scope doesn't use the same aliases with the parent relation's aliases

Building association scope in join dependency should respect the parent
relation's aliases to avoid using the same alias name more than once.

Fixes #30681.

7 files changed, 36 insertions, 25 deletions

diff --git a/activerecord/lib/active_record/associations/alias_tracker.rb b/activerecord/lib/active_record/associations/alias_tracker.rb
index 358cdabc7f..a8d8ee268a 100644
--- a/activerecord/lib/active_record/associations/alias_tracker.rb
+++ b/activerecord/lib/active_record/associations/alias_tracker.rb
@@ -30,6 +30,8 @@ module ActiveRecord
             ).size
           elsif join.respond_to? :left
             join.left.table_name == name ? 1 : 0
+          elsif join.is_a?(Hash)
+            join[name]
           else
             # this branch is reached by two tests:
             #
@@ -73,10 +75,7 @@ module ActiveRecord
         end
       end
 
-      # TODO Change this to private once we've dropped Ruby 2.2 support.
-      # Workaround for Ruby 2.2 "private attribute?" warning.
-      protected
-        attr_reader :aliases
+      attr_reader :aliases
 
       private
 
diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb
index b70b680885..df4bf07999 100644
--- a/activerecord/lib/active_record/associations/join_dependency.rb
+++ b/activerecord/lib/active_record/associations/join_dependency.rb
@@ -43,8 +43,6 @@ module ActiveRecord
         Column = Struct.new(:name, :alias)
       end
 
-      attr_reader :alias_tracker, :base_klass, :join_root
-
       def self.make_tree(associations)
         hash = {}
         walk_tree associations, hash
@@ -158,6 +156,9 @@ module ActiveRecord
         parents.values
       end
 
+      protected
+        attr_reader :alias_tracker, :base_klass, :join_root
+
       private
 
         def make_constraints(parent, child, tables, join_type)
@@ -224,7 +225,7 @@ module ActiveRecord
               raise EagerLoadPolymorphicError.new(reflection)
             end
 
-            JoinAssociation.new reflection, build(right, reflection.klass)
+            JoinAssociation.new(reflection, build(right, reflection.klass), alias_tracker)
           end.compact
         end
 
diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb
index a526468bf6..a007d0cf5f 100644
--- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb
+++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb
@@ -11,11 +11,12 @@ module ActiveRecord
 
         attr_accessor :tables
 
-        def initialize(reflection, children)
+        def initialize(reflection, children, alias_tracker)
           super(reflection.klass, children)
 
-          @reflection      = reflection
-          @tables          = nil
+          @alias_tracker = alias_tracker
+          @reflection    = reflection
+          @tables        = nil
         end
 
         def match?(other)
@@ -38,11 +39,12 @@ module ActiveRecord
             joins << table.create_join(table, table.create_on(constraint), join_type)
 
             join_scope = reflection.join_scope(table, foreign_klass)
+            arel = join_scope.arel(alias_tracker.aliases)
 
-            if join_scope.arel.constraints.any?
-              joins.concat join_scope.arel.join_sources
+            if arel.constraints.any?
+              joins.concat arel.join_sources
               right = joins.last.right
-              right.expr = right.expr.and(join_scope.arel.constraints)
+              right.expr = right.expr.and(arel.constraints)
             end
 
             # The current table in this iteration becomes the foreign table in the next
@@ -55,6 +57,9 @@ module ActiveRecord
         def table
           tables.first
         end
+
+        protected
+          attr_reader :alias_tracker
       end
     end
   end
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index 1f59a0c53a..997cfe4b5e 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -551,7 +551,8 @@ module ActiveRecord
       limit_value || offset_value
     end
 
-    def alias_tracker(joins = []) # :nodoc:
+    def alias_tracker(joins = [], aliases = nil) # :nodoc:
+      joins += [aliases] if aliases
       ActiveRecord::Associations::AliasTracker.create(connection, table.name, joins)
     end
 
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index fb5beeffef..578d8a6eb7 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -898,8 +898,8 @@ module ActiveRecord
     end
 
     # Returns the Arel object associated with the relation.
-    def arel # :nodoc:
-      @arel ||= build_arel
+    def arel(aliases = nil) # :nodoc:
+      @arel ||= build_arel(aliases)
     end
 
     protected
@@ -921,11 +921,11 @@ module ActiveRecord
         raise ImmutableRelation if defined?(@arel) && @arel
       end
 
-      def build_arel
+      def build_arel(aliases)
         arel = Arel::SelectManager.new(table)
 
-        build_joins(arel, joins_values.flatten) unless joins_values.empty?
-        build_left_outer_joins(arel, left_outer_joins_values.flatten) unless left_outer_joins_values.empty?
+        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?
 
         arel.where(where_clause.ast) unless where_clause.empty?
         arel.having(having_clause.ast) unless having_clause.empty?
@@ -970,7 +970,7 @@ module ActiveRecord
         end
       end
 
-      def build_left_outer_joins(manager, outer_joins)
+      def build_left_outer_joins(manager, outer_joins, aliases)
         buckets = outer_joins.group_by do |join|
           case join
           when Hash, Symbol, Array
@@ -980,10 +980,10 @@ module ActiveRecord
           end
         end
 
-        build_join_query(manager, buckets, Arel::Nodes::OuterJoin)
+        build_join_query(manager, buckets, Arel::Nodes::OuterJoin, aliases)
       end
 
-      def build_joins(manager, joins)
+      def build_joins(manager, joins, aliases)
         buckets = joins.group_by do |join|
           case join
           when String
@@ -999,10 +999,10 @@ module ActiveRecord
           end
         end
 
-        build_join_query(manager, buckets, Arel::Nodes::InnerJoin)
+        build_join_query(manager, buckets, Arel::Nodes::InnerJoin, aliases)
       end
 
-      def build_join_query(manager, buckets, join_type)
+      def build_join_query(manager, buckets, join_type, aliases)
         buckets.default = []
 
         association_joins         = buckets[:association_join]
@@ -1013,7 +1013,7 @@ module ActiveRecord
         join_list = join_nodes + convert_join_strings_to_ast(manager, string_joins)
 
         join_dependency = ActiveRecord::Associations::JoinDependency.new(
-          klass, table, association_joins, alias_tracker(join_list)
+          klass, table, association_joins, alias_tracker(join_list, aliases)
         )
 
         joins = join_dependency.join_constraints(stashed_association_joins, join_type)
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index 521b388cee..c6a4ac356f 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -1250,6 +1250,10 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
     TenantMembership.current_member = nil
   end
 
+  def test_has_many_trough_with_scope_that_has_joined_same_table_with_parent_relation
+    assert_equal authors(:david), Author.joins(:comments_for_first_author).take
+  end
+
   def test_has_many_through_with_unscope_should_affect_to_through_scope
     assert_equal [comments(:eager_other_comment1)], authors(:mary).unordered_comments
   end
diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb
index 3371fcbfcc..e9eba9be2e 100644
--- a/activerecord/test/models/author.rb
+++ b/activerecord/test/models/author.rb
@@ -22,6 +22,7 @@ class Author < ActiveRecord::Base
   has_many :comments_containing_the_letter_e, through: :posts, source: :comments
   has_many :comments_with_order_and_conditions, -> { order("comments.body").where("comments.body like 'Thank%'") }, through: :posts, source: :comments
   has_many :comments_with_include, -> { includes(:post).where(posts: { type: "Post" }) }, through: :posts, source: :comments
+  has_many :comments_for_first_author, -> { for_first_author }, through: :posts, source: :comments
 
   has_many :first_posts
   has_many :comments_on_first_posts, -> { order("posts.id desc, comments.id asc") }, through: :first_posts, source: :comments