diff options
| -rw-r--r-- | activerecord/CHANGELOG | 2 | ||||
| -rwxr-xr-x | activerecord/lib/active_record/associations.rb | 36 | ||||
| -rwxr-xr-x | activerecord/lib/active_record/base.rb | 4 | ||||
| -rw-r--r-- | activerecord/test/associations_go_eager_test.rb | 45 |
4 files changed, 76 insertions, 11 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 2dca819dfb..94d29ef3b5 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Added support for limit and offset with eager loading of has_one and belongs_to associations. Using the options with has_many and has_and_belongs_to_many associations will now raise an ActiveRecord::ConfigurationError #1692 [Rick Olsen] + * Fixed that assume_bottom_position (in acts_as_list) could be called on items already last in the list and they would move one position away from the list #1648 [tyler@kianta.com] * Added ActiveRecord::Base.threaded_connections flag to turn off 1-connection per thread (required for thread safety). By default it's on, but WEBrick in Rails need it off #1685 [Sam Stephenson] diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 6f49800df6..ffb0d5679b 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -168,7 +168,8 @@ module ActiveRecord # catch-all for performance problems, but its a great way to cut down on the number of queries in a situation as the one described above. # # Please note that because eager loading is fetching both models and associations in the same grab, it doesn't make sense to use the - # :limit property and it will be ignored if attempted. + # :limit and :offset options on has_many and has_and_belongs_to_many associations and an ConfigurationError exception will be raised + # if attempted. It does, however, work just fine with has_one and belongs_to associations. # # Also have in mind that since the eager loading is pulling from multiple tables, you'll have to disambiguate any column references # in both conditions and orders. So :order => "posts.id DESC" will work while :order => "id DESC" will not. This may require that @@ -740,12 +741,9 @@ module ActiveRecord def find_with_associations(options = {}) reflections = reflect_on_included_associations(options[:include]) - reflections.each do |r| - raise( - NoMethodError, - "Association was not found; perhaps you misspelled it? You specified :include=>:#{options[:include].join(', :')}" - ) if r.nil? - end + + guard_against_missing_reflections(reflections, options) + guard_against_unlimitable_reflections(reflections, options) schema_abbreviations = generate_schema_abbreviations(reflections) primary_key_table = generate_primary_key_table(reflections, schema_abbreviations) @@ -788,6 +786,24 @@ module ActiveRecord [ associations ].flatten.collect { |association| reflect_on_association(association) } end + def guard_against_missing_reflections(reflections, options) + reflections.each do |r| + raise( + ConfigurationError, + "Association was not found; perhaps you misspelled it? You specified :include => :#{options[:include].join(', :')}" + ) if r.nil? + end + end + + def guard_against_unlimitable_reflections(reflections, options) + if (options[:offset] || options[:limit]) && !using_limitable_reflections?(reflections) + raise( + ConfigurationError, + "You can not use offset and limit together with has_many or has_and_belongs_to_many associations" + ) + end + end + def generate_schema_abbreviations(reflections) schema = [ [ table_name, column_names ] ] schema += reflections.collect { |r| [ r.table_name, r.klass.column_names ] } @@ -830,10 +846,14 @@ module ActiveRecord add_conditions!(sql, options[:conditions]) add_sti_conditions!(sql, reflections) sql << "ORDER BY #{options[:order]} " if options[:order] - + add_limit!(sql, options) if using_limitable_reflections?(reflections) return sanitize_sql(sql) end + def using_limitable_reflections?(reflections) + reflections.reject { |r| [ :belongs_to, :has_one ].include?(r.macro) }.length.zero? + end + def add_sti_conditions!(sql, reflections) sti_sql = "" reflections.each do |reflection| diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index b32843d069..be64c1bbd3 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -26,6 +26,8 @@ module ActiveRecord #:nodoc: end class StaleObjectError < ActiveRecordError #:nodoc: end + class ConfigurationError < StandardError #:nodoc: + end class AttributeAssignmentError < ActiveRecordError #:nodoc: attr_reader :exception, :attribute @@ -336,7 +338,7 @@ module ActiveRecord #:nodoc: case args.first when :first - find(:all, options.merge({ :limit => 1 })).first + find(:all, options.merge(options[:include] ? { } : { :limit => 1 })).first when :all options[:include] ? find_with_associations(options) : find_by_sql(construct_finder_sql(options)) else diff --git a/activerecord/test/associations_go_eager_test.rb b/activerecord/test/associations_go_eager_test.rb index 892a879283..cecc41ab50 100644 --- a/activerecord/test/associations_go_eager_test.rb +++ b/activerecord/test/associations_go_eager_test.rb @@ -48,10 +48,51 @@ class EagerAssociationTest < Test::Unit::TestCase def test_eager_association_loading_with_belongs_to comments = Comment.find(:all, :include => :post) + assert_equal 9, comments.length titles = comments.map { |c| c.post.title } assert titles.include?(posts(:welcome).title) assert titles.include?(posts(:sti_post_and_comments).title) end + + def test_eager_association_loading_with_belongs_to_and_limit + comments = Comment.find(:all, :include => :post, :limit => 5) + assert_equal 5, comments.length + assert_equal [1,2,3,5,6], comments.collect { |c| c.id } + end + + def test_eager_association_loading_with_belongs_to_and_limit_and_conditions + comments = Comment.find(:all, :include => :post, :conditions => 'post_id = 4', :limit => 3) + assert_equal 3, comments.length + assert_equal [5,6,7], comments.collect { |c| c.id } + end + + def test_eager_association_loading_with_belongs_to_and_limit_and_offset + comments = Comment.find(:all, :include => :post, :limit => 3, :offset => 2) + assert_equal 3, comments.length + assert_equal [3,5,6], comments.collect { |c| c.id } + end + + def test_eager_association_loading_with_belongs_to_and_limit_and_offset_and_conditions + comments = Comment.find(:all, :include => :post, :conditions => 'post_id = 4', :limit => 3, :offset => 1) + assert_equal 3, comments.length + assert_equal [6,7,8], comments.collect { |c| c.id } + end + + def test_eager_association_loading_with_belongs_to_and_limit_and_multiple_associations + posts = Post.find(:all, :include => [:author, :very_special_comment], :limit => 1) + assert_equal 1, posts.length + assert_equal [4], posts.collect { |p| p.id } + end + + def test_eager_association_loading_with_belongs_to_and_limit_and_offset_and_multiple_associations + posts = Post.find(:all, :include => [:author, :very_special_comment], :limit => 1, :offset => 1) + assert_equal 0, posts.length + assert_equal [], posts + end + + def test_eager_association_raise_on_limit + assert_raises(ActiveRecord::ConfigurationError) { Post.find(:all, :include => [:author, :comments], :limit => 1) } + end def test_eager_association_loading_with_habtm posts = Post.find(:all, :include => :categories, :order => "posts.id") @@ -95,10 +136,10 @@ class EagerAssociationTest < Test::Unit::TestCase end def test_eager_with_invalid_association_reference - assert_raises(NoMethodError, "Association was not found; perhaps you misspelled it? You specified :include=>:monkeys") { + assert_raises(ActiveRecord::ConfigurationError, "Association was not found; perhaps you misspelled it? You specified :include => :monkeys") { post = Post.find(6, :include=>[ :monkeys ]) } - assert_raises(NoMethodError, "Association was not found; perhaps you misspelled it? You specified :include=>:monkeys, :elephants") { + assert_raises(ActiveRecord::ConfigurationError, "Association was not found; perhaps you misspelled it? You specified :include => :monkeys, :elephants") { post = Post.find(6, :include=>[ :monkeys, :elephants ]) } end |