Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Maintain eager loading joining order as before #37235

Merged
merged 1 commit into from Sep 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -70,6 +70,10 @@ def initialize(base, table, associations, join_type)
@join_type = join_type
end

def base_klass
join_root.base_klass
end

def reflections
join_root.drop(1).map!(&:reflection)
end
Expand Down
4 changes: 4 additions & 0 deletions activerecord/lib/active_record/relation/query_methods.rb
Expand Up @@ -1108,6 +1108,10 @@ def build_joins(manager, joins, aliases)
buckets[:stashed_join] << construct_join_dependency(left_joins, Arel::Nodes::OuterJoin)
end

if joins.last.is_a?(ActiveRecord::Associations::JoinDependency)
buckets[:stashed_join] << joins.pop if joins.last.base_klass == klass
end

joins.map! do |join|
if join.is_a?(String)
table.create_string_join(Arel.sql(join.strip)) unless join.blank?
Expand Down
Expand Up @@ -79,6 +79,14 @@ def test_deduplicate_joins
assert_equal [authors(:david)], authors
end

def test_eager_load_with_string_joins
string_join = <<~SQL
LEFT JOIN people agents_people ON agents_people.primary_contact_id = agents_people_2.id AND agents_people.id > agents_people_2.id
SQL

assert_equal 3, Person.eager_load(:agents).joins(string_join).count
end

def test_construct_finder_sql_ignores_empty_joins_hash
sql = Author.joins({}).to_sql
assert_no_match(/JOIN/i, sql)
Expand Down