Merge pull request #41394 from afrase/recursive-association-fix

Avoid stack level too deep in predicate builder

activerecord/lib/active_record/relation/predicate_builder.rb

@@ -103,7 +103,9 @@ def expand_from_hash(attributes, &block)
 
             klass ||= AssociationQueryValue
             queries = klass.new(associated_table, value).queries.map! do |query|
-              expand_from_hash(query)
+              # If the query produced is identical to attributes don't go any deeper.
+              # Prevents stack level too deep errors when association and foreign_key are identical.
+              query == attributes ? self[key, value] : expand_from_hash(query)
             end
 
             grouping_queries(queries)

activerecord/test/cases/associations/cascaded_eager_loading_test.rb

@@ -22,23 +22,23 @@ def test_eager_association_loading_with_cascaded_two_levels
     assert_equal 3, authors.size
     assert_equal 5, authors[0].posts.size
     assert_equal 3, authors[1].posts.size
-    assert_equal 10, authors[0].posts.collect { |post| post.comments.size }.inject(0) { |sum, i| sum + i }
+    assert_equal 11, authors[0].posts.collect { |post| post.comments.size }.inject(0) { |sum, i| sum + i }
   end
 
   def test_eager_association_loading_with_cascaded_two_levels_and_one_level
     authors = Author.includes({ posts: :comments }, :categorizations).order(:id).to_a
     assert_equal 3, authors.size
     assert_equal 5, authors[0].posts.size
     assert_equal 3, authors[1].posts.size
-    assert_equal 10, authors[0].posts.collect { |post| post.comments.size }.inject(0) { |sum, i| sum + i }
+    assert_equal 11, authors[0].posts.collect { |post| post.comments.size }.inject(0) { |sum, i| sum + i }
     assert_equal 1, authors[0].categorizations.size
     assert_equal 2, authors[1].categorizations.size
   end
 
   def test_eager_association_loading_with_hmt_does_not_table_name_collide_when_joining_associations
     authors = Author.joins(:posts).eager_load(:comments).where(posts: { tags_count: 1 }).order(:id).to_a
     assert_equal 3, assert_queries(0) { authors.size }
-    assert_equal 10, assert_queries(0) { authors[0].comments.size }
+    assert_equal 11, assert_queries(0) { authors[0].comments.size }
   end
 
   def test_eager_association_loading_grafts_stashed_associations_to_correct_parent
@@ -82,7 +82,7 @@ def test_eager_association_loading_with_cascaded_two_levels_with_two_has_many_as
     assert_equal 3, authors.size
     assert_equal 5, authors[0].posts.size
     assert_equal 3, authors[1].posts.size
-    assert_equal 10, authors[0].posts.collect { |post| post.comments.size }.inject(0) { |sum, i| sum + i }
+    assert_equal 11, authors[0].posts.collect { |post| post.comments.size }.inject(0) { |sum, i| sum + i }
   end
 
   def test_eager_association_loading_with_cascaded_two_levels_and_self_table_reference
@@ -101,7 +101,7 @@ def test_eager_association_loading_with_cascaded_two_levels_with_condition
 
   def test_eager_association_loading_with_cascaded_three_levels_by_ping_pong
     firms = Firm.all.merge!(includes: { account: { firm: :account } }, order: "companies.id").to_a
-    assert_equal 2, firms.size
+    assert_equal 3, firms.size
     assert_equal firms.first.account, firms.first.account.firm.account
     assert_equal companies(:first_firm).account, assert_queries(0) { firms.first.account.firm.account }
     assert_equal companies(:first_firm).account.firm.account, assert_queries(0) { firms.first.account.firm.account }
@@ -184,7 +184,7 @@ def test_eager_association_loading_with_cascaded_interdependent_one_level_and_tw
     authors_relation = Author.all.merge!(includes: [:comments, { posts: :categorizations }], order: "authors.id")
     authors = authors_relation.to_a
     assert_equal 3, authors.size
-    assert_equal 10, authors[0].comments.size
+    assert_equal 11, authors[0].comments.size
     assert_equal 1, authors[1].comments.size
     assert_equal 5, authors[0].posts.size
     assert_equal 3, authors[1].posts.size

