mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Keep INNER JOIN when merging relations
Doing `Author.joins(:posts).merge(Post.joins(:comments))` does this `SELECT ... INNER JOIN posts ON... LEFT OUTER JOIN comments ON...` instead of doing `SELECT ... INNER JOIN posts ON... INNER JOIN comments ON...`. This behavior is unexpected and makes little sense as, basically, doing `Post.joins(:comments)` means I want posts that have comments. Turning it to a LEFT JOIN means I want posts and join the comments data, if any. We can see this problem directly in the existing tests. The test_relation_merging_with_merged_joins_as_symbols only does joins from posts to comments to ratings while the ratings fixture isn't loaded, but the count is non-zero.
This commit is contained in:
parent
0787727cd6
commit
249ddd0c39
3 changed files with 40 additions and 5 deletions
|
@ -1,3 +1,19 @@
|
|||
* Merging two relations representing nested joins no longer transforms the joins of
|
||||
the merged relation into LEFT OUTER JOIN. Example to clarify:
|
||||
|
||||
```
|
||||
Author.joins(:posts).merge(Post.joins(:comments))
|
||||
# Before the change:
|
||||
#=> SELECT ... FROM authors INNER JOIN posts ON ... LEFT OUTER JOIN comments ON...
|
||||
|
||||
# After the change:
|
||||
#=> SELECT ... FROM authors INNER JOIN posts ON ... INNER JOIN comments ON...
|
||||
```
|
||||
|
||||
TODO: Add to the Rails 5.2 upgrade guide
|
||||
|
||||
*Maxime Handfield Lapointe*
|
||||
|
||||
* `ActiveRecord::Persistence#touch` does not work well when optimistic locking enabled and
|
||||
`locking_column`, without default value, is null in the database.
|
||||
|
||||
|
|
|
@ -104,17 +104,17 @@ module ActiveRecord
|
|||
join_root.drop(1).map!(&:reflection)
|
||||
end
|
||||
|
||||
def join_constraints(outer_joins, join_type)
|
||||
def join_constraints(joins_to_add, join_type)
|
||||
joins = join_root.children.flat_map { |child|
|
||||
make_join_constraints(join_root, child, join_type)
|
||||
}
|
||||
|
||||
joins.concat outer_joins.flat_map { |oj|
|
||||
joins.concat joins_to_add.flat_map { |oj|
|
||||
if join_root.match? oj.join_root
|
||||
walk join_root, oj.join_root
|
||||
else
|
||||
oj.join_root.children.flat_map { |child|
|
||||
make_outer_joins oj.join_root, child
|
||||
make_join_constraints(oj.join_root, child, join_type)
|
||||
}
|
||||
end
|
||||
}
|
||||
|
|
|
@ -6,7 +6,7 @@ require "models/rating"
|
|||
|
||||
module ActiveRecord
|
||||
class RelationTest < ActiveRecord::TestCase
|
||||
fixtures :posts, :comments, :authors, :author_addresses
|
||||
fixtures :posts, :comments, :authors, :author_addresses, :ratings
|
||||
|
||||
FakeKlass = Struct.new(:table_name, :name) do
|
||||
extend ActiveRecord::Delegation::DelegateCache
|
||||
|
@ -224,7 +224,26 @@ module ActiveRecord
|
|||
def test_relation_merging_with_merged_joins_as_symbols
|
||||
special_comments_with_ratings = SpecialComment.joins(:ratings)
|
||||
posts_with_special_comments_with_ratings = Post.group("posts.id").joins(:special_comments).merge(special_comments_with_ratings)
|
||||
assert_equal({ 2 => 1, 4 => 3, 5 => 1 }, authors(:david).posts.merge(posts_with_special_comments_with_ratings).count)
|
||||
assert_equal({ 4 => 2 }, authors(:david).posts.merge(posts_with_special_comments_with_ratings).count)
|
||||
end
|
||||
|
||||
def test_relation_merging_with_merged_symbol_joins_keeps_inner_joins
|
||||
queries = capture_sql { authors_with_commented_posts = Author.joins(:posts).merge(Post.joins(:comments)).to_a }
|
||||
|
||||
nb_inner_join = queries.sum { |sql| sql.scan(/INNER\s+JOIN/i).size }
|
||||
assert_equal 2, nb_inner_join, "Wrong amount of INNER JOIN in query"
|
||||
assert queries.none? { |sql| /LEFT\s+(OUTER)?\s+JOIN/i.match?(sql) }, "Shouldn't have any LEFT JOIN in query"
|
||||
end
|
||||
|
||||
def test_relation_merging_with_merged_symbol_joins_has_correct_size_and_count
|
||||
# Has one entry per comment
|
||||
merged_authors_with_commented_posts_relation = Author.joins(:posts).merge(Post.joins(:comments))
|
||||
|
||||
post_ids_with_author = Post.joins(:author).pluck(:id)
|
||||
manual_comments_on_post_that_have_author = Comment.where(post_id: post_ids_with_author).pluck(:id)
|
||||
|
||||
assert_equal manual_comments_on_post_that_have_author.size, merged_authors_with_commented_posts_relation.count
|
||||
assert_equal manual_comments_on_post_that_have_author.size, merged_authors_with_commented_posts_relation.to_a.size
|
||||
end
|
||||
|
||||
def test_relation_merging_with_joins_as_join_dependency_pick_proper_parent
|
||||
|
|
Loading…
Reference in a new issue