Skip to content

Commit

Permalink
Deduplicate joins values
Browse files Browse the repository at this point in the history
rails#36805 have one possible regression that failing deduplication if
`joins_values` have complex order (e.g. `joins_values = [join_node_a,
:comments, :tags, join_node_a]`).

This fixes the deduplication to take it in the first phase before
grouping.
  • Loading branch information
kamipo committed Aug 1, 2019
1 parent ad95727 commit e6f953f
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 3 deletions.
6 changes: 3 additions & 3 deletions activerecord/lib/active_record/relation/query_methods.rb
Expand Up @@ -1114,7 +1114,7 @@ def build_joins(manager, joins, aliases)
else
join
end
end.compact_blank!
end.compact_blank!.uniq!

while joins.first.is_a?(Arel::Nodes::Join)
join_node = joins.shift
Expand Down Expand Up @@ -1144,8 +1144,8 @@ def build_joins(manager, joins, aliases)
def build_join_query(manager, buckets, join_type, aliases)
association_joins = buckets[:association_join]
stashed_joins = buckets[:stashed_join]
leading_joins = buckets[:leading_join].tap(&:uniq!)
join_nodes = buckets[:join_node].tap(&:uniq!)
leading_joins = buckets[:leading_join]
join_nodes = buckets[:join_node]

join_sources = manager.join_sources
join_sources.concat(leading_joins) unless leading_joins.empty?
Expand Down
Expand Up @@ -69,6 +69,16 @@ def test_user_supplied_joins_order_should_be_preserved
assert_equal [expected], Person.joins(string_join).joins(agents.create_join(agents, agents.create_on(constraint)))
end

def test_deduplicate_joins
posts = Post.arel_table
constraint = posts[:author_id].eq(Author.arel_attribute(:id))

authors = Author.joins(posts.create_join(posts, posts.create_on(constraint)))
authors = authors.joins(:author_address).merge(authors.where("posts.type": "SpecialPost"))

assert_equal [authors(:david)], authors
end

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

0 comments on commit e6f953f

Please sign in to comment.