activerecord/test/cases/associations/eager_test.rb

@@ -414,7 +414,7 @@ def test_eager_association_loading_with_belongs_to_and_foreign_keys
 
   def test_eager_association_loading_with_belongs_to
     comments = Comment.all.merge!(includes: :post).to_a
-    assert_equal 11, comments.length
+    assert_equal 12, comments.length
     titles = comments.map { |c| c.post.title }
     assert_includes titles, posts(:welcome).title
     assert_includes titles, posts(:sti_post_and_comments).title
@@ -1175,8 +1175,8 @@ def test_eager_loading_with_order_on_joined_table_preloads
     posts = assert_queries(2) do
       Post.all.merge!(joins: :comments, includes: :author, order: "comments.id DESC").to_a
     end
-    assert_equal posts(:eager_other), posts[1]
-    assert_equal authors(:mary), assert_no_queries { posts[1].author }
+    assert_equal posts(:eager_other), posts[2]
+    assert_equal authors(:mary), assert_no_queries { posts[2].author }
   end
 
   def test_eager_loading_with_conditions_on_joined_table_preloads

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -2125,7 +2125,7 @@ def test_modifying_a_through_a_has_many_should_raise
 
   def test_associations_order_should_be_priority_over_throughs_order
     original = authors(:david)
-    expected = [12, 10, 9, 8, 7, 6, 5, 3, 2, 1]
+    expected = [13, 12, 10, 9, 8, 7, 6, 5, 3, 2, 1]
     assert_equal expected, original.comments_desc.map(&:id)
     preloaded = Author.includes(:comments_desc).find(original.id)
     assert_equal expected, preloaded.comments_desc.map(&:id)
@@ -3060,6 +3060,13 @@ def test_has_many_preloading_with_duplicate_records
     assert_equal [1, 2], posts.first.comments.map(&:id).sort
   end
 
+  def test_has_many_association_with_same_foreign_key_name
+    assert_nothing_raised do
+      firm = Firm.find(15)
+      assert_not_nil(firm.comments.first)
+    end
+  end
+
   private
     def force_signal37_to_load_all_clients_of_firm
       companies(:first_firm).clients_of_firm.load_target

activerecord/test/cases/associations/join_model_test.rb

@@ -432,7 +432,7 @@ def test_eager_load_has_many_through_has_many
     author = Author.all.merge!(where: ["name = ?", "David"], includes: :comments, order: "comments.id").first
     SpecialComment.new; VerySpecialComment.new
     assert_no_queries do
-      assert_equal [1, 2, 3, 5, 6, 7, 8, 9, 10, 12], author.comments.collect(&:id)
+      assert_equal [1, 2, 3, 5, 6, 7, 8, 9, 10, 12, 13], author.comments.collect(&:id)
     end
   end
 
@@ -537,7 +537,7 @@ def test_add_to_join_table_with_no_id
 
   def test_has_many_through_collection_size_doesnt_load_target_if_not_loaded
     author = authors(:david)
-    assert_equal 10, author.comments.size
+    assert_equal 11, author.comments.size
     assert_not_predicate author.comments, :loaded?
   end
 
@@ -748,7 +748,7 @@ def test_has_many_through_goes_through_all_sti_classes
     sub_sti_post = SubStiPost.create!(title: "test", body: "test", author_id: 1)
     new_comment = sub_sti_post.comments.create(body: "test")
 
-    assert_equal [9, 10, new_comment.id], authors(:david).sti_post_comments.map(&:id).sort
+    assert_equal [9, 10, 13, new_comment.id], authors(:david).sti_post_comments.map(&:id).sort
   end
 
   def test_has_many_with_pluralize_table_names_false

activerecord/test/cases/associations/left_outer_join_association_test.rb

@@ -15,10 +15,10 @@ class LeftOuterJoinAssociationTest < ActiveRecord::TestCase
 
   def test_merging_multiple_left_joins_from_different_associations
     count = Author.joins(:posts).merge(Post.left_joins(:comments).merge(Comment.left_joins(:ratings))).count
