From 9809d4d21bcb4eae153b354f9c3ac3399af203e4 Mon Sep 17 00:00:00 2001 From: Aaron Frase Date: Thu, 31 Oct 2019 20:53:49 -0400 Subject: [PATCH] Avoid stack level too deep in predicate builder Check if the query is different from attributes before recursively calling `expand_from_hash` --- .../lib/active_record/relation/predicate_builder.rb | 4 +++- .../test/cases/associations/has_many_associations_test.rb | 7 +++++++ activerecord/test/fixtures/comments.yml | 7 +++++++ activerecord/test/fixtures/companies.yml | 5 +++++ activerecord/test/models/comment.rb | 1 + activerecord/test/models/company.rb | 1 + activerecord/test/schema/schema.rb | 1 + 7 files changed, 25 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 240de3bb693f3..d05edbdf68067 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -85,7 +85,9 @@ def expand_from_hash(attributes) klass ||= AssociationQueryValue queries = klass.new(associated_table, value).queries.map do |query| - expand_from_hash(query).reduce(&:and) + # 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 ? build(table.arel_attribute(key), value) : expand_from_hash(query).reduce(&:and) end queries.reduce(&:or) elsif table.aggregated_with?(key) diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index cb3985a45cbd3..77591c90e6adb 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -2991,6 +2991,13 @@ def test_has_many_preloading_with_duplicate_records assert_equal [1, 2], posts.first.comments.map(&:id) 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 diff --git a/activerecord/test/fixtures/comments.yml b/activerecord/test/fixtures/comments.yml index ddbb823c49f7a..33dc9462602c8 100644 --- a/activerecord/test/fixtures/comments.yml +++ b/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 diff --git a/activerecord/test/fixtures/companies.yml b/activerecord/test/fixtures/companies.yml index ab9d5378ad506..b05997038a5fa 100644 --- a/activerecord/test/fixtures/companies.yml +++ b/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 diff --git a/activerecord/test/models/comment.rb b/activerecord/test/models/comment.rb index f0f057670957f..23fa006bc70ef 100644 --- a/activerecord/test/models/comment.rb +++ b/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 diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 339b5c8ca88e6..7946818540ea0 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -12,6 +12,7 @@ class Company < AbstractCompany has_one :dummy_account, foreign_key: "firm_id", class_name: "Account" has_many :contracts has_many :developers, through: :contracts + has_many :comments, foreign_key: 'company' attribute :metadata, :json diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index cae2890c9e1ba..c98b26f28d8dc 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -213,6 +213,7 @@ t.datetime :updated_at t.datetime :deleted_at t.integer :comments + t.integer :company end create_table :companies, force: true do |t|