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.

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
 

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
 

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

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
 

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)

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

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