-    assert_equal 16, count
+    assert_equal 17, count
 
     count = Author.left_joins(:posts).merge(Post.left_joins(:comments).merge(Comment.left_joins(:ratings))).count
-    assert_equal 16, count
+    assert_equal 17, count
   end
 
   def test_construct_finder_sql_applies_aliases_tables_on_association_conditions
@@ -37,8 +37,8 @@ def test_construct_finder_sql_does_not_table_name_collide_on_duplicate_associati
   end
 
   def test_left_outer_joins_count_is_same_as_size_of_loaded_results
-    assert_equal 17, Post.left_outer_joins(:comments).to_a.size
-    assert_equal 17, Post.left_outer_joins(:comments).count
+    assert_equal 18, Post.left_outer_joins(:comments).to_a.size
+    assert_equal 18, Post.left_outer_joins(:comments).count
   end
 
   def test_merging_left_joins_should_be_left_joins
@@ -80,7 +80,7 @@ def test_left_outer_joins_forbids_to_use_string_as_argument
   end
 
   def test_left_outer_joins_with_string_join
-    assert_equal 16, Author.left_outer_joins(:posts).joins("LEFT OUTER JOIN comments ON comments.post_id = posts.id").count
+    assert_equal 17, Author.left_outer_joins(:posts).joins("LEFT OUTER JOIN comments ON comments.post_id = posts.id").count
   end
 
   def test_left_outer_joins_with_arel_join
@@ -89,7 +89,7 @@ def test_left_outer_joins_with_arel_join
     constraint = comments[:post_id].eq(posts[:id])
     arel_join = comments.create_join(comments, comments.create_on(constraint), Arel::Nodes::OuterJoin)
 
-    assert_equal 16, Author.left_outer_joins(:posts).joins(arel_join).count
+    assert_equal 17, Author.left_outer_joins(:posts).joins(arel_join).count
   end
 
   def test_join_conditions_added_to_join_clause

activerecord/test/cases/calculations_test.rb

@@ -465,15 +465,15 @@ def test_should_calculate_grouped_by_function
     assert_equal 2, c[nil]
     assert_equal 1, c["DEPENDENTFIRM"]
     assert_equal 5, c["CLIENT"]
-    assert_equal 2, c["FIRM"]
+    assert_equal 3, c["FIRM"]
   end
 
   def test_should_calculate_grouped_by_function_with_table_alias
     c = Company.group("UPPER(companies.#{QUOTED_TYPE})").count(:all)
     assert_equal 2, c[nil]
     assert_equal 1, c["DEPENDENTFIRM"]
     assert_equal 5, c["CLIENT"]
-    assert_equal 2, c["FIRM"]
+    assert_equal 3, c["FIRM"]
   end
 
   def test_should_not_overshadow_enumerable_sum
@@ -608,7 +608,7 @@ def test_should_count_field_in_joined_table_with_group_by
   end
 
   def test_should_count_field_of_root_table_with_conflicting_group_by_column
-    expected = { 1 => 2, 2 => 1, 4 => 5, 5 => 2, 7 => 1 }
+    expected = { 1 => 2, 2 => 1, 4 => 5, 5 => 3, 7 => 1 }
     assert_equal expected, Post.joins(:comments).group(:post_id).count
     assert_equal expected, Post.joins(:comments).group("comments.post_id").count
     assert_equal expected, Post.joins(:comments).group(:post_id).select("DISTINCT posts.author_id").count(:all)
@@ -865,7 +865,7 @@ def test_group_by_with_order_by_virtual_count_attribute
   end if current_adapter?(:PostgreSQLAdapter)
 
   def test_group_by_with_limit
-    expected = { "StiPost" => 2, "SpecialPost" => 1 }
+    expected = { "StiPost" => 3, "SpecialPost" => 1 }
     actual = Post.includes(:comments).group(:type).order(type: :desc).limit(2).count("comments.id")
     assert_equal expected, actual
   end

activerecord/test/cases/finder_test.rb

@@ -1035,7 +1035,7 @@ def test_find_on_hash_conditions_with_explicit_table_name_and_aggregate
   end
 
   def test_find_on_association_proxy_conditions
