diff options
author | Paul Nikitochkin <paul.nikitochkin@gmail.com> | 2013-08-24 19:12:05 +0300 |
---|---|---|
committer | Paul Nikitochkin <paul.nikitochkin@gmail.com> | 2013-09-13 16:37:10 +0300 |
commit | fbbb6c87fd5e8fcd913c0dbf518024edd7538072 (patch) | |
tree | 0663c678b8a52e45de6eb2e86d8ead0d65fb543f | |
parent | 97a19c6b4b71d1e12d62d880ec8c8eed357b3d3c (diff) | |
download | rails-fbbb6c87fd5e8fcd913c0dbf518024edd7538072.tar.gz rails-fbbb6c87fd5e8fcd913c0dbf518024edd7538072.tar.bz2 rails-fbbb6c87fd5e8fcd913c0dbf518024edd7538072.zip |
Collapse where constraints to one where constraint
In order to remove duplication with joining arel where constraints with
`AND`, all constraints on `build_arel` are collapsed into one head node: `Arel::Nodes::And`
Closes: #11963
-rw-r--r-- | activerecord/CHANGELOG.md | 8 | ||||
-rw-r--r-- | activerecord/lib/active_record/relation/query_methods.rb | 11 | ||||
-rw-r--r-- | activerecord/test/cases/associations/inner_join_association_test.rb | 5 | ||||
-rw-r--r-- | activerecord/test/models/author.rb | 7 |
4 files changed, 25 insertions, 6 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index b79b9a87c6..38f8bfced9 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,11 @@ +* Fix: joins association, with defined in the scope block constraints by using several + where constraints and at least of them is not `Arel::Nodes::Equality`, + generates invalid SQL expression. + + Fixes: #11963 + + *Paul Nikitochkin* + * Re-use `order` argument pre-processing for `reorder`. *Paul Nikitochkin* diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 1c6ea94c0b..5d0df32582 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -882,14 +882,13 @@ module ActiveRecord end def collapse_wheres(arel, wheres) - equalities = wheres.grep(Arel::Nodes::Equality) - - arel.where(Arel::Nodes::And.new(equalities)) unless equalities.empty? - - (wheres - equalities).each do |where| + predicates = wheres.map do |where| + next where if ::Arel::Nodes::Equality === where where = Arel.sql(where) if String === where - arel.where(Arel::Nodes::Grouping.new(where)) + Arel::Nodes::Grouping.new(where) end + + arel.where(Arel::Nodes::And.new(predicates)) if predicates.present? end def build_where(opts, other = []) diff --git a/activerecord/test/cases/associations/inner_join_association_test.rb b/activerecord/test/cases/associations/inner_join_association_test.rb index de47a576c6..9fe5ff50d9 100644 --- a/activerecord/test/cases/associations/inner_join_association_test.rb +++ b/activerecord/test/cases/associations/inner_join_association_test.rb @@ -41,6 +41,11 @@ class InnerJoinAssociationTest < ActiveRecord::TestCase assert_no_match(/WHERE/i, sql) end + def test_join_association_conditions_support_string_and_arel_expressions + assert_equal 0, Author.joins(:welcome_posts_with_comment).count + assert_equal 1, Author.joins(:welcome_posts_with_comments).count + end + def test_join_conditions_allow_nil_associations authors = Author.includes(:essays).where(:essays => {:id => nil}) assert_equal 2, authors.count diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index feb828de31..5d20389331 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -31,6 +31,13 @@ class Author < ActiveRecord::Base has_many :thinking_posts, -> { where(:title => 'So I was thinking') }, :dependent => :delete_all, :class_name => 'Post' has_many :welcome_posts, -> { where(:title => 'Welcome to the weblog') }, :class_name => 'Post' + has_many :welcome_posts_with_comment, + -> { where(title: 'Welcome to the weblog').where('comments_count = ?', 1) }, + class_name: 'Post' + has_many :welcome_posts_with_comments, + -> { where(title: 'Welcome to the weblog').where(Post.arel_table[:comments_count].gt(0)) }, + class_name: 'Post' + has_many :comments_desc, -> { order('comments.id DESC') }, :through => :posts, :source => :comments has_many :limited_comments, -> { limit(1) }, :through => :posts, :source => :comments has_many :funky_comments, :through => :posts, :source => :comments |