Skip to content

Commit

Permalink
Fix complicated has_many through with nested where condition
Browse files Browse the repository at this point in the history
#40779 will be caused if (1) having nested where condition for through
association, and (2) through scope has references values, and (3) the
model has no association which is the same name with table name.

In that case, the join root will be found by the table name but the join
root isn't related with association reflection.

This changes to return the table klass from join dependency tree, it
works regardless of whether the join root or join association children.

Fixes #40779.
  • Loading branch information
kamipo committed Jan 5, 2021
1 parent a94315b commit cbb19eb
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 12 deletions.
13 changes: 8 additions & 5 deletions activerecord/lib/active_record/associations/association_scope.rb
Expand Up @@ -131,11 +131,14 @@ def add_constraints(scope, owner, chain)
if scope_chain_item == chain_head.scope
scope.merge! item.except(:where, :includes, :unscope, :order)
elsif !item.references_values.empty?
join_dependency = item.construct_join_dependency(
item.eager_load_values | item.includes_values, Arel::Nodes::OuterJoin
)
scope.joins!(*item.joins_values, join_dependency)
scope.left_outer_joins!(*item.left_outer_joins_values)
scope.joins_values |= item.joins_values
scope.left_outer_joins_values |= item.left_outer_joins_values

associations = item.eager_load_values | item.includes_values

unless associations.empty?
scope.joins_values << item.construct_join_dependency(associations, Arel::Nodes::OuterJoin)
end
end

reflection.all_includes do
Expand Down
8 changes: 4 additions & 4 deletions activerecord/lib/active_record/relation/query_methods.rb
Expand Up @@ -1140,7 +1140,7 @@ def build_where_clause(opts, rest = []) # :nodoc:
self.references_values |= references unless references.empty?

parts = predicate_builder.build_from_hash(opts) do |table_name|
lookup_reflection_from_join_dependencies(table_name)
lookup_table_klass_from_join_dependencies(table_name)
end
when Arel::Nodes::Node
parts = [opts]
Expand All @@ -1153,9 +1153,9 @@ def build_where_clause(opts, rest = []) # :nodoc:
alias :build_having_clause :build_where_clause

private
def lookup_reflection_from_join_dependencies(table_name)
def lookup_table_klass_from_join_dependencies(table_name)
each_join_dependencies do |join|
return join.reflection if table_name == join.table_name
return join.base_klass if table_name == join.table_name
end
nil
end
Expand Down Expand Up @@ -1366,7 +1366,7 @@ def arel_column(field)
elsif field.match?(/\A\w+\.\w+\z/)
table, column = field.split(".")
predicate_builder.resolve_arel_attribute(table, column) do
lookup_reflection_from_join_dependencies(table)
lookup_table_klass_from_join_dependencies(table)
end
else
yield field
Expand Down
9 changes: 6 additions & 3 deletions activerecord/lib/active_record/table_metadata.rb
Expand Up @@ -33,10 +33,13 @@ def associated_table(table_name)
return self
end

reflection ||= yield table_name if block_given?
if reflection
association_klass = reflection.klass unless reflection.polymorphic?
elsif block_given?
association_klass = yield table_name
end

if reflection && !reflection.polymorphic?
association_klass = reflection.klass
if association_klass
arel_table = association_klass.arel_table
arel_table = arel_table.alias(table_name) if arel_table.name != table_name
TableMetadata.new(association_klass, arel_table, reflection)
Expand Down
Expand Up @@ -66,6 +66,14 @@ def test_through_association_with_left_joins
assert_equal [comments(:eager_other_comment1)], authors(:mary).comments.merge(Post.left_joins(:comments))
end

def test_through_association_with_through_scope_and_nested_where
company = Company.create!(name: "special")
developer = SpecialDeveloper.create!
SpecialContract.create!(company: company, special_developer: developer)

assert_equal [developer], company.special_developers.where.not("contracts.id": nil)
end

def test_preload_with_nested_association
posts = Post.where(id: [authors(:david).id, authors(:mary).id]).
preload(:author, :author_favorites_with_scope).order(:id).to_a
Expand Down
2 changes: 2 additions & 0 deletions activerecord/test/models/company.rb
Expand Up @@ -13,6 +13,8 @@ class Company < AbstractCompany
has_one :dummy_account, foreign_key: "firm_id", class_name: "Account"
has_many :contracts
has_many :developers, through: :contracts
has_many :special_contracts, -> { includes(:special_developer).where.not("developers.id": nil) }
has_many :special_developers, through: :special_contracts

alias_attribute :new_name, :name
attribute :metadata, :json
Expand Down
6 changes: 6 additions & 0 deletions activerecord/test/models/contract.rb
Expand Up @@ -30,3 +30,9 @@ def update_metadata
class NewContract < Contract
validates :company_id, presence: true
end

class SpecialContract < ActiveRecord::Base
self.table_name = "contracts"
belongs_to :company
belongs_to :special_developer, foreign_key: "developer_id"
end
5 changes: 5 additions & 0 deletions activerecord/test/models/developer.rb
Expand Up @@ -114,6 +114,11 @@ def track_instance_count
class SubDeveloper < Developer
end

class SpecialDeveloper < ActiveRecord::Base
self.table_name = "developers"
has_many :special_contracts, foreign_key: "developer_id"
end

class SymbolIgnoredDeveloper < ActiveRecord::Base
self.table_name = "developers"
self.ignored_columns = [:first_name, :last_name]
Expand Down

0 comments on commit cbb19eb

Please sign in to comment.