From 7412b7f8a6a2634548671c8ca16941796fac87c4 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Sun, 19 May 2019 19:13:33 +0900
Subject: Implicit through table joins should be appeared before user supplied
joins
#36293 was an issue for through association with `joins` for a long
time, but since #35864 through association with `left_joins` would also
be affected by the issue.
Implicit through table joins should be appeared before user supplied
joins, otherwise loading through association with joins will cause a
statement invalid error.
Fixes #36293.
```
% ARCONN=postgresql bundle exec ruby -w -Itest test/cases/associations/has_many_through_associations_test
.rb -n test_through_association_with_joins
Using postgresql
Run options: -n test_through_association_with_joins --seed 7116
# Running:
E
Error:
HasManyThroughAssociationsTest#test_through_association_with_joins:
ActiveRecord::StatementInvalid: PG::UndefinedTable: ERROR: missing FROM-clause entry for table "posts"
LINE 1: ... "comments_posts" ON "comments_posts"."post_id" = "posts"."i...
^
: SELECT "comments".* FROM "comments" INNER JOIN "comments" "comments_posts" ON "comments_posts"."post_id" = "posts"."id" INNER JOIN "posts" ON "comments"."post_id" = "posts"."id" WHERE "posts"."author_id" = $1
rails test test/cases/associations/has_many_through_associations_test.rb:61
Finished in 0.388657s, 2.5730 runs/s, 0.0000 assertions/s.
1 runs, 0 assertions, 0 failures, 1 errors, 0 skips
```
---
.../lib/active_record/relation/query_methods.rb | 28 +++++++++-------------
.../has_many_through_associations_test.rb | 8 +++++++
2 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index b8fd2fce14..50ff733dc7 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -1128,27 +1128,21 @@ module ActiveRecord
association_joins = buckets[:association_join]
stashed_joins = buckets[:stashed_join]
- join_nodes = buckets[:join_node].uniq
- string_joins = buckets[:string_join].map(&:strip).uniq
+ join_nodes = buckets[:join_node].tap(&:uniq!)
+ string_joins = buckets[:string_join].delete_if(&:blank?).map!(&:strip).tap(&:uniq!)
- join_list = join_nodes + convert_join_strings_to_ast(string_joins)
- alias_tracker = alias_tracker(join_list, aliases)
+ string_joins.map! { |join| table.create_string_join(Arel.sql(join)) }
- join_dependency = construct_join_dependency(association_joins, join_type)
+ join_sources = manager.join_sources
+ join_sources.concat(join_nodes) unless join_nodes.empty?
- joins = join_dependency.join_constraints(stashed_joins, alias_tracker)
- joins.each { |join| manager.from(join) }
-
- manager.join_sources.concat(join_list)
-
- alias_tracker.aliases
- end
+ unless association_joins.empty? && stashed_joins.empty?
+ alias_tracker = alias_tracker(join_nodes + string_joins, aliases)
+ join_dependency = construct_join_dependency(association_joins, join_type)
+ join_sources.concat(join_dependency.join_constraints(stashed_joins, alias_tracker))
+ end
- def convert_join_strings_to_ast(joins)
- joins
- .flatten
- .reject(&:blank?)
- .map { |join| table.create_string_join(Arel.sql(join)) }
+ join_sources.concat(string_joins) unless string_joins.empty?
end
def build_select(arel)
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index 0ab99aa6cd..affa024d77 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -58,6 +58,14 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
assert_equal preloaded, Marshal.load(Marshal.dump(preloaded))
end
+ def test_through_association_with_joins
+ assert_equal [comments(:eager_other_comment1)], authors(:mary).comments.merge(Post.joins(:comments))
+ end
+
+ def test_through_association_with_left_joins
+ assert_equal [comments(:eager_other_comment1)], authors(:mary).comments.merge(Post.left_joins(:comments))
+ end
+
def test_preload_with_nested_association
posts = Post.preload(:author, :author_favorites_with_scope).to_a
--
cgit v1.2.3