From 2a35baa0bb4312d95e1340074cce731afedecde0 Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Sat, 24 Sep 2005 23:58:13 +0000 Subject: Wrap :conditions in parentheses to prevent problems with OR's #1871 git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@2324 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- activerecord/CHANGELOG | 2 ++ .../associations/has_many_association.rb | 2 +- activerecord/lib/active_record/base.rb | 2 +- activerecord/test/associations_go_eager_test.rb | 21 +++++++++++++-------- activerecord/test/finder_test.rb | 10 +++++++++- activerecord/test/fixtures/comments.yml | 6 ++++++ activerecord/test/fixtures/posts.yml | 7 +++++++ 7 files changed, 39 insertions(+), 11 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index cce58f7587..1983a18e50 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Wrap :conditions in parentheses to prevent problems with OR's #1871 + * Allow the postgresql adapter to work with the SchemaDumper. * Add ActiveRecord::SchemaDumper for dumping a DB schema to a pure-ruby file, making it easier to consolidate large migration lists and port database schemas between databases. diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index fdeadfc181..f9ca655560 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -68,7 +68,7 @@ module ActiveRecord else conditions = "#{@finder_sql}" if sanitized_conditions = sanitize_sql(options[:conditions]) - conditions << " AND #{sanitized_conditions}" + conditions << " AND (#{sanitized_conditions})" end options[:conditions] = conditions diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 03123758ef..6f655df26f 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -350,7 +350,7 @@ module ActiveRecord #:nodoc: return args.first if args.first.kind_of?(Array) && args.first.empty? expects_array = args.first.kind_of?(Array) - conditions = " AND #{sanitize_sql(options[:conditions])}" if options[:conditions] + conditions = " AND (#{sanitize_sql(options[:conditions])})" if options[:conditions] ids = args.flatten.compact.uniq case ids.size diff --git a/activerecord/test/associations_go_eager_test.rb b/activerecord/test/associations_go_eager_test.rb index 750e8bd208..ec088eebb7 100644 --- a/activerecord/test/associations_go_eager_test.rb +++ b/activerecord/test/associations_go_eager_test.rb @@ -20,14 +20,19 @@ class EagerAssociationTest < Test::Unit::TestCase assert post.comments.include?(comments(:greetings)) end + def test_loading_conditions_with_or + posts = authors(:david).posts.find(:all, :include => :comments, :conditions => "comments.body like 'Normal%' OR comments.type = 'SpecialComment'") + assert_nil posts.detect { |p| p.author_id != authors(:david).id }, + "expected to find only david's posts" + end + def test_with_ordering - posts = Post.find(:all, :include => :comments, :order => "posts.id DESC") - assert_equal posts(:sti_habtm), posts[0] - assert_equal posts(:sti_post_and_comments), posts[1] - assert_equal posts(:sti_comments), posts[2] - assert_equal posts(:authorless), posts[3] - assert_equal posts(:thinking), posts[4] - assert_equal posts(:welcome), posts[5] + list = Post.find(:all, :include => :comments, :order => "posts.id DESC") + [:eager_other, :sti_habtm, :sti_post_and_comments, :sti_comments, + :authorless, :thinking, :welcome + ].each_with_index do |post, index| + assert_equal posts(post), list[index] + end end def test_loading_with_multiple_associations @@ -48,7 +53,7 @@ class EagerAssociationTest < Test::Unit::TestCase def test_eager_association_loading_with_belongs_to comments = Comment.find(:all, :include => :post) - assert_equal 9, comments.length + assert_equal 10, comments.length titles = comments.map { |c| c.post.title } assert titles.include?(posts(:welcome).title) assert titles.include?(posts(:sti_post_and_comments).title) diff --git a/activerecord/test/finder_test.rb b/activerecord/test/finder_test.rb index 8fc89f3808..35c96288fe 100644 --- a/activerecord/test/finder_test.rb +++ b/activerecord/test/finder_test.rb @@ -3,9 +3,10 @@ require 'fixtures/company' require 'fixtures/topic' require 'fixtures/entrant' require 'fixtures/developer' +require 'fixtures/post' class FinderTest < Test::Unit::TestCase - fixtures :companies, :topics, :entrants, :developers + fixtures :companies, :topics, :entrants, :developers, :posts def test_find assert_equal(topics(:first).title, Topic.find(1).title) @@ -311,6 +312,13 @@ class FinderTest < Test::Unit::TestCase assert developer_names.include?('Jamis') end + def test_find_by_id_with_conditions_with_or + assert_nothing_raised do + Post.find([1,2,3], + :conditions => "posts.id <= 3 OR posts.type = 'Post'") + end + end + def test_select_value assert_equal "37signals", Company.connection.select_value("SELECT name FROM companies WHERE id = 1") assert_nil Company.connection.select_value("SELECT name FROM companies WHERE id = -1") diff --git a/activerecord/test/fixtures/comments.yml b/activerecord/test/fixtures/comments.yml index bfe61068e1..758eaf6dbf 100644 --- a/activerecord/test/fixtures/comments.yml +++ b/activerecord/test/fixtures/comments.yml @@ -57,3 +57,9 @@ check_eager_sti_on_associations2: post_id: 5 body: Special Type type: SpecialComment + +eager_other_comment1: + id: 11 + post_id: 7 + body: go crazy + type: SpecialComment diff --git a/activerecord/test/fixtures/posts.yml b/activerecord/test/fixtures/posts.yml index 190993dcdd..0f1445b638 100644 --- a/activerecord/test/fixtures/posts.yml +++ b/activerecord/test/fixtures/posts.yml @@ -39,3 +39,10 @@ sti_habtm: title: habtm sti test body: hello type: Post + +eager_other: + id: 7 + author_id: 2 + title: eager loading with OR'd conditions + body: hello + type: Post -- cgit v1.2.3