-    assert_equal [1, 2, 3, 5, 6, 7, 8, 9, 10, 12], Comment.where(post_id: authors(:david).posts).map(&:id).sort
+    assert_equal [1, 2, 3, 5, 6, 7, 8, 9, 10, 12, 13], Comment.where(post_id: authors(:david).posts).map(&:id).sort
   end
 
   def test_find_on_hash_conditions_with_range
@@ -1435,8 +1435,8 @@ def test_select_value
   end
 
   def test_select_values
-    assert_equal ["1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11"], Company.connection.select_values("SELECT id FROM companies ORDER BY id").map!(&:to_s)
-    assert_equal ["37signals", "Summit", "Microsoft", "Flamboyant Software", "Ex Nihilo", "RailsCore", "Leetsoft", "Jadedpixel", "Odegy", "Ex Nihilo Part Deux", "Apex"], Company.connection.select_values("SELECT name FROM companies ORDER BY id")
+    assert_equal ["1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "15"], Company.connection.select_values("SELECT id FROM companies ORDER BY id").map!(&:to_s)
+    assert_equal ["37signals", "Summit", "Microsoft", "Flamboyant Software", "Ex Nihilo", "RailsCore", "Leetsoft", "Jadedpixel", "Odegy", "Ex Nihilo Part Deux", "Apex", "RVshare"], Company.connection.select_values("SELECT name FROM companies ORDER BY id")
   end
 
   def test_select_rows

activerecord/test/cases/inheritance_test.rb

@@ -392,8 +392,8 @@ def test_new_with_autoload_paths
   end
 
   def test_inheritance_condition
-    assert_equal 11, Company.count
-    assert_equal 2, Firm.count
+    assert_equal 12, Company.count
+    assert_equal 3, Firm.count
     assert_equal 5, Client.count
   end
 
@@ -429,7 +429,7 @@ def test_alt_update_all_within_inheritance
   def test_destroy_all_within_inheritance
     Client.destroy_all
     assert_equal 0, Client.count
-    assert_equal 2, Firm.count
+    assert_equal 3, Firm.count
   end
 
   def test_alt_destroy_all_within_inheritance

activerecord/test/cases/scoping/relation_scoping_test.rb

@@ -439,7 +439,7 @@ def test_forwarding_of_static_methods
   end
 
   def test_forwarding_to_scoped
-    assert_equal 4, Comment.search_by_type("Comment").size
+    assert_equal 5, Comment.search_by_type("Comment").size
     assert_equal 2, @welcome.comments.search_by_type("Comment").size
   end
 

activerecord/test/fixtures/comments.yml

@@ -63,3 +63,10 @@ sub_special_comment:
   post_id: 4
   body: Sub special comment
   type: SubSpecialComment
+
+recursive_association_comment:
+  id: 13
+  post_id: 5
+  body: afrase
+  type: Comment
+  company: 15

activerecord/test/fixtures/companies.yml

@@ -65,3 +65,8 @@ another_first_firm_client:
   client_of: 1
   name: Apex
   firm_name: 37signals
+
+recursive_association_fk:
+  id: 15
+  type: Firm
+  name: RVshare

activerecord/test/models/comment.rb

@@ -14,6 +14,7 @@ class Comment < ActiveRecord::Base
   belongs_to :post, counter_cache: true
   belongs_to :author,   polymorphic: true
   belongs_to :resource, polymorphic: true
+  belongs_to :company, foreign_key: "company"
 
   has_many :ratings
 

activerecord/test/models/company.rb

@@ -15,6 +15,7 @@ class Company < AbstractCompany
   has_many :developers, through: :contracts
   has_many :special_contracts, -> { includes(:special_developer).where.not("developers.id": nil) }
   has_many :special_developers, through: :special_contracts
+  has_many :comments, foreign_key: "company"
 
   alias_attribute :new_name, :name
   attribute :metadata, :json

activerecord/test/schema/schema.rb

@@ -225,6 +225,7 @@
     t.datetime :updated_at
     t.datetime :deleted_at
     t.integer :comments
+    t.integer :company
   end
 
   create_table :companies, force: true do |t|